www.digitalmars.com         C & C++   DMDScript  

digitalmars.D - Spurious compiler warning ?

reply knommad <spam_D asylum.id.au> writes:
Hi,

I'm a relative newcomer to D, and am enjoying the experience.

I have a small testcase here that causes what seems to be a spurious warning 
from the compiler (resulting code still works, however). 

Am I missing something??

import std.stdio;
import std.traits;

enum TestEnum 
{
    ONE = 1,
    TWO = 2, 
    THREE = 3, 
    FOUR = 4
}

void main()
{
    // Generates warning - works as expected though
    foreach( TestEnum member; EnumMembers!TestEnum )
    {
        if ( member == TestEnum.TWO ) 
            continue;
        writeln( "A: ", member );
    }

    // No warning generated
    foreach( TestEnum member; EnumMembers!TestEnum )
    {
        if ( member == TestEnum.TWO ) 
            continue;
        else
            writeln( "B: ", member );
    }

    // No warning generated
    auto list = EnumMembers!TestEnum;
    foreach( TestEnum member; list )
    {
        if ( member == TestEnum.TWO ) 
            continue;
        writeln( "C: ", member );
    }

}

compiled with: dmd -wi -oftest test.d

(dmd version 2.056)

output is as expected:
A: ONE
A: THREE
A: FOUR
B: ONE
B: THREE
B: FOUR
C: ONE
C: THREE
C: FOUR


regards,
ted

-- 
Money can't buy happiness. But it can buy marshmallows, which are kinda the 
same thing.
Nov 03 2011
next sibling parent reply Don <nospam nospam.com> writes:
On 03.11.2011 08:47, knommad wrote:

 Hi,

 I'm a relative newcomer to D, and am enjoying the experience.

 I have a small testcase here that causes what seems to be a spurious warning
 from the compiler (resulting code still works, however).

 Am I missing something??



It's an interesting situation that you've found. The foreach is a static 
foreach, so it gets unrolled at compile time. In each unrolled bit, 
'member' is a compile-time constant, so the if statement becomes either 
true or false. The code is in fact unreachable in that iteration.
So the warning is correct, but arguably not helpful.

Maybe we should suppress unreachable code warnings inside unrolled code.
Nov 03 2011
next sibling parent reply kenji hara <k.hara.pg gmail.com> writes:
2011/11/3 Nick Sabalausky <a a.a>:

 I don't know much about the internals of DMD, but would this make any
 sense?:

 When unrolling code, keep track of where the unrolled parts came from. Only
 issue an unreachable code warning if *all* the unrolled versions of the
 original line X are unreachable.



First, when we use -w option, we usually want to show *all* warnings of codes.
Isn't it your expects?

Second, each unrolled loop bodies is separately analyzable by the compiler.
There is no reason to drop the detectable information, unless other
factors affect (such as performance).

I don't know why you want to reduce warnings that (probably) you expects.

Kenji Hara
Nov 03 2011
parent "Nick Sabalausky" <a a.a> writes:
"kenji hara" <k.hara.pg gmail.com> wrote in message 
news:mailman.655.1320322897.24802.digitalmars-d puremagic.com...

 2011/11/3 Nick Sabalausky <a a.a>:

 I don't know much about the internals of DMD, but would this make any
 sense?:

 When unrolling code, keep track of where the unrolled parts came from. 
 Only
 issue an unreachable code warning if *all* the unrolled versions of the
 original line X are unreachable.



 First, when we use -w option, we usually want to show *all* warnings of 
 codes.
 Isn't it your expects?



All real warnings, yes, but this one is just simply incorrect. The line *in 
the source code* that says "writeln( "A: ", member );"  *does*, in fact, get 
reached. Just because it doesn't get reached in *every* iteration doesn't 
change that fact that it *does* get reached and is therefore *not* 
unreachable code. The fact that the compiler unrolls it behind-the-scenes is 
irrelevent to the user - that merely explains why the warning is *currently* 
thrown, not why it *should* be thrown.


 Second, each unrolled loop bodies is separately analyzable by the 
 compiler.
 There is no reason to drop the detectable information, unless other
 factors affect (such as performance).

 I don't know why you want to reduce warnings that (probably) you expects.



