www.digitalmars.com         C & C++   DMDScript  

digitalmars.D - Feature request: assert expressions should live inside version(assert)

reply "Tommi" <tommitissari hotmail.com> writes:
Sometimes you need to have some extra data to check against in 
the assert expression. That data isn't needed in release mode 
when assertions are ignored. Therefore, you put that extra data 
inside a version(assert). But then those assertions fail to 
compile in release mode because the symbol lookup for that extra 
data fails. For this reason, assert statements should live inside 
version(assert) blocks by default.

Example:

version (assert)
{
     const int[1000] maximums = 123;
}

void foo(int value, int index)
{
     assert(value < maximums[index]); // [1]
}

void main()
{
     foo(11, 22);
}

[1] (In release mode) Error: undefined identifier maximums

...so you need to introduce a redundant version(assert):

void foo(int value, int index)
{
     version (assert)
     {
         assert(value < maximums[index]);
     }
}
Jul 07 2013
parent reply Andrej Mitrovic <andrej.mitrovich gmail.com> writes:
On 7/7/13, Tommi <tommitissari hotmail.com> wrote:

 Sometimes you need to have some extra data to check against in
 the assert expression. That data isn't needed in release mode
 when assertions are ignored. Therefore, you put that extra data
 inside a version(assert). But then those assertions fail to
 compile in release mode because the symbol lookup for that extra
 data fails. For this reason, assert statements should live inside
 version(assert) blocks by default.


I've ran into an issue when implementing this feature back in February
(see the pull request):

http://d.puremagic.com/issues/show_bug.cgi?id=9450
https://github.com/D-Programming-Language/dmd/pull/1614
Jul 07 2013
next sibling parent "Peter Alexander" <peter.alexander.au gmail.com> writes:
On Sunday, 7 July 2013 at 12:30:28 UTC, Andrej Mitrovic wrote:

 I've ran into an issue when implementing this feature back in 
 February
 (see the pull request):

 http://d.puremagic.com/issues/show_bug.cgi?id=9450
 https://github.com/D-Programming-Language/dmd/pull/1614


WTF:


 Hmm.. I think we might have to close the report as invalid. The
 trouble is, you can't just ignore running semantic during 
 -release
 because the assert expression could be a compile-time false
 value, which turns the assert into a halt instruction 
 regardless of
 the -release switch.


I had no idea of this. What a terrible decision!
Jul 07 2013
prev sibling parent reply "Tommi" <tommitissari hotmail.com> writes:
On Sunday, 7 July 2013 at 12:30:28 UTC, Andrej Mitrovic wrote:

 On 7/7/13, Tommi <tommitissari hotmail.com> wrote:

 Sometimes you need to have some extra data to check against in
 the assert expression. That data isn't needed in release mode
 when assertions are ignored. Therefore, you put that extra data
 inside a version(assert). But then those assertions fail to
 compile in release mode because the symbol lookup for that 
 extra
 data fails. For this reason, assert statements should live 
 inside
 version(assert) blocks by default.


 I've ran into an issue when implementing this feature back in 
 February
 (see the pull request):

 http://d.puremagic.com/issues/show_bug.cgi?id=9450
 https://github.com/D-Programming-Language/dmd/pull/1614


Oh, should have searched bugzilla before posting this.

But would it be possible to implement it something like:
During a release build, even though version(assert) blocks are 
compiled out of existence, the compiler would keep a separate 
list of symbols (and other info) of the things declared inside 
those version(assert) blocks. Then, when parsing assert 
expressions, if an undefined symbol is found, the compiler would 
check that separate list of symbols that it has been keeping, and 
if the symbol is found there and use of the symbol is 
syntactically correct, the compiler would just keep on going 
instead of spewing an "unidentified identifier" error.

That way we'd make sure that things like:

assert(this_identifier_doesnt_exist < 12);

...wouldn't compile.
Jul 07 2013
next sibling parent "Tommi" <tommitissari hotmail.com> writes:
On Sunday, 7 July 2013 at 13:12:06 UTC, Tommi wrote:

 [..] Then, when parsing assert expressions, if an undefined 
 symbol is found, the compiler would check that separate list of 
 symbols that it has been keeping, and if the symbol is found 
 there and use of the symbol is syntactically correct, the 
 compiler would just keep on going instead of spewing an 
 "unidentified identifier" error.


