Fix #18670 - Default arguments bypass most attribute checks

changelog/dmd.default-arg-attributes.dd

@@ -0,0 +1,19 @@
+Attribute violations in default arguments now emit deprecations
+
+Calling an impure, `@system`, or non-`@nogc` function in a default argument
+of a `pure`, `@safe`, or `@nogc` function was silently accepted.
+These violations are now reported as deprecations.
+
+```d
+int stderr() => 0;
+void write(int outfile = stderr()) pure {}
+
+void main() pure
+{
+    write(); // stderr() gets inserted as argument
+}
+```
+
+$(CONSOLE
+test.d(2): Deprecation: `pure` function `main` calling impure function `stderr` in default argument
+)

compiler/src/dmd/expressionsem.d

@@ -3086,21 +3086,33 @@ private bool checkPurity(FuncDeclaration f, Loc loc, Scope* sc)
         return false;
     if (sc.ctfe || sc.debug_)
         return false;
-    if (sc.inDefaultArg || sc.callLoc.isValid)
-        return false;
+
+    const bool useDeprecation = sc.inDefaultArg || sc.callLoc.isValid;
 
     // If the call has a pure parent, then the called func must be pure.
-    if (!f.isPure() && checkImpure(sc, loc, null, f))
+    if (!f.isPure())
     {
-        error(loc, "`pure` %s `%s` cannot call impure %s `%s`",
-            sc.func.kind(), sc.func.toPrettyChars(), f.kind(),
-            f.toPrettyChars());
+        const bool violated = isRootTraitsCompilesScope(sc) || useDeprecation
+            ? sc.func.isPureBypassingInference() >= PURE.weak
+            : checkImpure(sc, loc, null, f);
+        if (violated)
+        {
+            if (useDeprecation)
+            {
+                .deprecation(loc, "`pure` %s `%s` calling impure %s `%s` in default argument",
+                    sc.func.kind(), sc.func.toPrettyChars(), f.kind(), f.toPrettyChars());
+                return false;
+            }
+            error(loc, "`pure` %s `%s` cannot call impure %s `%s`",
+                sc.func.kind(), sc.func.toPrettyChars(), f.kind(),
+                f.toPrettyChars());
 
-        if (!f.isDtorDeclaration())
-            errorSupplementalInferredAttr(f, /*max depth*/ 10, /*deprecation*/ false, STC.pure_, global.errorSink);
+            if (!f.isDtorDeclaration())
+                errorSupplementalInferredAttr(f, /*max depth*/ 10, /*deprecation*/ false, STC.pure_, global.errorSink);
 
-        f.checkOverriddenDtor(sc, loc, dd => dd.type.toTypeFunction().purity != PURE.impure, "impure");
-        return true;
+            f.checkOverriddenDtor(sc, loc, dd => dd.type.toTypeFunction().purity != PURE.impure, "impure");
+            return true;
+        }
     }
     return false;
 }
@@ -3283,6 +3295,8 @@ private bool checkPurity(VarDeclaration v, Loc loc, Scope* sc)
         }
     }
 
+    const bool useDeprecation = sc.callLoc.isValid;
+
     bool err = false;
     if (v.isDataseg())
     {
@@ -3291,11 +3305,20 @@ private bool checkPurity(VarDeclaration v, Loc loc, Scope* sc)
         if (v.ident == Id.gate)
             return false;
 
-        if (checkImpure(sc, loc, "accessing mutable static data `%s`", v))
+        const bool violated = useDeprecation
+            ? sc.func.isPureBypassingInference() >= PURE.weak
+            : checkImpure(sc, loc, "accessing mutable static data `%s`", v);
+        if (violated)
         {
-            error(loc, "`pure` %s `%s` cannot access mutable static data `%s`",
-                sc.func.kind(), sc.func.toPrettyChars(), v.toErrMsg());
-            err = true;
+            if (useDeprecation)
+                .deprecation(loc, "`pure` %s `%s` accessing mutable static data `%s` in default argument",
+                    sc.func.kind(), sc.func.toPrettyChars(), v.toErrMsg());
+            else
+            {
+                error(loc, "`pure` %s `%s` cannot access mutable static data `%s`",
+                    sc.func.kind(), sc.func.toPrettyChars(), v.toErrMsg());
+                err = true;
+            }
         }
     }
     else