I don't see why anyone would ever expect this warning at all. *All* lines 
inside the user's foreach body *do* get used. It's the *compiler's* internal 
processing that *adds* in some dead code behind-the-scenes. The source 
doesn't have any dead code. It's just like inlining:

int foo(int x)
{
    if(x > 5)
        return 5;
    return 1;
}

void main()
{
    int x;
    x = foo(22);
}


Inlining could turn that into:

void main()
{
    int x;
    if(22 > 5)
        x = 5;
    else
        x = 1;
}

Now you have dead code: "x = 1;". But nobody would ever expect to get an 
"unreachable code" warning from that. It would never even be helpful. It 
would always just be noise. Same goes for the OP's foreach example.

In both this inlining example and the OP's example, what's important is 
whether any of the *user's* code is unreachable, not whether any of the 
compiler's internally-generated code is unreachable.
Nov 03 2011
prev sibling next sibling parent kenji hara <k.hara.pg gmail.com> writes:
2011/11/4 Nick Sabalausky <a a.a>:

 "kenji hara" <k.hara.pg gmail.com> wrote in message
 news:mailman.655.1320322897.24802.digitalmars-d puremagic.com...

 2011/11/3 Nick Sabalausky <a a.a>:

 I don't know much about the internals of DMD, but would this make any
 sense?:

 When unrolling code, keep track of where the unrolled parts came from.
 Only
 issue an unreachable code warning if *all* the unrolled versions of the
 original line X are unreachable.



 First, when we use -w option, we usually want to show *all* warnings of
 codes.
 Isn't it your expects?



 All real warnings, yes, but this one is just simply incorrect. The line *=




 the source code* that says "writeln( "A: ", member );" =A0*does*, in fact=




 reached. Just because it doesn't get reached in *every* iteration doesn't
 change that fact that it *does* get reached and is therefore *not*
 unreachable code. The fact that the compiler unrolls it behind-the-scenes=




 irrelevent to the user - that merely explains why the warning is *current=




 thrown, not why it *should* be thrown.


 Second, each unrolled loop bodies is separately analyzable by the
 compiler.
 There is no reason to drop the detectable information, unless other
 factors affect (such as performance).

 I don't know why you want to reduce warnings that (probably) you expects=









 I don't see why anyone would ever expect this warning at all. *All* lines
 inside the user's foreach body *do* get used. It's the *compiler's* inter=




 processing that *adds* in some dead code behind-the-scenes. The source
 doesn't have any dead code. It's just like inlining:

 int foo(int x)
 {
 =A0 =A0if(x > 5)
 =A0 =A0 =A0 =A0return 5;
 =A0 =A0return 1;
 }

 void main()
 {
 =A0 =A0int x;
 =A0 =A0x =3D foo(22);
 }


 Inlining could turn that into:

 void main()
 {
 =A0 =A0int x;
 =A0 =A0if(22 > 5)
 =A0 =A0 =A0 =A0x =3D 5;
 =A0 =A0else
 =A0 =A0 =A0 =A0x =3D 1;
 }

 Now you have dead code: "x =3D 1;". But nobody would ever expect to get a=




 "unreachable code" warning from that. It would never even be helpful. It
 would always just be noise. Same goes for the OP's foreach example.

 In both this inlining example and the OP's example, what's important is
 whether any of the *user's* code is unreachable, not whether any of the
 compiler's internally-generated code is unreachable.



OK. You want to detect unreachable codes that *human feels* with -w.
I want to detect codes that is *computed* as unreachable.

I think that the -w option's Raison d'etre is to detect all of the
*potentially* unreachable codes.
For the purpose, -w should detect many codes as far as possible.
There is no reason to reduce information by merging unrolled loops.


 The fact that the compiler unrolls it behind-the-scenes is




