Compiler Warning

[John Z]

if you declare

Code: [Select]

HICON hIcon = NULL;
then within

Code: [Select]

case WM_INITDIALOG:
   hIcon = LoadImage( ghInstance,MAKEINTRESOURCE(JGZ),IMAGE_ICON,
                           32,32,LR_SHARED);                           
then within

Code: [Select]

case WM_CLOSE:
   if (hIcon) {DestroyIcon(hIcon);}        
The compiler generates a Warning message for > 'if (hIcon) {DestroyIcon(hIcon);}'
: warning #2154: Unreachable code.
 
Then change the declare to

Code: [Select]

HICON hIcon;
The compiler generates a Warning message for > 'HICON hIcon;'
:warning #2229: Local 'hIcon' is potentially used without being initialized.

Is there a third way that has no warning ?  I use warnings Level 2 and C11.

John Z

[frankie]

There is something wrong here.
You should get the message:

Code: [Select]

: warning #2154: Unreachable code. because the compiler evaluates that the the condition:

Code: [Select]

hIcon != NULL is never met.
For this reason the whole block:

Code: [Select]

if (hIcon) {DestroyIcon(hIcon);} is removed from compilation output as "dead code".
This could happen if:

• hicon is a local variable
• No change happens to its value so the compiler can safely assume that it will always be null.

To understand if these conditions are really met, and this is not a compiler bug, you should post the smallest piece of complete code that shows the problem.

If tis is a bug you can try as workaround:

Code: [Select]

volatile HICON hIcon;The 'volatile' qualifier informs the compiler that the variable can be changed by external causes, and for this reason no assumptions on its value have to be made.

[John Z]

Yes I understand why, but one or the other should not be in error I think.

static HICON hIcon = NULL;

Also 'fixes' (no warnings) it but clearly not that desirable.

I'll try to generate a simple example. 

John Z

[frankie]

I'll try to generate a simple example. 

John Z

Yes please, because I can't reproduce such error with PellesC V11.

[John Z]

Frankie, Thanks for the reminder to include the Pelles C version too.

 Initial work was from version 10.  I'm a slow adopter 

However attached is a simple program test case which I ran in version 10 and version 11.  The reported
warnings are somewhat different from each but both versions show the only way to satisfy the compiler (ie no warning)
is to use 'static HICON hIcon;' or 'volatile HICON hIcon = NULL;'

IMO should not be the case because clearly

Code: [Select]

hIcon = LoadImage( ghInstance,MAKEINTRESOURCE(JGZ),IMAGE_ICON,
                           32,32,LR_SHARED);will affect the value of hIcon.

Just uncomment the variable declare at the top of MainDlgProc and compile.  The warning line number I put in the comments will be off a bit because I added comments while testing.  There are 5 test cases.  Should have been 6
I forgot to include 'static HICON hIcon = NULL;'


John Z

[MrBcx]

This works for me on V11

HICON hIcon = {0};

[frankie]

Ok John I see now.
The compiler seems correct, there is a flaw in the logic of the function 'MainDlgProc'.
When you declare:

Code: [Select]

HICON hIcon = NULL; or

Code: [Select]

HICON hIcon;You are creating an automatic variable that will be lost as soon as execution flow exits the function. This imply that the value of the variable 'hicon' exists for a single switch case per call.
So you can have:

• When 'uMsg' is 'WM_INITDIALOG' you are assigning a value to it, that will be lost on return.
• When 'uMsg' is 'WM_CLOSE':
  
  • If 'hicon' isn't initialized you're using a dangling value for the 'if' test (warning #2116)
  • if 'hicon' is initialized to NULL the comparison is constant and always false and the conditional code never executed ('warning #2154: Unreachable code' and 'warning #2154: Unreachable code')

The problem is essentially related to the scope of the variable. Probably this piece of code was originally positioned in the 'WinMain' function and its scope was persistent for the whole program execution ('WinMain' scope end when program ends). Moving the code to a function the scope was modified and local to the function itself.
Making it 'static' guarantee that the variable exists and retain its value between calls, miming the functionality as when declared in the 'WinMain'.

Anyway something that I've not fully understand happens using the 'volatile' qualifier.
Using:

Code: [Select]

volatile HICON hIcon = NULL; the compiler doesn't complain, while omitting the initialization to 'NULL' V11 warns about constant compare. What's the rationale behind this?
Anyway making it 'static' no more complains are made.

[John Z]

Thanks frankie,

The code didn't come from anywhere other than my mind. .....

I guess it is my misunderstanding coming from a C procedural flow world to a Windows event driven world.
I thought I could use WM_INITDIALOG to set the hIcon once and it would be there for the duration of the existence
of the dialog, but I knew it needed to be destroyed before closing the dialog.  I had ended up using Static in the code but thought, as I was trying to find a bug, that this might be my bug source.  So since the only dumb question is the one you don't ask, I figured I'd better check.   

Appreciate the help,
John

[frankie]

John you're always welcome.
It is good IMHO to talk and discuss some detail that could be interesting for many, also considering that more skilled is the programmer more often she/he stops at very simple problems because of the mind orientation that looks always for major issues... 
It is the human nature indeed at last... 
On my side I still have doubts about the compiler warnings when the 'volatile' qualifier is used, as I reported in my previous post.

[Grincheux]

[size=0px]The compiler generates a Warning message for > 'if (hIcon) {DestroyIcon(hIcon);}'[/size][/size][size=0px]: warning #2154: Unreachable code.[/size]

[size=0px]

Yes because it is declared as NULL

If hIcon is local this is normal[/size]