@@ -3393,8 +3416,8 @@ private bool checkSafety(FuncDeclaration f, ref Loc loc, Scope* sc, Expressions*
         return false;
     if (sc.ctfe && sc.func)
         return false;
-    if (sc.inDefaultArg || sc.callLoc.isValid)
-        return false;
+
+    const bool useDeprecation = sc.inDefaultArg || sc.callLoc.isValid;
 
     if (!sc.func)
     {
@@ -3445,12 +3468,22 @@ private bool checkSafety(FuncDeclaration f, ref Loc loc, Scope* sc, Expressions*
 
     if (!f.isSafe() && !f.isTrusted())
     {
-        if (isRootTraitsCompilesScope(sc) ? sc.func.isSafeBypassingInference() : sc.func.setUnsafeCall(f))
+        const bool violated = isRootTraitsCompilesScope(sc) || useDeprecation
+            ? sc.func.isSafeBypassingInference()
+            : sc.func.setUnsafeCall(f);
+        if (violated)
         {
             if (!loc.isValid()) // e.g. implicitly generated dtor
                 loc = sc.func.loc;
 
             const prettyChars = f.toPrettyChars();
+            if (useDeprecation)
+            {
+                // Introduced in 2.113
+                .deprecation(loc, "`@safe` %s `%s` calling `@system` %s `%s` in default argument",
+                    sc.func.kind(), sc.func.toPrettyChars(), f.kind(), prettyChars);
+                return false;
+            }
             error(loc, "`@safe` %s `%s` cannot call `@system` %s `%s`",
                 sc.func.kind(), sc.func.toPrettyChars(), f.kind(),
                 prettyChars);
@@ -3496,8 +3529,7 @@ private bool checkNogc(FuncDeclaration f, ref Loc loc, Scope* sc)
         return false;
     if (sc.ctfe || sc.debug_)
         return false;
-    if (sc.inDefaultArg || sc.callLoc.isValid)
-        return false;
+    const bool useDeprecation = sc.inDefaultArg || sc.callLoc.isValid;
 
     /* The original expressions (`new S(...)` or `new S[...]``) will be
      * verified instead. This is to keep errors related to the original code
@@ -3509,12 +3541,23 @@ private bool checkNogc(FuncDeclaration f, ref Loc loc, Scope* sc)
     if (f.isNogc())
         return false;
 
-    if (isRootTraitsCompilesScope(sc) ? !sc.func.isNogcBypassingInference() : !sc.setGCCall(sc.func, f))
+    const bool violated = isRootTraitsCompilesScope(sc) || useDeprecation
+        ? sc.func.isNogcBypassingInference()
+        : sc.setGCCall(sc.func, f);
+    if (!violated)
         return false;
 
     if (loc == Loc.initial) // e.g. implicitly generated dtor
         loc = sc.func.loc;
 
+    if (useDeprecation)
+    {
+        // Introduced in 2.113
+        .deprecation(loc, "`@nogc` %s `%s` calling non-@nogc %s `%s` in default argument",
+            sc.func.kind(), sc.func.toPrettyChars(), f.kind(), f.toPrettyChars());
+        return false;
+    }
+
     // Lowered non-@nogc'd hooks will print their own error message inside of nogc.d (NOGCVisitor.visit(CallExp e)),
     // so don't print anything to avoid double error messages.
     if (!(f.ident == Id._d_HookTraceImpl || f.ident == Id._d_arraysetlengthT
@@ -7264,7 +7307,9 @@ private extern (C++) final class ExpressionSemanticVisitor : Visitor
             // https://issues.dlang.org/show_bug.cgi?id=12025
             // If the variable is not actually used in runtime code,
             // the purity violation error is redundant.
-            //checkPurity(sc, vd);
+            // However, check for violations in default arguments (emits deprecation).
+            if (sc.callLoc.isValid)
+                vd.checkPurity(e.loc, sc);
         }
         else if (fd)
         {

compiler/test/fail_compilation/test18670.d

@@ -0,0 +1,57 @@
+/*
+https://github.com/dlang/dmd/issues/18670
+Default arguments bypass most attributes check (pure, @safe, @nogc)
+
+TEST_OUTPUT:
+---
+fail_compilation/test18670.d(23): Deprecation: `pure` function `test18670.testPure` calling impure function `test18670.impure` in default argument
+fail_compilation/test18670.d(28): Deprecation: `pure` function `test18670.testPure2` accessing mutable static data `globalVar` in default argument
+fail_compilation/test18670.d(33): Deprecation: `@safe` function `test18670.testSafe` calling `@system` function `test18670.unsafe` in default argument
+fail_compilation/test18670.d(38): Error: indexing pointer `globalPtr` is not allowed in a `@safe` function
+fail_compilation/test18670.d(43): Deprecation: `@nogc` function `test18670.testNogc` calling non-@nogc function `test18670.gcAllocate` in default argument
+fail_compilation/test18670.d(47): Error: allocating with `new` causes a GC allocation in `@nogc` function `testNogc2`
+fail_compilation/test18670.d(53): Error: function `test18670.throwing` is not `nothrow`
+fail_compilation/test18670.d(53): Error: function `test18670.testNothrow` may throw but is marked as `nothrow`
+fail_compilation/test18670.d(56): Error: `object.Exception` is thrown but not caught
+fail_compilation/test18670.d(57): Error: function `test18670.testNothrow2` may throw but is marked as `nothrow`
+---
+*/
+
+
+// pure call
+int impure() => 0;
+void defaultImpure(int x = impure()) pure {}
+void testPure() pure => defaultImpure();
+
+// pure direct
+int globalVar = 7;
+void defaultImpure2(int x = globalVar) pure {}
+void testPure2() pure => defaultImpure2();
+
+// @safe call
+int unsafe() @system => 0;
+void defaultUnsafe(int x = unsafe()) @safe {}
+void testSafe() @safe => defaultUnsafe();
+
+// @safe direct
+int* globalPtr;
+void defaultUnsafe2(int x = globalPtr[1]) @safe {}
+void testSafe2() @safe => defaultUnsafe2();
+
+// @nogc call
+int gcAllocate() => *new int(3);
+void defaultGc(int x = gcAllocate()) @nogc {}
+void testNogc() @nogc => defaultGc();
+
+// @nogc direct
+void defaultGc2(int x = *new int(3)) @nogc {}
+void testNogc2() @nogc => defaultGc2();
+
+// nothrow call
+int throwing() => throw Exception.init;
+void defaultThrowing(int x = throwing()) nothrow {}
+void testNothrow() nothrow => defaultThrowing();
+
+// nothrow direct
+void defaultThrowing2(int x = throw new Exception("")) nothrow {}
+void testNothrow2() nothrow => defaultThrowing2();