No, foreach with tuple always unrolled. It is basically truth in
strict-typed language.
Because each elements can have different types.

And with an unrolled loop, its body code does *small generative
programming*, and the generated codes may have unreachable statements.
Current -w is useful tool to terminate them.

Finally from compiler developer sight:
Current dmd uses same statically analyzer against unrolled and normal
loops. It has been the compiler implementation simple.

Kenji Hara



 Nov 03 2011



prev sibling next sibling parent  "Marco Leise" <Marco.Leise gmx.de>  writes:


Am 03.11.2011, 17:21 Uhr, schrieb kenji hara <k.hara.pg gmail.com>:


 2011/11/4 Nick Sabalausky <a a.a>:

 "kenji hara" <k.hara.pg gmail.com> wrote in message
 news:mailman.655.1320322897.24802.digitalmars-d puremagic.com...

 2011/11/3 Nick Sabalausky <a a.a>:

 I don't know much about the internals of DMD, but would this make any
 sense?:

 When unrolling code, keep track of where the unrolled parts came from.
 Only
 issue an unreachable code warning if *all* the unrolled versions of  
 the
 original line X are unreachable.



 First, when we use -w option, we usually want to show *all* warnings of
 codes.
 Isn't it your expects?



 All real warnings, yes, but this one is just simply incorrect. The line  
 *in
 the source code* that says "writeln( "A: ", member );"  *does*, in  
 fact, get
 reached. Just because it doesn't get reached in *every* iteration  
 doesn't
 change that fact that it *does* get reached and is therefore *not*
 unreachable code. The fact that the compiler unrolls it  
 behind-the-scenes is
 irrelevent to the user - that merely explains why the warning is  
 *currently*
 thrown, not why it *should* be thrown.


 Second, each unrolled loop bodies is separately analyzable by the
 compiler.
 There is no reason to drop the detectable information, unless other
 factors affect (such as performance).

 I don't know why you want to reduce warnings that (probably) you  
 expects.



 I don't see why anyone would ever expect this warning at all. *All*  
 lines
 inside the user's foreach body *do* get used. It's the *compiler's*  
 internal
 processing that *adds* in some dead code behind-the-scenes. The source
 doesn't have any dead code. It's just like inlining:

 int foo(int x)
 {
    if(x > 5)
        return 5;
    return 1;
 }

 void main()
 {
    int x;
    x = foo(22);
 }


 Inlining could turn that into:

 void main()
 {
    int x;
    if(22 > 5)
        x = 5;
    else
        x = 1;
 }

 Now you have dead code: "x = 1;". But nobody would ever expect to get an
 "unreachable code" warning from that. It would never even be helpful. It
 would always just be noise. Same goes for the OP's foreach example.

 In both this inlining example and the OP's example, what's important is
 whether any of the *user's* code is unreachable, not whether any of the
 compiler's internally-generated code is unreachable.



 OK. You want to detect unreachable codes that *human feels* with -w.
 I want to detect codes that is *computed* as unreachable.

 I think that the -w option's Raison d'etre is to detect all of the
 *potentially* unreachable codes.
 For the purpose, -w should detect many codes as far as possible.
 There is no reason to reduce information by merging unrolled loops.


 The fact that the compiler unrolls it behind-the-scenes is




 No, foreach with tuple always unrolled. It is basically truth in
 strict-typed language.
 Because each elements can have different types.

 And with an unrolled loop, its body code does *small generative
 programming*, and the generated codes may have unreachable statements.
 Current -w is useful tool to terminate them.

 Finally from compiler developer sight:
 Current dmd uses same statically analyzer against unrolled and normal
 loops. It has been the compiler implementation simple.

 Kenji Hara



I don't understand why you want it to behave like this. Like Nick, I think  
the loop-unrolling or inlining should be transparent and not issue  
warnings that aren't understandable from the code at hand. And second the  
compiler should be free to remedy the situation itself by removing the  
dead code it just generated :p God damn, isn't that just obvious?



 Nov 03 2011



prev sibling  parent  Mehrdad <wfunction hotmail.com>  writes:


