Skip to content

add -check command line switch #8972

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.

Sign up for GitHub

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

Merged
andralex merged 1 commit into from
Dec 10, 2018
Merged

add -check command line switch #8972

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 18 additions & 0 deletions changelog/check-switch.dd
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
Added -check switch to turn on and off each category of runtime checks.

Option("check=[assert|bounds|in|invariant|out|switch][=[on|off]]",
`Overrides default, -boundscheck, -release and -unittest options to enable or disable specific checks.
$(UL
$(LI $(B assert): assertion checking)
$(LI $(B bounds): array bounds)
$(LI $(B in): in contracts)
$(LI $(B invariant): class/struct invariants)
$(LI $(B out): out contracts)
$(LI $(B switch): switch default)
)
$(UL
$(LI $(B on) or not specified: specified check is enabled.)
$(LI $(B off): specified check is disabled.)
)`
)

22 changes: 19 additions & 3 deletions src/dmd/cli.d
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -169,14 +169,30 @@ struct Usage
Option("c",
"compile only, do not link"
),
Option("check=[assert|bounds|in|invariant|out|switch][=[on|off]]",
Copy link
Member

@rainers rainers Nov 19, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't want to start bikeshedding, but the duplicate = looks strange. Why not use check-[assert|...]?

Copy link
Member Author

@WalterBright WalterBright Nov 19, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because 1) we use on|off elsewhere, not +|- 2) I want to add a safe option as well 3) other options may become useful than just on|off|safe

Copy link
Member

@rainers rainers Nov 19, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't mean to remove the on/off, but to replace the first = with -, e.g. -check-assert=on instead of -check=assert=on.

Copy link
Member Author

@WalterBright WalterBright Nov 20, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We already use = for the mv switch.

Copy link

@joseph-wakeling-sociomantic joseph-wakeling-sociomantic Nov 20, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another syntax question: is it assumed that the user will write separate -check=assert -check=in -check=out -check=bounds calls, or do we want to support a syntax like -check=assert,bounds,in,out that allows all the options to be specified in a single flag?

Copy link
Member Author

@WalterBright WalterBright Nov 21, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps in the future. For right now, I want to maximize simplicity.

"Enable or disable specific checks",
`Overrides default, -boundscheck, -release and -unittest options to enable or disable specific checks.
$(UL
$(LI $(B assert): assertion checking)
$(LI $(B bounds): array bounds)
$(LI $(B in): in contracts)
$(LI $(B invariant): class/struct invariants)
$(LI $(B out): out contracts)
$(LI $(B switch): switch default)
)
$(UL
$(LI $(B on) or not specified: specified check is enabled.)
$(LI $(B off): specified check is disabled.)
)`
),
Option("checkaction=D|C|halt",
"behavior on assert/boundscheck/finalswitch failure",
`Sets behavior when an assert fails, and array boundscheck fails,
or a final switch errors.
$(UL
$(LI $(I D): Default behavior, which throws an unrecoverable $(D Error).)
$(LI $(I C): Calls the C runtime library assert failure function.)
$(LI $(I halt): Executes a halt instruction, terminating the program.)
$(LI $(B D): Default behavior, which throws an unrecoverable $(D Error).)
$(LI $(B C): Calls the C runtime library assert failure function.)
$(LI $(B halt): Executes a halt instruction, terminating the program.)
)`
),
Option("color",
Expand Down
1 change: 1 addition & 0 deletions src/dmd/globals.d
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,7 @@ struct Param
CHECKENABLE useArrayBounds = CHECKENABLE._default; // when to generate code for array bounds checks
CHECKENABLE useAssert = CHECKENABLE._default; // when to generate code for assert()'s
CHECKENABLE useSwitchError = CHECKENABLE._default; // check for switches without a default
CHECKENABLE boundscheck = CHECKENABLE._default; // state of -boundscheck switch
Copy link
Member

@rainers rainers Nov 22, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While this now works as documented, I think it's a bit confusing to have both useArrayBounds and boundscheck as fields of Param. I would expect boundscheck to be a local variable in parseCommandLine().


CHECKACTION checkAction = CHECKACTION.D; // action to take when bounds, asserts or switch defaults are violated

Expand Down
1 change: 1 addition & 0 deletions src/dmd/globals.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,7 @@ struct Param
CHECKENABLE useArrayBounds; // when to generate code for array bounds checks
CHECKENABLE useAssert; // when to generate code for assert()'s
CHECKENABLE useSwitchError; // check for switches without a default
CHECKENABLE boundscheck; // state of -boundscheck switch

CHECKACTION checkAction; // action to take when bounds, asserts or switch defaults are violated

Expand Down
60 changes: 53 additions & 7 deletions src/dmd/mars.d
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -335,6 +335,19 @@ private int tryMain(size_t argc, const(char)** argv)
global.params.mscrtlib = vsopt.defaultRuntimeLibrary(global.params.is64bit);
}
}