One thing that might also help there, is the fact that once an 
unidentified identifier is encountered in an assert expression 
and that same identifier is found on that separate list of things 
we've been keeping, then we know that the current assert 
expression can only end up either being an "unidentified 
identifier" error or being compiled out of existence, but it 
can't end up being turned into an assert(0).
Jul 07 2013
prev sibling next sibling parent Denis Shelomovskij <verylonglogin.reg gmail.com> writes:
07.07.2013 17:12, Tommi пишет:

 On Sunday, 7 July 2013 at 12:30:28 UTC, Andrej Mitrovic wrote:

 On 7/7/13, Tommi <tommitissari hotmail.com> wrote:

 Sometimes you need to have some extra data to check against in
 the assert expression. That data isn't needed in release mode
 when assertions are ignored. Therefore, you put that extra data
 inside a version(assert). But then those assertions fail to
 compile in release mode because the symbol lookup for that extra
 data fails. For this reason, assert statements should live inside
 version(assert) blocks by default.


 I've ran into an issue when implementing this feature back in February
 (see the pull request):

 http://d.puremagic.com/issues/show_bug.cgi?id=9450
 https://github.com/D-Programming-Language/dmd/pull/1614


 Oh, should have searched bugzilla before posting this.

 But would it be possible to implement it something like:
 During a release build, even though version(assert) blocks are compiled
 out of existence, the compiler would keep a separate list of symbols
 (and other info) of the things declared inside those version(assert)
 blocks. Then, when parsing assert expressions, if an undefined symbol is
 found, the compiler would check that separate list of symbols that it
 has been keeping, and if the symbol is found there and use of the symbol
 is syntactically correct, the compiler would just keep on going instead
 of spewing an "unidentified identifier" error.

 That way we'd make sure that things like:

 assert(this_identifier_doesnt_exist < 12);

 ...wouldn't compile.


It's always a bad idea to add special cases like this one for 
`version(assert)`. Anyway one will get into huge troubles trying to 
implement such stuff as I even can't imagine how it can be implemented 
consistently.

-- 
Денис В. Шеломовский
Denis V. Shelomovskij
Jul 07 2013
prev sibling parent reply "Tommi" <tommitissari hotmail.com> writes:
On Sunday, 7 July 2013 at 13:12:06 UTC, Tommi wrote:

 But would it be possible to implement it something like:
 [..]


Although, I don't know if the best possible behaviour is to 
silently compile the following assert out of existence in release 
mode:

version (assert)
{
     enum cond = false;
}

void main()
{
     assert(cond);
}

A better behaviour probably would be to spew a warning message 
asking the user if he's sure he knows what he's doing.

Then, yet another solution (a code breaking one) would be to make 
it so that only literally saying:
assert(0);
or
assert(false);
or
assert(null);

...would exhibit that special assert behaviour.

Anything else would be semantically runtime-evaluated and no 
other forms of assert would remain in release builds. For 
example, this kind of an assert would be compiled out of 
existence in release mode:

enum bool cond = false;
assert(cond);
Jul 07 2013
parent "Tommi" <tommitissari hotmail.com> writes:
On Sunday, 7 July 2013 at 13:42:52 UTC, Tommi wrote:

 Then, yet another solution (a code breaking one) would be to 
 make it so that only literally saying:
 assert(0);
 or
 assert(false);
 or
 assert(null);

 ...would exhibit that special assert behaviour.

 Anything else would be semantically runtime-evaluated and no 
 other forms of assert would remain in release builds. For 
 example, this kind of an assert would be compiled out of 
 existence in release mode:

 enum bool cond = false;
 assert(cond);


The programmer can always work around this new behaviour.

Old code:

enum bool shouldHalt = someExpression();
assert(shouldHalt);

New code:

enum bool shouldHalt = someExpression();
static if (shouldHalt) assert(0);
Jul 07 2013