On 11/3/2011 1:15 AM, Don wrote:

 On 03.11.2011 08:47, knommad wrote:

 Hi,

 I'm a relative newcomer to D, and am enjoying the experience.

 I have a small testcase here that causes what seems to be a spurious 
 warning
 from the compiler (resulting code still works, however).

 Am I missing something??



 It's an interesting situation that you've found. The foreach is a 
 static foreach, so it gets unrolled at compile time. In each unrolled 
 bit, 'member' is a compile-time constant, so the if statement becomes 
 either true or false. The code is in fact unreachable in that iteration.
 So the warning is correct, but arguably not helpful.

 Maybe we should suppress unreachable code warnings inside unrolled code.






 Nov 06 2011



prev sibling next sibling parent reply kenji hara <k.hara.pg gmail.com>  writes:


2011/11/3 knommad <spam_D asylum.id.au>:

 Hi,

 I'm a relative newcomer to D, and am enjoying the experience.

 I have a small testcase here that causes what seems to be a spurious warn=




 from the compiler (resulting code still works, however).

 Am I missing something??

 import std.stdio;
 import std.traits;

 enum TestEnum
 {
 =A0 =A0ONE =3D 1,
 =A0 =A0TWO =3D 2,
 =A0 =A0THREE =3D 3,
 =A0 =A0FOUR =3D 4
 }

 void main()
 {
 =A0 =A0// Generates warning - works as expected though
 =A0 =A0foreach( TestEnum member; EnumMembers!TestEnum )
 =A0 =A0{
 =A0 =A0 =A0 =A0if ( member =3D=3D TestEnum.TWO )
 =A0 =A0 =A0 =A0 =A0 =A0continue;
 =A0 =A0 =A0 =A0writeln( "A: ", member );
 =A0 =A0}

 =A0 =A0// No warning generated
 =A0 =A0foreach( TestEnum member; EnumMembers!TestEnum )
 =A0 =A0{
 =A0 =A0 =A0 =A0if ( member =3D=3D TestEnum.TWO )
 =A0 =A0 =A0 =A0 =A0 =A0continue;
 =A0 =A0 =A0 =A0else
 =A0 =A0 =A0 =A0 =A0 =A0writeln( "B: ", member );
 =A0 =A0}

 =A0 =A0// No warning generated
 =A0 =A0auto list =3D EnumMembers!TestEnum;
 =A0 =A0foreach( TestEnum member; list )
 =A0 =A0{
 =A0 =A0 =A0 =A0if ( member =3D=3D TestEnum.TWO )
 =A0 =A0 =A0 =A0 =A0 =A0continue;
 =A0 =A0 =A0 =A0writeln( "C: ", member );
 =A0 =A0}

 }

 compiled with: dmd -wi -oftest test.d

 (dmd version 2.056)

 output is as expected:
 A: ONE
 A: THREE
 A: FOUR
 B: ONE
 B: THREE
 B: FOUR
 C: ONE
 C: THREE
 C: FOUR


 regards,
 ted

 --
 Money can't buy happiness. But it can buy marshmallows, which are kinda t=




 same thing.



EnumMember returns tuple, it is compile-time sequence.
And foreach with tuple unrolls its body. Then, Loop A is unrolled like foll=
ows.

__foreachEntry0: {
  if ( TestEnum.ONE =3D=3D TestEnum.TWO )
    goto __foreachEntry1;
  writeln( "A: ", TestEnum.ONE );
}
__foreachEntry1: {
  if ( TestEnum.TWO =3D=3D TestEnum.TWO )
    goto __foreachEntry2;
  writeln( "A: ", TestEnum.TWO );  // LineX
}
__foreachEntry2: {
  if ( TestEnum.THREE =3D=3D TestEnum.TWO )
    goto __foreachEntry3;
  writeln( "A: ", TestEnum.THREE );
}
__foreachEntry3: {
  if ( TestEnum.FOUR =3D=3D TestEnum.TWO )
    goto __foreachEntry4;
  writeln( "A: ", TestEnum.FOUR );
}
__foreachEntry4:
  ;