if (global.params.boundscheck != CHECKENABLE._default)
{
if (global.params.useArrayBounds == CHECKENABLE._default)
global.params.useArrayBounds = global.params.boundscheck;
}

if (global.params.useUnitTests)
{
if (global.params.useAssert == CHECKENABLE._default)
global.params.useAssert = CHECKENABLE.on;
}

if (global.params.release)
{
if (global.params.useInvariants == CHECKENABLE._default)
Expand Down Expand Up @@ -376,9 +389,6 @@ private int tryMain(size_t argc, const(char)** argv)
global.params.useSwitchError = CHECKENABLE.on;
}

if (global.params.useUnitTests)
global.params.useAssert = CHECKENABLE.on;

if (global.params.betterC)
{
global.params.checkAction = CHECKACTION.C;
Expand Down Expand Up @@ -1499,6 +1509,42 @@ bool parseCommandLine(const ref Strings arguments, const size_t argc, ref Param
params.useDeprecated = Diagnostic.inform;
else if (arg == "-c") // https://dlang.org/dmd.html#switch-c
params.link = false;
else if (startsWith(p + 1, "check=")) // https://dlang.org/dmd.html#switch-check
{
/* Parse:
* -check=[assert|bounds|in|invariant|out|switch][=[on|off]]
*/

// Check for legal option string; return true if so
static bool check(const(char)* p, string name, ref CHECKENABLE ce)
Copy link
Member

@UplinkCoder UplinkCoder Nov 20, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems awfully rigid, re checking for exactly =on or =off
For a first implementation it might be alight though.

{
p += "-check=".length;
if (startsWith(p, name))
{
p += name.length;
if (*p == 0 ||
strcmp(p, "=on") == 0)
{
ce = CHECKENABLE.on;
return true;
}
else if (strcmp(p, "=off") == 0)
{
ce = CHECKENABLE.off;
return true;
}
}
return false;
}

if (!(check(p, "assert", params.useAssert ) ||
check(p, "bounds", params.useArrayBounds) ||
check(p, "in", params.useIn ) ||
check(p, "invariant", params.useInvariants ) ||
check(p, "out", params.useOut ) ||
check(p, "switch", params.useSwitchError)))
goto Lerror;
}
else if (startsWith(p + 1, "checkaction=")) // https://dlang.org/dmd.html#switch-checkaction
{
/* Parse:
Expand Down Expand Up @@ -1956,7 +2002,7 @@ bool parseCommandLine(const ref Strings arguments, const size_t argc, ref Param
params.betterC = true;
else if (arg == "-noboundscheck") // https://dlang.org/dmd.html#switch-noboundscheck
{
params.useArrayBounds = CHECKENABLE.off;
params.boundscheck = CHECKENABLE.off;
}
else if (startsWith(p + 1, "boundscheck")) // https://dlang.org/dmd.html#switch-boundscheck
{
Expand All @@ -1966,15 +2012,15 @@ bool parseCommandLine(const ref Strings arguments, const size_t argc, ref Param
{
if (strcmp(p + 13, "on") == 0)
{
params.useArrayBounds = CHECKENABLE.on;
params.boundscheck = CHECKENABLE.on;
}
else if (strcmp(p + 13, "safeonly") == 0)
{
params.useArrayBounds = CHECKENABLE.safeonly;
params.boundscheck = CHECKENABLE.safeonly;
}
else if (strcmp(p + 13, "off") == 0)
{
params.useArrayBounds = CHECKENABLE.off;
params.boundscheck = CHECKENABLE.off;
}
else
goto Lerror;
Expand Down