-
-
Notifications
You must be signed in to change notification settings - Fork 716
Implement DIP1006 - more selective control over run-time checks #7980
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,4 @@ | ||
| Allow to disable only specific run-time checks | ||
|
|
||
| The $(LINK2 $(TT -release), https://dlang.org/dmd.html#switch-release) switch now accepts optional arguments to only disable some run-time checks, e.g. $(TT -release=in,out,invariant) disables in-/out-contracts and invariants, but still leaves assertions enabled. | ||
| Also see $(LINK2 DIP1006, https://github.com/dlang/DIPs/blob/master/DIPs/DIP1006.md). |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -396,37 +396,23 @@ private int tryMain(size_t argc, const(char)** argv) | |
| global.params.mscrtlib = vsopt.defaultRuntimeLibrary(global.params.is64bit); | ||
| } | ||
| } | ||
| if (global.params.release) | ||
| { | ||
| global.params.useInvariants = false; | ||
| global.params.useIn = false; | ||
| global.params.useOut = false; | ||
|
|
||
| if (global.params.useArrayBounds == CHECKENABLE._default) | ||
| global.params.useArrayBounds = CHECKENABLE.safeonly; | ||
|
|
||
| if (global.params.useAssert == CHECKENABLE._default) | ||
| global.params.useAssert = CHECKENABLE.off; | ||
|
|
||
| if (global.params.useSwitchError == CHECKENABLE._default) | ||
| global.params.useSwitchError = CHECKENABLE.off; | ||
| } | ||
| if (global.params.betterC) | ||
| { | ||
| global.params.checkAction = CHECKACTION.C; | ||
| global.params.useModuleInfo = false; | ||
| global.params.useTypeInfo = false; | ||
| global.params.useExceptions = false; | ||
| } | ||
| // -release -unittest enables assertions in unittests AND function bodies | ||
| if (global.params.useUnitTests) | ||
| global.params.useAssert = CHECKENABLE.on; | ||
|
|
||
| if (global.params.useArrayBounds == CHECKENABLE._default) | ||
| global.params.useArrayBounds = CHECKENABLE.on; | ||
|
|
||
| if (global.params.useAssert == CHECKENABLE._default) | ||
| global.params.useAssert = CHECKENABLE.on; | ||
|
|
||
| if (global.params.useArrayBounds == CHECKENABLE._default) | ||
| global.params.useArrayBounds = CHECKENABLE.on; | ||
|
|
||
| if (global.params.useSwitchError == CHECKENABLE._default) | ||
| global.params.useSwitchError = CHECKENABLE.on; | ||
|
|
||
|
|
@@ -1503,9 +1489,9 @@ private bool parseCommandLine(const ref Strings arguments, const size_t argc, re | |
| { | ||
| bool errors; | ||
|
|
||
| void error(const(char)* format, const(char*) arg = null) | ||
| void error(Args...)(const(char)* format, Args args) | ||
| { | ||
| dmd.errors.error(Loc.initial, format, arg); | ||
| dmd.errors.error(Loc.initial, format, args); | ||
| errors = true; | ||
| } | ||
|
|
||
|
|
@@ -2005,8 +1991,66 @@ private bool parseCommandLine(const ref Strings arguments, const size_t argc, re | |
| { | ||
| // Ignore | ||
| } | ||
| else if (arg == "-release") // https://dlang.org/dmd.html#switch-release | ||
| params.release = true; | ||
| else if (startsWith(p + 1, "release")) // https://dlang.org/dmd.html#switch-release | ||
| { | ||
| // Parse: | ||
| // -release[=assert[,in,out,invariant]] | ||
| if (p["-release".length] == '=') | ||
| { | ||
| if (arg.length == "-release=".length) | ||
| goto Lnoarg; | ||
|
|
||
| auto tail = arg["-release=".length .. $]; | ||
| while (true) | ||
| { | ||
| auto delim = strchr(tail.ptr, ','); | ||
| auto check = tail[0 .. delim ? delim - tail.ptr : $]; | ||
| switch (check) | ||
| { | ||
| case "assert": | ||
| // can be overridden by -unittest | ||
| if (params.useAssert == CHECKENABLE._default) | ||
| params.useAssert = CHECKENABLE.off; | ||
| break; | ||
| case "in": | ||
| params.useIn = false; | ||
| break; | ||
| case "out": | ||
| params.useOut = false; | ||
| break; | ||
| case "invariant": | ||
| params.useInvariants = false; | ||
| break; | ||
| default: | ||
| error("unrecognized argument '%.*s' for -release=", cast(int)check.length, check.ptr); | ||
| break; | ||
| } | ||
| if (check.length == tail.length) | ||
| break; | ||
| tail = tail[check.length + ",".length .. $]; | ||
| } | ||
| } | ||
| else if (arg.length > "-release".length) | ||
| goto Lerror; | ||
| else // -release | ||
| { | ||
| if (params.useAssert == CHECKENABLE._default) | ||
| params.useAssert = CHECKENABLE.off; | ||
| params.useIn = false; | ||
| params.useOut = false; | ||
| params.useInvariants = false; | ||
| } | ||
|
|
||
| // for additional linker flags | ||
| params.optimizeLinking = true; | ||
|
|
||
| if (params.useArrayBounds == CHECKENABLE._default) | ||
| params.useArrayBounds = CHECKENABLE.safeonly; | ||
|
|
||
| // switch without default is now a compile-time error anyhow | ||
| if (params.useSwitchError == CHECKENABLE._default) | ||
| params.useSwitchError = CHECKENABLE.off; | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do I understand correctly that those 2 get affected even if I just provide
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Switch fallthrough errors are dead, they're a compile-time error nowadays. |
||
| } | ||
| else if (arg == "-betterC") // https://dlang.org/dmd.html#switch-betterC | ||
| params.betterC = true; | ||
| else if (arg == "-noboundscheck") // https://dlang.org/dmd.html#switch-noboundscheck | ||
|
|
@@ -2245,6 +2289,53 @@ private bool parseCommandLine(const ref Strings arguments, const size_t argc, re | |
| return errors; | ||
| } | ||
|
|
||
| unittest | ||
| { | ||
| global.gag = 1; | ||
| immutable errorsave = global.errors; | ||
| scope (exit) { global.gag = 0; global.errors = errorsave; } | ||
|
|
||
| static bool test(const(char*)[] args, out Param params, out Strings files) | ||
| { | ||
| Strings sargs; | ||
| sargs.setDim(args.length + 1); | ||
| sargs[0] = "dmd"; | ||
| foreach (i, ref sarg; sargs[1 .. sargs.dim]) | ||
| sarg = args[i]; | ||
| return !parseCommandLine(sargs, sargs.dim, params, files); | ||
| } | ||
|
|
||
| Param params; | ||
| Strings files; | ||
| assert(test(["-release"], params, files)); | ||
| assert(params.optimizeLinking); | ||
| assert(params.useAssert == CHECKENABLE.off && !params.useIn && !params.useOut && !params.useInvariants); | ||
| assert(params.useArrayBounds == CHECKENABLE.safeonly && params.useSwitchError == CHECKENABLE.off); | ||
| assert(test(["-release=assert,in,out,invariant"], params, files)); | ||
| assert(params.optimizeLinking); | ||
| assert(params.useAssert == CHECKENABLE.off && !params.useIn && !params.useOut && !params.useInvariants); | ||
| assert(params.useArrayBounds == CHECKENABLE.safeonly && params.useSwitchError == CHECKENABLE.off); | ||
| assert(test(["-release=in"], params, files)); | ||
| assert(params.optimizeLinking); | ||
| assert(params.useAssert == CHECKENABLE._default); | ||
| assert(!params.useIn && params.useOut && params.useInvariants); | ||
| assert(params.useArrayBounds == CHECKENABLE.safeonly && params.useSwitchError == CHECKENABLE.off); | ||
| assert(test(["-release=in,out"], params, files)); | ||
| assert(params.optimizeLinking); | ||
| assert(params.useAssert == CHECKENABLE._default && !params.useIn && !params.useOut && params.useInvariants); | ||
| assert(params.useArrayBounds == CHECKENABLE.safeonly && params.useSwitchError == CHECKENABLE.off); | ||
| assert(test(["-release=in,out,assert,invariant"], params, files)); | ||
| assert(params.optimizeLinking); | ||
| assert(params.useAssert == CHECKENABLE.off && !params.useIn && !params.useOut && !params.useInvariants); | ||
| assert(params.useArrayBounds == CHECKENABLE.safeonly && params.useSwitchError == CHECKENABLE.off); | ||
|
|
||
| assert(!test(["-release="], params, files)); | ||
| assert(!test(["-release=in,"], params, files)); | ||
| assert(!test(["-release=foo"], params, files)); | ||
| assert(!test(["-release=assert,in,put"], params, files)); | ||
| assert(!test(["-release=assert;in"], params, files)); | ||
| assert(!test(["-release:in,out"], params, files)); | ||
| } | ||
|
|
||
| private __gshared bool includeImports = false; | ||
| // array of module patterns used to include/exclude imported modules | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,38 @@ | ||
| // REQUIRED_ARGS: -release=in,out,invariant | ||
| // PERMUTE_ARGS: | ||
| class C | ||
| { | ||
| int foo(int a) | ||
| in { assert(a != 0); } // skipped | ||
| out(res) { assert(res != 0); } // skipped | ||
| body | ||
| { | ||
| return a; | ||
| } | ||
|
|
||
| invariant // skipped | ||
| { | ||
| assert(false); | ||
| } | ||
|
|
||
| void bar(int a) | ||
| { | ||
| assert(a != 0); // triggered | ||
| } | ||
| } | ||
|
|
||
| void main() | ||
| { | ||
| import core.exception : AssertError; | ||
|
|
||
| auto c = new C; | ||
| c.foo(0); | ||
|
|
||
| bool catched; | ||
| try | ||
| c.bar(0); | ||
| catch (AssertError e) | ||
| catched = true; | ||
| if (!catched) | ||
| assert(0); | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,32 @@ | ||
| // REQUIRED_ARGS: -release=in,invariant | ||
| // PERMUTE_ARGS: | ||
| class C | ||
| { | ||
| int foo(int a) | ||
| in { assert(a != 0); } // skipped | ||
| out(res) { assert(res != 0, "out"); } // triggered | ||
| body | ||
| { | ||
| return a; | ||
| } | ||
|
|
||
| invariant // skipped | ||
| { | ||
| assert(false); | ||
| } | ||
| } | ||
|
|
||
| void main() | ||
| { | ||
| import core.exception : AssertError; | ||
|
|
||
| auto c = new C; | ||
| bool catched; | ||
| try | ||
| c.foo(0); | ||
| catch (AssertError e) | ||
| catched = e.msg == "out"; | ||
|
|
||
| if (!catched) | ||
| assert(0); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought we were not supporting CLI with comma ? After #7863
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not familiar with the approved CLI interface for DIP1006, whether commas were approved. If commas were approved then we should establish a convention on when and when not to support commas, and then use a common implementation for comma support. If it were approved, I would think it would also apply to options like
-iand-version...but this is all moot if commas are not approvedThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I picked commas because it allows to override previous occurences of that argument, sometimes useful when using other peoples projects and Makefiles. Less of an argument for switches like include paths that are expected to be additive.