And, when member =3D=3D TestEnum.TWO, the LineX is not reachable.

With loop B, unrolling is similar, but dmd does not warn the
unreachable else block.
Because dmd treats it as that is stated by programmers as "may become
unreachable branch".
(This is reasonable assumption, sometimes the condition of if
statement folded to constant.)

With loop C, list is "tuple of local variable symbols", it is similar
to follows.

auto __list0 =3D TestEnum.ONE;
auto __list1 =3D TestEnum.TWO;
auto __list2 =3D TestEnum.THREE;
auto __list3 =3D TestEnum.FOUR;
alias TypeTuple!(list0, list1, list2, list3) list;

list itself is compile time tuple, but its elements are evaluated at
runtime. Loop C is unrolled like A and B, but the conditions are
evaluated at runtime, then compiler does not warn the branches as
unreachable.

Kenji Hara



 Nov 03 2011



 next sibling parent  Denis Shelomovskij <verylonglogin.reg gmail.com>  writes:


03.11.2011 11:29, kenji hara пишет:

 EnumMember returns tuple, it is compile-time sequence.
 And foreach with tuple unrolls its body. Then, Loop A is unrolled like follows.

 __foreachEntry0: {
    if ( TestEnum.ONE == TestEnum.TWO )
      goto __foreachEntry1;
    writeln( "A: ", TestEnum.ONE );
 }
 __foreachEntry1: {
    if ( TestEnum.TWO == TestEnum.TWO )
      goto __foreachEntry2;
    writeln( "A: ", TestEnum.TWO );  // LineX
 }
 __foreachEntry2: {
    if ( TestEnum.THREE == TestEnum.TWO )
      goto __foreachEntry3;
    writeln( "A: ", TestEnum.THREE );
 }
 __foreachEntry3: {
    if ( TestEnum.FOUR == TestEnum.TWO )
      goto __foreachEntry4;
    writeln( "A: ", TestEnum.FOUR );
 }
 __foreachEntry4:
    ;

 And, when member == TestEnum.TWO, the LineX is not reachable.

 With loop B, unrolling is similar, but dmd does not warn the
 unreachable else block.
 Because dmd treats it as that is stated by programmers as "may become
 unreachable branch".
 (This is reasonable assumption, sometimes the condition of if
 statement folded to constant.)

 With loop C, list is "tuple of local variable symbols", it is similar
 to follows.

 auto __list0 = TestEnum.ONE;
 auto __list1 = TestEnum.TWO;
 auto __list2 = TestEnum.THREE;
 auto __list3 = TestEnum.FOUR;
 alias TypeTuple!(list0, list1, list2, list3) list;

 list itself is compile time tuple, but its elements are evaluated at
 runtime. Loop C is unrolled like A and B, but the conditions are
 evaluated at runtime, then compiler does not warn the branches as
 unreachable.

 Kenji Hara



Wow... Indeed. Excellent answer. Just like an article from 
documentation. Thanks, it's usable for me too.



 Nov 03 2011



prev sibling  parent  knommad <spam_D asylum.id.au>  writes:


