www.digitalmars.com         C & C++   DMDScript  

digitalmars.D - A code example that shows why I don't like warnings

reply Adam D. Ruppe <destructionator gmail.com> writes:
Consider this code:

-----

string defaultFormat(alias method)() {
         static foreach(attr; __traits(getAttributes, method))
                 static if(is(typeof(attr) == string))
                         return attr;
         return "default";
}

void test() {}
 ("test") void test2() {}

pragma(msg, defaultFormat!test);
pragma(msg, defaultFormat!test2);

void main() {}

-----


If you compile that with warnings enabled, you will get

Warning: statement is not reachable

on test, but on test2, that very *same code* it complains about 
is executed. If you remove it, it errors on the other test2 
condition!



I know it is possible to refactor the attribute thing into a 
function (like phobos' getAttribute), but nevertheless, this kind 
of early return in a loop is a common enough pattern that I don't 
think the compiler should warn on.

I almost put this in the "Learn" forum asking for techniques, but 
really, I know how to refactor it already. I am more complaining 
about the compiler here :)

basically not a lot to say, just wanna make sure code like this 
is on the dev's radar if they ever revisit those warnings.
Oct 18
next sibling parent reply FeepingCreature <feepingcreature gmail.com> writes:
On Friday, 18 October 2019 at 16:38:38 UTC, Adam D. Ruppe wrote:
 Consider this code:

 -----

 string defaultFormat(alias method)() {
         static foreach(attr; __traits(getAttributes, method))
                 static if(is(typeof(attr) == string))
                         return attr;
         return "default";
 }

 void test() {}
  ("test") void test2() {}

 pragma(msg, defaultFormat!test);
 pragma(msg, defaultFormat!test2);

 void main() {}

 -----


 If you compile that with warnings enabled, you will get

 Warning: statement is not reachable
Behold, the shitty workaround: string defaultFormat(alias method)() { alias attributes = __traits(getAttributes, method); static if (attributes.length == 0) return "default"; else static foreach(i, attr; attributes) static if(is(typeof(attr) == string)) return attr; else static if (i == __traits(getAttributes, method).length - 1) return "default"; } This does **not** spark joy. But it works.
Oct 18
parent reply Adam D. Ruppe <destructionator gmail.com> writes:
On Friday, 18 October 2019 at 16:43:54 UTC, FeepingCreature wrote:
 This does **not** spark joy. But it works.
Yes, I just put in a runtime condition to trick the compiler: bool isScriptable(attributes...)() { bool nonConstConditionForWorkingAroundASpuriousDmdWarning = true; foreach(attribute; attributes) { static if(is(typeof(attribute) == string)) { static if(attribute == scriptable) { if(nonConstConditionForWorkingAroundASpuriousDmdWarning) return true; } } } return false; } the optimizer can prolly remove that condition anyway but it is good enough to silence the warning. still just all the work arounds are kinda gross.
Oct 18
parent reply Alexandru Ermicioi <alexandru.ermicioi gmail.com> writes:
On Friday, 18 October 2019 at 16:46:36 UTC, Adam D. Ruppe wrote:
 On Friday, 18 October 2019 at 16:43:54 UTC, FeepingCreature 
 wrote:
 This does **not** spark joy. But it works.
Yes, I just put in a runtime condition to trick the compiler: bool isScriptable(attributes...)() { bool nonConstConditionForWorkingAroundASpuriousDmdWarning = true; foreach(attribute; attributes) { static if(is(typeof(attribute) == string)) { static if(attribute == scriptable) { if(nonConstConditionForWorkingAroundASpuriousDmdWarning) return true; } } } return false; } the optimizer can prolly remove that condition anyway but it is good enough to silence the warning. still just all the work arounds are kinda gross.
You can define a bool enum in the body of static if (inside foreach) and then wrap default return statement with static if that checks if that enum was defined. If it wasn't then return "default".
Oct 18
parent reply Adam D. Ruppe <destructionator gmail.com> writes:
On Friday, 18 October 2019 at 18:19:55 UTC, Alexandru Ermicioi 
wrote:
 You can define a bool enum in the body of static if (inside 
 foreach)  and then wrap default return statement with static if 
 that checks if that enum was defined. If it wasn't then return 
 "default".
oh that's not horrible and it works too. But still I don't love it :) BTW as of today my whole lib will compile with warnings including when exercising the more complicated templates!
Oct 18
parent reply Alexandru Ermicioi <alexandru.ermicioi gmail.com> writes:
On Friday, 18 October 2019 at 18:22:33 UTC, Adam D. Ruppe wrote:
 On Friday, 18 October 2019 at 18:19:55 UTC, Alexandru Ermicioi 
 wrote:
 You can define a bool enum in the body of static if (inside 
 foreach)  and then wrap default return statement with static 
 if that checks if that enum was defined. If it wasn't then 
 return "default".
