Improve error messages for opUnary (#20801)

dlang · Jan 30, 2025 · 06d0cfe · 06d0cfe
1 parent 06e0096
commit 06d0cfe
Show file tree

Hide file tree

Showing 5 changed files with 192 additions and 173 deletions.
diff --git a/changelog/dmd.error-messages.dd b/changelog/dmd.error-messages.dd
@@ -30,7 +30,12 @@ Error: `app.bar` called with argument types `(string)` matches multiple overload
 */
 ---
 
-When there's no index / slice operator overload found for a type, a new supplemental message suggests where to implement it.
+Error messages related to operator overloading have been improved.
+When the related template functions (`opUnary`, `opBinary`, `opBinaryRight`, `opOpAssign`, `opIndex`, `opSlice`)
+are missing, a suggestion to implement them is given.
+
+When they do exist but fail to instantiate, the error from instantiation is shown.
+There's no longer a need to manually e.g. rewrite `s + 1` to `s.opBinary!"+"(1)` to diagnose the error.
 
 ---
 struct S {}
@@ -53,9 +58,6 @@ app.d(1):        perhaps define `auto opSlice(int lower, string upper) {}` for `
 */
 ---
 
-When overloading binary operators, and `opBinary`, `opBinaryRight` or `opOpAssign` is missing / fails to instantiate,
-the error message now points out the problem:
-
 ---
 struct Str {}
 

diff --git a/compiler/src/dmd/expressionsem.d b/compiler/src/dmd/expressionsem.d
@@ -8857,9 +8857,8 @@ private extern (C++) final class ExpressionSemanticVisitor : Visitor
             result = exp.incompatibleTypes();
             return;
         }
-        if (exp.e1.checkNoBool())
-            return setError();
-        if (exp.e1.checkArithmetic(exp.op) ||
+        if (exp.e1.checkNoBool() ||
+            exp.e1.checkArithmetic(exp.op) ||
             exp.e1.checkSharedAccess(sc))
             return setError();
 
@@ -8885,11 +8884,10 @@ private extern (C++) final class ExpressionSemanticVisitor : Visitor
             result = exp.incompatibleTypes();
             return;
         }
-        if (exp.e1.checkNoBool())
-            return setError();
-        if (exp.e1.checkArithmetic(exp.op))
-            return setError();
-        if (exp.e1.checkSharedAccess(sc))
+
+        if (exp.e1.checkNoBool() ||
+            exp.e1.checkArithmetic(exp.op) ||
+            exp.e1.checkSharedAccess(sc))
             return setError();
 
         result = exp.e1;
@@ -8922,9 +8920,8 @@ private extern (C++) final class ExpressionSemanticVisitor : Visitor
             result = exp.incompatibleTypes();
             return;
         }
-        if (exp.e1.checkNoBool())
-            return setError();
-        if (exp.e1.checkIntegral() ||
+        if (exp.e1.checkNoBool() ||
+            exp.e1.checkIntegral() ||
             exp.e1.checkSharedAccess(sc))
             return setError();
 

diff --git a/compiler/src/dmd/opover.d b/compiler/src/dmd/opover.d
@@ -293,6 +293,16 @@ Expression opOverloadUnary(UnaExp e, Scope* sc)
             }
             break;
         }
+
+        // For ++ and --, rewrites to += and -= are also tried, so don't error yet
+        if (!e.isPreExp())
+        {
+            error(e.loc, "operator `%s` is not defined for `%s`", EXPtoString(e.op).ptr, ad.toChars());
+            errorSupplemental(ad.loc, "perhaps overload the operator with `auto opUnary(string op : \"%s\")() {}`",
+                EXPtoString(e.op).ptr);
+            return ErrorExp.get();
+        }
+
         break;
     }
     return result;