Merge pull request #22285 from CyberShadow/dip1000-scope-errors

Improve scope violation error messages to show inference chain

Author: dkorpel

compiler/src/dmd/escape.d

@@ -293,7 +293,7 @@ bool checkAssocArrayLiteralEscape(ref Scope sc, AssocArrayLiteralExp ae, bool ga
  *    recursionLimit = recursion limit for printing the reason
  *    positive = true for telling why v is scope, false for telling why v is not scope
  */
-private
+public
 void printScopeReason(E)(E printFunc, VarDeclaration v, int recursionLimit, bool positive)
 {
     recursionLimit--;
@@ -375,8 +375,8 @@ bool checkParamArgumentEscape(ref Scope sc, FuncDeclaration fdc, Identifier parI
             (desc ~ " `%s` assigned to non-scope anonymous parameter");
 
         if (isThis ?
-            sc.setUnsafeDIP1000(gag, arg.loc, msg, arg, fdc.toParent2(), fdc) :
-            sc.setUnsafeDIP1000(gag, arg.loc, msg, v, parId ? parId : fdc, fdc))
+            sc.setUnsafeDIP1000(gag, arg.loc, vPar, msg, arg, fdc.toParent2(), fdc) :
+            sc.setUnsafeDIP1000(gag, arg.loc, vPar, msg, v, parId ? parId : fdc, fdc))
         {
             result = true;
             printScopeReason(previewSupplementalFunc(sc.isDeprecated(), sc.useDIP1000), vPar, 10, false);
@@ -2214,6 +2214,14 @@ bool setUnsafeDIP1000(ref Scope sc, bool gag, Loc loc, const(char)* msg,
     return setUnsafePreview(&sc, sc.useDIP1000, gag, loc, msg, args);
 }
 
+/// Overload for scope violations that also stores the variable whose scope status caused the issue
+private
+bool setUnsafeDIP1000(ref Scope sc, bool gag, Loc loc, VarDeclaration scopeVar,
+    const(char)* msg, RootObject[] args...)
+{
+    return setUnsafePreview(&sc, sc.useDIP1000, gag, loc, scopeVar, msg, args);
+}
+
 /***************************************
  * Check that taking the address of `v` is `@safe`
  *

compiler/src/dmd/expressionsem.d

@@ -3164,6 +3164,12 @@ public void errorSupplementalInferredAttr(FuncDeclaration fd, int maxDepth, bool
     if (s.action.length > 0)
     {
         errorFunc(s.loc, "and %.*s makes it fail to infer `%.*s`", s.action.fTuple.expand, attr.fTuple.expand);
+        // For scope violations, also print why the target parameter is not scope
+        if (s.scopeVar)
+        {
+            import dmd.escape : printScopeReason;
+            printScopeReason(errorFunc, s.scopeVar, 10, false);
+        }
     }
     else if (s.fd)
     {

compiler/src/dmd/frontend.h

@@ -3707,10 +3707,12 @@ struct AttributeViolation final
     Loc loc;
     FuncDeclaration* fd;
     _d_dynamicArray< const char > action;
+    VarDeclaration* scopeVar;
     AttributeViolation() :
         loc(),
         fd(),
-        action()
+        action(),
+        scopeVar()
     {
     }
 };

compiler/src/dmd/func.d

@@ -1430,6 +1430,8 @@ struct AttributeViolation
 
     string action;   /// Action that made the attribute fail to get inferred
 
+    VarDeclaration scopeVar;  /// For scope violations: the parameter whose scope status caused the issue
+
     this(Loc loc, FuncDeclaration fd) { this.loc = loc; this.fd = fd; }
 
     this(Loc loc, const(char)* fmt, RootObject[] args)
@@ -1445,4 +1447,10 @@ struct AttributeViolation
         );
         this.action = buf.extractSlice();
     }
+
+    this(Loc loc, const(char)* fmt, VarDeclaration scopeVar, RootObject[] args)
+    {
+        this(loc, fmt, args);
+        this.scopeVar = scopeVar;
+    }
 }

compiler/src/dmd/safe.d

@@ -370,12 +370,22 @@ bool isTrusted(FuncDeclaration fd)
  */
 extern (D) void reportSafeError(FuncDeclaration fd, bool gag, Loc loc,
     const(char)* format, RootObject[] args...)
+{
+    reportSafeError(fd, gag, loc, null, format, args);
+}
+
+/// Overload that also stores the variable whose scope status caused the violation
+extern (D) void reportSafeError(FuncDeclaration fd, bool gag, Loc loc,
+    VarDeclaration scopeVar, const(char)* format, RootObject[] args...)
 {
     if (fd.type.toTypeFunction().trust == TRUST.system) // function was just inferred to be @system
     {
         if (format)
         {
-            fd.safetyViolation = new AttributeViolation(loc, format, args);
+            if (scopeVar)
+                fd.safetyViolation = new AttributeViolation(loc, format, scopeVar, args);
+            else
+                fd.safetyViolation = new AttributeViolation(loc, format, args);
         }
         else if (args.length > 0)
         {
@@ -460,6 +470,13 @@ extern (D) bool setUnsafeCall(FuncDeclaration fd, FuncDeclaration f)
  * Returns: whether there is a safe error
  */
 bool setUnsafe(Scope* sc, bool gag, Loc loc, const(char)* format, RootObject[] args...)
+{
+    return sc.setUnsafe(gag, loc, null, format, args);
+}
+
+/// Overload that also stores the variable whose scope status caused the violation
+bool setUnsafe(Scope* sc, bool gag, Loc loc, VarDeclaration scopeVar,
+    const(char)* format, RootObject[] args...)
 {
     if (sc.intypeof)
         return false; // typeof(cast(int*)0) is safe
@@ -504,7 +521,7 @@ bool setUnsafe(Scope* sc, bool gag, Loc loc, const(char)* format, RootObject[] a
     {
         if (format || args.length > 0)
         {
-            reportSafeError(sc.func, gag, loc, format, args);
+            reportSafeError(sc.func, gag, loc, scopeVar, format, args);
         }
         return sc.func.isSafe(); // it is only an error if in an @safe function
     }
@@ -531,6 +548,13 @@ bool setUnsafe(Scope* sc, bool gag, Loc loc, const(char)* format, RootObject[] a
  * Returns: whether an actual safe error (not deprecation) occured
  */
 bool setUnsafePreview(Scope* sc, FeatureState fs, bool gag, Loc loc, const(char)* format, RootObject[] args...)
+{
+    return setUnsafePreview(sc, fs, gag, loc, null, format, args);
+}
+
+/// Overload for scope violations that also stores the variable whose scope status caused the issue
+bool setUnsafePreview(Scope* sc, FeatureState fs, bool gag, Loc loc, VarDeclaration scopeVar,
+    const(char)* format, RootObject[] args...)
 {
     //printf("setUnsafePreview() fs:%d %s\n", fs, fmt);
     assert(format);
@@ -540,7 +564,7 @@ bool setUnsafePreview(Scope* sc, FeatureState fs, bool gag, Loc loc, const(char)
         return false;
 
       case enabled:
-        return sc.setUnsafe(gag, loc, format, args);
+        return sc.setUnsafe(gag, loc, scopeVar, format, args);
 
       case default_:
         if (!sc.func)
@@ -556,7 +580,10 @@ bool setUnsafePreview(Scope* sc, FeatureState fs, bool gag, Loc loc, const(char)
         else if (!sc.func.safetyViolation)
         {
             import dmd.func : AttributeViolation;
-            sc.func.safetyViolation = new AttributeViolation(loc, format, args);
+            if (scopeVar)
+                sc.func.safetyViolation = new AttributeViolation(loc, format, scopeVar, args);
+            else
+                sc.func.safetyViolation = new AttributeViolation(loc, format, args);
         }
         return false;
     }

compiler/test/fail_compilation/scope_infer_diagnostic.d

@@ -0,0 +1,35 @@
+/*
+REQUIRED_ARGS: -preview=dip1000
+TEST_OUTPUT:
+---
+fail_compilation/scope_infer_diagnostic.d(34): Error: `@safe` function `scope_infer_diagnostic.outer` cannot call `@system` function `scope_infer_diagnostic.inner!(void delegate(const(char)[]) pure nothrow @nogc @safe).inner`
+fail_compilation/scope_infer_diagnostic.d(28):        and assigning scope variable `w` to non-scope parameter `w` calling `callee` makes it fail to infer `@safe`
+fail_compilation/scope_infer_diagnostic.d(21):        `w` is not `scope` because of `globalPtr = & w`
+fail_compilation/scope_infer_diagnostic.d(25):        `scope_infer_diagnostic.inner!(void delegate(const(char)[]) pure nothrow @nogc @safe).inner` is declared here
+---
+*/
+
+// Test that scope violation error messages show WHY the callee's parameter
+// is not scope when going through the @safe inference chain.
+// This is similar to how @nogc violations show the full inference chain.
+
+void* globalPtr;
+
+void callee(Writer)(Writer w) @trusted
+{
+    // This escapes w, preventing it from being inferred as scope
+    globalPtr = cast(void*)&w;
+    w("x");
+}
+
+void inner(Writer)(scope Writer w)
+{
+    // Scope violation: passing scope w to non-scope parameter
+    callee(w);
+}
+
+void outer() @safe
+{
+    int x;
+    inner((const(char)[] s) { x++; });
+}