oh that's not horrible and it works too. But still I don't love it :) BTW as of today my whole lib will compile with warnings including when exercising the more complicated templates!
Yeah it is not ideal. This trick can also shield you in case if multiple elements from static foreach matches inner static if, just by adding a check to it if this enum was defined earlier. Also beware of code blocks for foreach or inner static if, since it will limit enum declaration to that block and wont be available to static if defined outside of foreach (the one for default return).
Oct 19
parent Daniel N <no public.email> writes:
On Saturday, 19 October 2019 at 16:14:20 UTC, Alexandru Ermicioi 
wrote:
 On Friday, 18 October 2019 at 18:22:33 UTC, Adam D. Ruppe wrote:
 On Friday, 18 October 2019 at 18:19:55 UTC, Alexandru Ermicioi 
 wrote:
 You can define a bool enum in the body of static if (inside 
 foreach)  and then wrap default return statement with static 
 if that checks if that enum was defined. If it wasn't then 
 return "default".
oh that's not horrible and it works too. But still I don't love it :) BTW as of today my whole lib will compile with warnings including when exercising the more complicated templates!
Yeah it is not ideal. This trick can also shield you in case if multiple elements from static foreach matches inner static if, just by adding a check to it if this enum was defined earlier. Also beware of code blocks for foreach or inner static if, since it will limit enum declaration to that block and wont be available to static if defined outside of foreach (the one for default return).
template defaultFormat(alias method) { static foreach(attr; __traits(getAttributes, method)) static if(is(typeof(attr) == string)) enum defaultFormat = attr; static if(is(typeof(defaultFormat) == void)) enum defaultFormat = "default"; }
Oct 20
prev sibling next sibling parent reply Ethan <gooberman gmail.com> writes:
On Friday, 18 October 2019 at 16:38:38 UTC, Adam D. Ruppe wrote:
 Warning: statement is not reachable
This is also another valid argument for error/warning codes. This exact thing happens *all the time* in my code, and it results in me having to write my static ifs and CTFE functions in a roundabout way just to not get the warning. Whereas if I had the warning code, I could tell the compiler to suppress that warning. Exactly like I do in C++.
Oct 20
parent Johan <j j.nl> writes:
On Sunday, 20 October 2019 at 12:53:22 UTC, Ethan wrote:
 On Friday, 18 October 2019 at 16:38:38 UTC, Adam D. Ruppe wrote:
 Warning: statement is not reachable
This is also another valid argument for error/warning codes. This exact thing happens *all the time* in my code, and it results in me having to write my static ifs and CTFE functions in a roundabout way just to not get the warning. Whereas if I had the warning code, I could tell the compiler to suppress that warning. Exactly like I do in C++.
I once submitted a PR that enabled that functionality (being able to silence specific warnings, and the warning itself would mention its category), but it was refused. I haven't pursued it any further, but am still considering adding it to the better compiler for professional use (LDC ofc ;). -Johan [1] https://github.com/dlang/dmd/pull/5592
Oct 20
prev sibling parent reply Walter Bright <newshound2 digitalmars.com> writes:
On 10/18/2019 9:38 AM, Adam D. Ruppe wrote:
 basically not a lot to say, just wanna make sure code like this is on the
dev's 
 radar if they ever revisit those warnings.
Nothing gets on the radar if you don't put it in bugzilla !
Oct 21
next sibling parent tsbockman <thomas.bockman gmail.com> writes:
On Monday, 21 October 2019 at 07:08:23 UTC, Walter Bright wrote:
 On 10/18/2019 9:38 AM, Adam D. Ruppe wrote:
 basically not a lot to say, just wanna make sure code like 
 this is on the dev's radar if they ever revisit those warnings.
Nothing gets on the radar if you don't put it in bugzilla !
This specific problem has been on Bugzilla for ages: https://issues.dlang.org/show_bug.cgi?id=14835 . It went nowhere because no one could figure out a reasonable way to make the compiler apply the warning *intelligently* in generic code, and there was no consensus to remove the current broken version, because it is still useful in non-generic code. This issue also killed efforts to improve various aspects of the compiler's handling of integer comparisons (twice): https://github.com/dlang/dmd/pull/5229
Oct 21
prev sibling parent tsbockman <thomas.bockman gmail.com> writes:
On Monday, 21 October 2019 at 07:08:23 UTC, Walter Bright wrote:
 On 10/18/2019 9:38 AM, Adam D. Ruppe wrote:
 basically not a lot to say, just wanna make sure code like 
 this is on the dev's radar if they ever revisit those warnings.
Nothing gets on the radar if you don't put it in bugzilla !
Previous, inconclusive forum discussion on why/how/if to fix issue 14385: https://forum.dlang.org/post/bnzfuekewfguvnwdznfk forum.dlang.org Walter Bright promising to look into it personally (I never heard back on this...): https://github.com/dlang/dmd/pull/5229#issuecomment-226634471
Oct 21