kenji hara wrote:


 2011/11/3 knommad <spam_D asylum.id.au>:

 Hi,

 I'm a relative newcomer to D, and am enjoying the experience.

 I have a small testcase here that causes what seems to be a spurious
 warning from the compiler (resulting code still works, however).

 Am I missing something??

 import std.stdio;
 import std.traits;

 enum TestEnum
 {
 ONE = 1,
 TWO = 2,
 THREE = 3,
 FOUR = 4
 }

 void main()
 {
 // Generates warning - works as expected though
 foreach( TestEnum member; EnumMembers!TestEnum )
 {
 if ( member == TestEnum.TWO )
 continue;
 writeln( "A: ", member );
 }

 // No warning generated
 foreach( TestEnum member; EnumMembers!TestEnum )
 {
 if ( member == TestEnum.TWO )
 continue;
 else
 writeln( "B: ", member );
 }

 // No warning generated
 auto list = EnumMembers!TestEnum;
 foreach( TestEnum member; list )
 {
 if ( member == TestEnum.TWO )
 continue;
 writeln( "C: ", member );
 }

 }

 compiled with: dmd -wi -oftest test.d

 (dmd version 2.056)

 output is as expected:
 A: ONE
 A: THREE
 A: FOUR
 B: ONE
 B: THREE
 B: FOUR
 C: ONE
 C: THREE
 C: FOUR


 regards,
 ted

 --
 Money can't buy happiness. But it can buy marshmallows, which are kinda
 the same thing.



 EnumMember returns tuple, it is compile-time sequence.
 And foreach with tuple unrolls its body. Then, Loop A is unrolled like
 follows.
 
 __foreachEntry0: {
   if ( TestEnum.ONE == TestEnum.TWO )
     goto __foreachEntry1;
   writeln( "A: ", TestEnum.ONE );
 }
 __foreachEntry1: {
   if ( TestEnum.TWO == TestEnum.TWO )
     goto __foreachEntry2;
   writeln( "A: ", TestEnum.TWO );  // LineX
 }
 __foreachEntry2: {
   if ( TestEnum.THREE == TestEnum.TWO )
     goto __foreachEntry3;
   writeln( "A: ", TestEnum.THREE );
 }
 __foreachEntry3: {
   if ( TestEnum.FOUR == TestEnum.TWO )
     goto __foreachEntry4;
   writeln( "A: ", TestEnum.FOUR );
 }
 __foreachEntry4:
   ;
 
 And, when member == TestEnum.TWO, the LineX is not reachable.
 
 With loop B, unrolling is similar, but dmd does not warn the
 unreachable else block.
 Because dmd treats it as that is stated by programmers as "may become
 unreachable branch".
 (This is reasonable assumption, sometimes the condition of if
 statement folded to constant.)
 
 With loop C, list is "tuple of local variable symbols", it is similar
 to follows.
 
 auto __list0 = TestEnum.ONE;
 auto __list1 = TestEnum.TWO;
 auto __list2 = TestEnum.THREE;
 auto __list3 = TestEnum.FOUR;
 alias TypeTuple!(list0, list1, list2, list3) list;
 
 list itself is compile time tuple, but its elements are evaluated at
 runtime. Loop C is unrolled like A and B, but the conditions are
 evaluated at runtime, then compiler does not warn the branches as
 unreachable.
 
 Kenji Hara



Hi,
Thanks for the detailed and informative answer. 

I am puzzled, however, that there is an inconsistency. There were 3 examples 
in the test case, which I would have expected to yield the same compiler 
output - but it only generates a warning in the first example.

regards,
ted



-- 
If you're not failing every now and again, it's a sign you're not doing 
anything very innovative.
                                                                               
   -- 
Woody Allen



 Nov 04 2011



prev sibling next sibling parent reply kenji hara <k.hara.pg gmail.com>  writes:


2011/11/3 Don <nospam nospam.com>:

 Maybe we should suppress unreachable code warnings inside unrolled code.



I don't agree with this opinion of you.
It is useful when we write generic code.

Kenji Hara



 Nov 03 2011



  parent  "Nick Sabalausky" <a a.a>  writes:


"kenji hara" <k.hara.pg gmail.com> wrote in message 
news:mailman.649.1320309243.24802.digitalmars-d puremagic.com...

 2011/11/3 Don <nospam nospam.com>:

 Maybe we should suppress unreachable code warnings inside unrolled code.



 I don't agree with this opinion of you.
 It is useful when we write generic code.



I don't know much about the internals of DMD, but would this make any 
sense?:

When unrolling code, keep track of where the unrolled parts came from. Only 
issue an unreachable code warning if *all* the unrolled versions of the 
original line X are unreachable.



 Nov 03 2011



prev sibling  parent  Trass3r <un known.com>  writes:


Very enlightening, thanks!



 Nov 03 2011