From a55dff5f286a7876fedef9893ee65268866cdbde Mon Sep 17 00:00:00 2001
From: Dennis <dkorpel@users.noreply.github.com>
Date: Tue, 28 Jan 2025 11:44:46 +0100
Subject: [PATCH] Improve opIndex/opSlice error messages (#20791)

---
 changelog/dmd.error-messages.dd               | 20 +++++++++--
 compiler/src/dmd/expressionsem.d              | 35 ++++++++++++++++--
 compiler/src/dmd/opover.d                     |  6 ++--
 compiler/test/fail_compilation/fail20616.d    |  4 +--
 .../test/fail_compilation/fail_arrayexp.d     | 23 +++++++++++-
 compiler/test/fail_compilation/fail_opover.d  | 36 +++++++++----------
 6 files changed, 95 insertions(+), 29 deletions(-)

diff --git a/changelog/dmd.error-messages.dd b/changelog/dmd.error-messages.dd
index 62d362f9ce..ddb3f6341f 100644
--- a/changelog/dmd.error-messages.dd
+++ b/changelog/dmd.error-messages.dd
@@ -30,12 +30,26 @@ Error: `app.bar` called with argument types `(string)` matches multiple overload
 */
 ---
 
-When there's no operator overload found for a type, a new supplemental message points to the declaration.
+When there's no index / slice operator overload found for a type, a new supplemental message suggests where to implement it.
 
 ---
+struct S {}
+
+void main()
+{
+    S s;
+    const x = s[3 .. "4"];
+}
+
 /*
-app.d(8): Error: no `[]` operator overload for type `U`
-app.d(6):        `app.U` declared here
+Before:
+
+app.d(6): Error: no `[]` operator overload for type `S`
+
+After:
+
+app.d(6): Error: no `[3.."4"]` operator overload for type `S`
+app.d(1):        perhaps define `auto opSlice(int lower, string upper) {}` for `app.S`
 */
 ---
 
diff --git a/compiler/src/dmd/expressionsem.d b/compiler/src/dmd/expressionsem.d
index d281e1fede..bb50cc4a39 100644
--- a/compiler/src/dmd/expressionsem.d
+++ b/compiler/src/dmd/expressionsem.d
@@ -9706,8 +9706,39 @@ private extern (C++) final class ExpressionSemanticVisitor : Visitor
 
         if (auto ad = isAggregate(exp.e1.type))
         {
-            error(exp.loc, "no `[]` operator overload for type `%s`", exp.e1.type.toChars());
-            errorSupplemental(ad.loc, "`%s` declared here", ad.toPrettyChars());
+            if (exp.arguments.length == 0)
+            {
+                error(exp.loc, "no `[]` operator overload for type `%s`", exp.e1.type.toChars());
+                errorSupplemental(ad.loc, "perhaps define `auto opIndex() {}` for `%s`", ad.toPrettyChars());
+            }
+            else
+            {
+                const(char)* typeString(Expression exp)
+                {
+                    if (auto e = exp.trySemantic(sc))
+                        return e.type.toChars();
+                    else
+                        return "__error__";
+                }
+
+                if (auto ie = (*exp.arguments)[0].isIntervalExp())
+                {
+                    error(exp.loc, "no `[%s]` operator overload for type `%s`", ie.toChars(), exp.e1.type.toChars());
+                    errorSupplemental(ad.loc, "perhaps define `auto opSlice(%s lower, %s upper) {}` for `%s`",
+                        typeString(ie.lwr), typeString(ie.upr), ad.toPrettyChars());
+                }
+                else
+                {
+                    OutBuffer buf;
+                    buf.printf("%s", typeString((*exp.arguments)[0]));
+                    foreach (e; (*exp.arguments)[1 .. $])
+                        buf.printf(", %s", typeString(e));
+
+                    error(exp.loc, "no `[]` operator overload for type `%s`", exp.e1.type.toChars());
+                    errorSupplemental(ad.loc, "perhaps define `auto opIndex(%s) {}` for `%s`",
+                        buf.extractChars, ad.toPrettyChars());
+                }
+            }
         }
         else if (exp.e1.op == EXP.type && exp.e1.type.ty != Ttuple)
             error(exp.loc, "static array of `%s` with multiple lengths not allowed", exp.e1.type.toChars());
diff --git a/compiler/src/dmd/opover.d b/compiler/src/dmd/opover.d
index 1aeeb2e2e4..d1b2ef480b 100644
--- a/compiler/src/dmd/opover.d
+++ b/compiler/src/dmd/opover.d
@@ -403,9 +403,9 @@ Expression opOverloadArray(ArrayExp ae, Scope* sc)
 
             if (result.op == EXP.error)
             {
-                if (!e0 && !search_function(ad, Id.dollar)) {
-                    ae.loc.errorSupplemental("Aggregate declaration '%s' does not define 'opDollar'", ae.e1.toChars());
-                }
+                if (!e0 && !search_function(ad, Id.dollar))
+                    ad.loc.errorSupplemental("perhaps define `opDollar` for `%s`", ad.toChars());
+
                 return result;
             }
             /* Rewrite a[i..j] as:
diff --git a/compiler/test/fail_compilation/fail20616.d b/compiler/test/fail_compilation/fail20616.d
index 0f76e315ac..3acf84fb8f 100644
--- a/compiler/test/fail_compilation/fail20616.d
+++ b/compiler/test/fail_compilation/fail20616.d
@@ -2,9 +2,9 @@
 TEST_OUTPUT:
 ---
 fail_compilation/fail20616.d(16): Error: undefined identifier `$`
-fail_compilation/fail20616.d(16):        Aggregate declaration 'X()' does not define 'opDollar'
+fail_compilation/fail20616.d(13):        perhaps define `opDollar` for `X`
 fail_compilation/fail20616.d(18): Error: undefined identifier `$`
-fail_compilation/fail20616.d(18):        Aggregate declaration 'b' does not define 'opDollar'
+fail_compilation/fail20616.d(13):        perhaps define `opDollar` for `X`
 ---
 */
 module fail20616;
diff --git a/compiler/test/fail_compilation/fail_arrayexp.d b/compiler/test/fail_compilation/fail_arrayexp.d
index c20541392e..6f9868a48b 100644
--- a/compiler/test/fail_compilation/fail_arrayexp.d
+++ b/compiler/test/fail_compilation/fail_arrayexp.d
@@ -6,7 +6,7 @@ fail_compilation/fail_arrayexp.d(25): Error: cannot use `[]` operator on express
 fail_compilation/fail_arrayexp.d(26): Error: static array of `const(int)[]` with multiple lengths not allowed
 fail_compilation/fail_arrayexp.d(27): Error: only one index allowed to index `string`
 fail_compilation/fail_arrayexp.d(28): Error: no `[]` operator overload for type `U`
-fail_compilation/fail_arrayexp.d(16):        `fail_arrayexp.U` declared here
+fail_compilation/fail_arrayexp.d(16):        perhaps define `auto opIndex() {}` for `fail_arrayexp.U`
 fail_compilation/fail_arrayexp.d(29): Error: only one index allowed to index `(int, string)`
 ---
 */
@@ -29,3 +29,24 @@ void test19534() // https://issues.dlang.org/show_bug.cgi?id=19534
     auto t = agg[];
     auto u = getTuple!(int, string)[1, 2];
 }
