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 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 reply "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
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 reply "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
parent reply 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 *=
in
 the source code* that says "writeln( "A: ", member );" =A0*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 *current=
ly*
 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=
nal
 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=
n
 "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
irrelevent to the user 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
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
irrelevent to the user 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.
Isn't this what static if is for?
Nov 06 2011
prev 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=
ing
 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=
he
 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 next sibling parent Trass3r <un known.com> writes:
Very enlightening, thanks!
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