diff --git a/changelog/dmd.default-arg-attributes.dd b/changelog/dmd.default-arg-attributes.dd new file mode 100644 index 000000000000..2e6ede6036c4 --- /dev/null +++ b/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 +) diff --git a/compiler/src/dmd/expressionsem.d b/compiler/src/dmd/expressionsem.d index da1d253d3fbd..7919ca58efeb 100644 --- a/compiler/src/dmd/expressionsem.d +++ b/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) { diff --git a/compiler/test/fail_compilation/test18670.d b/compiler/test/fail_compilation/test18670.d new file mode 100644 index 000000000000..996c294b8512 --- /dev/null +++ b/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();