+
+/*
+TEST_OUTPUT:
+---
+fail_compilation/fail_arrayexp.d(49): Error: no `[3.."4"]` operator overload for type `S`
+fail_compilation/fail_arrayexp.d(42):        perhaps define `auto opSlice(int lower, string upper) {}` for `fail_arrayexp.S`
+fail_compilation/fail_arrayexp.d(50): Error: no `[]` operator overload for type `S`
+fail_compilation/fail_arrayexp.d(42):        perhaps define `auto opIndex(int, string, char) {}` for `fail_arrayexp.S`
+---
+*/
+
+struct S
+{
+}
+
+void testSlice()
+{
+    S s;
+    const b = s[3 .. "4"];
+    const c = s[3, "4", 'c'];
+}
diff --git a/compiler/test/fail_compilation/fail_opover.d b/compiler/test/fail_compilation/fail_opover.d
index 368755ae78..1a0c9e1ac1 100644
--- a/compiler/test/fail_compilation/fail_opover.d
+++ b/compiler/test/fail_compilation/fail_opover.d
@@ -4,33 +4,33 @@
 TEST_OUTPUT:
 ---
 fail_compilation/fail_opover.d(39): Error: no `[]` operator overload for type `object.Object`
-$p:object.d$(110):        `object.Object` declared here
+$p:object.d$(110):        perhaps define `auto opIndex() {}` for `object.Object`
 fail_compilation/fail_opover.d(43): Error: no `[]` operator overload for type `TestS`
-fail_compilation/fail_opover.d(41):        `fail_opover.test1.TestS` declared here
+fail_compilation/fail_opover.d(41):        perhaps define `auto opIndex() {}` for `fail_opover.test1.TestS`
 fail_compilation/fail_opover.d(55): Error: no `[]` operator overload for type `S`
-fail_compilation/fail_opover.d(48):        `fail_opover.test2.S` declared here
+fail_compilation/fail_opover.d(48):        perhaps define `auto opIndex() {}` for `fail_opover.test2.S`
 fail_compilation/fail_opover.d(56): Error: no `[]` operator overload for type `S`
-fail_compilation/fail_opover.d(48):        `fail_opover.test2.S` declared here
-fail_compilation/fail_opover.d(57): Error: no `[]` operator overload for type `S`
-fail_compilation/fail_opover.d(48):        `fail_opover.test2.S` declared here
+fail_compilation/fail_opover.d(48):        perhaps define `auto opIndex(int) {}` for `fail_opover.test2.S`
+fail_compilation/fail_opover.d(57): Error: no `[1..2]` operator overload for type `S`
+fail_compilation/fail_opover.d(48):        perhaps define `auto opSlice(int lower, int upper) {}` for `fail_opover.test2.S`
 fail_compilation/fail_opover.d(58): Error: no `[]` operator overload for type `S`
-fail_compilation/fail_opover.d(48):        `fail_opover.test2.S` declared here
+fail_compilation/fail_opover.d(48):        perhaps define `auto opIndex() {}` for `fail_opover.test2.S`
 fail_compilation/fail_opover.d(59): Error: no `[]` operator overload for type `S`
-fail_compilation/fail_opover.d(48):        `fail_opover.test2.S` declared here
-fail_compilation/fail_opover.d(60): Error: no `[]` operator overload for type `S`
-fail_compilation/fail_opover.d(48):        `fail_opover.test2.S` declared here
+fail_compilation/fail_opover.d(48):        perhaps define `auto opIndex(int) {}` for `fail_opover.test2.S`
+fail_compilation/fail_opover.d(60): Error: no `[1..2]` operator overload for type `S`
+fail_compilation/fail_opover.d(48):        perhaps define `auto opSlice(int lower, int upper) {}` for `fail_opover.test2.S`
 fail_compilation/fail_opover.d(61): Error: no `[]` operator overload for type `S`
-fail_compilation/fail_opover.d(48):        `fail_opover.test2.S` declared here
+fail_compilation/fail_opover.d(48):        perhaps define `auto opIndex() {}` for `fail_opover.test2.S`
 fail_compilation/fail_opover.d(62): Error: no `[]` operator overload for type `S`
-fail_compilation/fail_opover.d(48):        `fail_opover.test2.S` declared here
-fail_compilation/fail_opover.d(63): Error: no `[]` operator overload for type `S`
-fail_compilation/fail_opover.d(48):        `fail_opover.test2.S` declared here
+fail_compilation/fail_opover.d(48):        perhaps define `auto opIndex(int) {}` for `fail_opover.test2.S`
+fail_compilation/fail_opover.d(63): Error: no `[1..2]` operator overload for type `S`
+fail_compilation/fail_opover.d(48):        perhaps define `auto opSlice(int lower, int upper) {}` for `fail_opover.test2.S`
 fail_compilation/fail_opover.d(64): Error: no `[]` operator overload for type `S`
-fail_compilation/fail_opover.d(48):        `fail_opover.test2.S` declared here
+fail_compilation/fail_opover.d(48):        perhaps define `auto opIndex() {}` for `fail_opover.test2.S`
 fail_compilation/fail_opover.d(65): Error: no `[]` operator overload for type `S`
-fail_compilation/fail_opover.d(48):        `fail_opover.test2.S` declared here
-fail_compilation/fail_opover.d(66): Error: no `[]` operator overload for type `S`
-fail_compilation/fail_opover.d(48):        `fail_opover.test2.S` declared here
+fail_compilation/fail_opover.d(48):        perhaps define `auto opIndex(int) {}` for `fail_opover.test2.S`
+fail_compilation/fail_opover.d(66): Error: no `[1..2]` operator overload for type `S`
+fail_compilation/fail_opover.d(48):        perhaps define `auto opSlice(int lower, int upper) {}` for `fail_opover.test2.S`
 ---
 */
 void test1()