2

I'm getting:

warning C4533: initialization of 'b' is skipped by goto FreeDC.

But if the code gets to the label FreeDC in WM_CREATE, 'b' is not initialized. How its initialization could be skiped, if it is not initialized in that situation. I just don't understand the warning.

#include <windows.h>

class A
{
    int i;

    public:
    A() {};
    A(int i) : i(i) {}
};

LRESULT CALLBACK WndProc(HWND, UINT, UINT, LONG);

HINSTANCE ghInstance;

/************************************************************************************************************************

    WinMain(hInstance, hPrevInstance, pszCmdLine, nCmdShow)

************************************************************************************************************************/

int APIENTRY WinMain (HINSTANCE hInstance, HINSTANCE hPrevInstance, LPSTR pszCmdLine, int nCmdShow)
{
    ghInstance = hInstance;

    WNDCLASSEX  wndclassx;

    wndclassx.cbSize        = sizeof(WNDCLASSEX);
    wndclassx.style         = CS_HREDRAW | CS_VREDRAW;
    wndclassx.lpfnWndProc   = WndProc;
    wndclassx.cbClsExtra    = 0;
    wndclassx.cbWndExtra    = 0;
    wndclassx.hInstance     = hInstance;
    wndclassx.hIcon         = NULL;
    wndclassx.hCursor       = LoadCursor(NULL, IDC_ARROW);
    wndclassx.hbrBackground = (HBRUSH)(COLOR_WINDOW + 1);
    wndclassx.lpszMenuName  = NULL;
    wndclassx.lpszClassName = L"WndProc";
    wndclassx.hIconSm       = NULL;

    if( !RegisterClassEx(&wndclassx) ) return 0;

    HWND hWnd = CreateWindow(L"WndProc", L"", WS_OVERLAPPEDWINDOW, CW_USEDEFAULT, CW_USEDEFAULT, CW_USEDEFAULT,
                             CW_USEDEFAULT, NULL, NULL, hInstance, NULL);

    ShowWindow(hWnd, SW_SHOWMAXIMIZED);
    UpdateWindow(hWnd);

    MSG msg;

    while( GetMessage(&msg, NULL, 0, 0) )
    {
        TranslateMessage(&msg);
        DispatchMessage(&msg);
    }

    //  Retorna msg.wParam

    return (int)msg.wParam;
}

/************************************************************************************************************************

    WndProc(hwnd, message, wParam, lParam)

************************************************************************************************************************/

LRESULT CALLBACK WndProc (HWND hwnd, UINT message, UINT wParam, LONG lParam)

{
    static A a;
    static int i;

    switch ( message )
    {
        case WM_CREATE:
        {
            HDC hDC;
            if( !(hDC = GetDC(hwnd)) ) return -1;

            int iLogPixelsY = GetDeviceCaps(hDC, LOGPIXELSY);

            LOGFONT lf;
            memset(&lf, 0, sizeof(LOGFONT));
            lf.lfHeight = -MulDiv(11, iLogPixelsY, 72);
            wcscpy_s(lf.lfFaceName, LF_FACESIZE, L"Cambria Math");

            HFONT hFont;
            if( !(hFont = CreateFontIndirect(&lf)) ) goto FreeDC;

            hFont = (HFONT)SelectObject(hDC, hFont);

            int j = 5;
            i = j;

            A b(2);
            a = b;
            return 0;

            FreeDC: ReleaseDC(hwnd, hDC);
            return -1; 
        }
        break;

        case WM_DESTROY:

        PostQuitMessage(0);
        break;

        default:

        return DefWindowProc(hwnd, message, wParam, lParam);
    }
    return 0;
}
Ayrosa
  • 3,385
  • 1
  • 18
  • 29
  • Please consider a minimal test-case. It will make the post more useful to others, perhaps. –  Nov 11 '11 at 23:43
  • 3
    The message is telling you that the constructor for `b` is not invoked if you jump to `FreeDC`. – Oliver Charlesworth Nov 11 '11 at 23:44
  • I never thought I would see a `goto` in code like this. Consider RAII instead. – K-ballo Nov 11 '11 at 23:48
  • 2
    I'm a little confused by the question. "Why does the compiler tell me that initialization is skipped in the case where I skipped initialization?" Um, because you skipped initialization? The point is that `b`'s constructor did not run (because you skipped over it), but its destructor will run, which means you're destructing an object that was never constructed. – Raymond Chen Nov 11 '11 at 23:55
  • @Raymond Could you explain a bit more this destruction of 'b'. How is it going to be destructed, when it is not constructed ? – Ayrosa Nov 12 '11 at 00:03
  • Your question is not clear. You are essentially asking "how it is not initialized when it is not initialized". That just doesn't make sense. – AnT stands with Russia Nov 12 '11 at 00:09
  • *You* know the object was never constructed, but the compiler doesn't. It is just going to insert a destructor call, and hope for the best. – Dennis Zickefoose Nov 12 '11 at 00:10
  • @Dennis I'm sorry to insist, but why the compiler is going to insert a destructor call, if the object was not even constructed ? – Ayrosa Nov 12 '11 at 00:33
  • Because that's what the compiler does when an object goes out of scope, it calls the object's destructor. – Dennis Zickefoose Nov 12 '11 at 00:37
  • 2
    The variable `b` is in scope. Therefore, it will be destructed when control exits the scope. Section 6.7(3) of the C++ language specification explicitly says that you are not allowed to jump over the initialization of an object into a scope where the object exists, with one exception (which does not apply here). – Raymond Chen Nov 12 '11 at 00:37
  • @Raymond Thanks. That is pretty clear now. – Ayrosa Nov 12 '11 at 00:47

5 Answers5

6

b's constructor won't called if you use goto, and yet it's still in scope. This is technically an error, although some compilers only give off a warning.

Here's an example:

int main() {
  goto foo;
  int bar = 5;
  foo:
  ++bar; // doesn't work if goto is used - bar isn't initialized
}

It may seem like you are not using b, but its destructor is still being called:

int main() {
  goto foo;
  A b;
  foo:
  b.~A(); // compiler silently adds destructor and other cleanup here
          // won't work if goto is used - b isn't initialized
}
Pubby
  • 51,882
  • 13
  • 139
  • 180
  • +1, I didn't realize at first that the destructor would be called. – Ed S. Nov 12 '11 at 00:08
  • Your first example is different than mine, since the object 'b' is not constructed if the code reaches FreeDC. Regarding your second example, why is it that 'b' destructor will be called, once it was not constructed. – Ayrosa Nov 12 '11 at 00:09
  • 1
    @jaayrosa It's not constructed but it's still in scope. Destructors are called when objects leave scope - checking if an object is constructed before destructing it is unnecessary overhead. – Pubby Nov 12 '11 at 00:18
3

You could avoid the problem by introducing a suitable local scope that gets skipped by the goto:

        HFONT hFont;

        if( !(hFont = CreateFontIndirect(&lf)) )
        {
          goto FreeDC;
        }

        hFont = (HFONT)SelectObject(hDC, hFont);

        {               // new scope; skipped entirely by goto
          int j = 5;
          i = j;

          A b;
          a = b(2);
        }

        return 0;

    FreeDC:
        ReleaseDC(hwnd, hDC);
        return -1;

If you think about C++ and scopes and automatic object lifetimes really carefully, you'll come to conclude that goto really wreaks havoc with the entire programming model. That's why there are many (often quietly implied) conditions on where you can go-to and wher not. Generally, jumping into the middle of a scope is problematic if the scope contains new automatic variables. We avoid this by introducing a new, local scope that the goto jump skips entirely.

Kerrek SB
  • 464,522
  • 92
  • 875
  • 1,084
  • But why the warning for the initialization of j doesn't happen ? – Ayrosa Nov 12 '11 at 00:19
  • @jaayrosa: Because you don't jump over `i`s initialization... that happens as soon as you enter the function for the first time, and the program hits the `static int i;` line. – Dennis Zickefoose Nov 12 '11 at 00:21
  • @jaayrosa: Who knows what's going in the mind of the compiler... `int` is a fundamental type, so its initialization cannot have side effects, so perhaps the compiler can figure out that skipping the initialization of `j` cannot possibly lead to undefined behaviour. (Note that you're not referring to `j` after the goto label, either.) – Kerrek SB Nov 12 '11 at 00:23
  • @jaayrosa: Also do think about the *destructor* of `b` in your code, and compare it to mine. – Kerrek SB Nov 12 '11 at 00:24
  • @Kerrek Your solution solves the problem. The warning disappears. But I'm just trying to understand what's going on. Nobody has asnwered me about the destruction of 'b' being called, notwithstanding the fact that its constructor was not called. How can that be ?? – Ayrosa Nov 12 '11 at 00:31
  • 1
    @jaayrosa: Well, that's exactly the problem. What should the correct semantics be? Should `b`'s destructor be called, even though `b` is not in a valid state? Clearly not. But should it be skipped? That's clearly also a violation of lexical scoping. So the answer is that you simply mustn't jump across initializations, which is what the warning is trying to tell you. (A more adult answer would be that you simply shouldn't use `goto`, but we're already past that.) – Kerrek SB Nov 12 '11 at 00:35
  • 1
    @jaayrosa: As I said elsewhere, *the compiler does not know the constructor was not called.* When the variable goes out of scope, it calls all relevant destructors, period. – Dennis Zickefoose Nov 12 '11 at 00:35
  • 4
    The real flaw is jumping over the initialization, which is prohibited in section 6.7. People are talking about the destructor of `b` because they are trying to explain why rule 6.7 exists. Rule 6.7 does not say "It's okay to jump over the initialization of an object if you don't use it," which appears to be what you wish rule 6.7 said (but it doesn't). – Raymond Chen Nov 12 '11 at 00:46
  • @Kerrek Thanks for your help. I think I understand now what the problem was. – Ayrosa Nov 12 '11 at 00:48
1

I honestly don't know, but why are you using a goto when an if statement will suffice?

if( (hFont = CreateFontIndirect(&lf)) ) {
    hFont = (HFONT)SelectObject(hDC, hFont);

    int j = 5;
    i = j;

    A b;
    a = b(2);
    return 0;
}
else {
    FreeDC: ReleaseDC(hwnd, hDC);
    return -1; 
}

// break; here is unnecessary.
Ed S.
  • 122,712
  • 22
  • 185
  • 265
  • I just simplified the code. In reality I have several other cases where the window creation must be aborted returning -1. The goto allows me not to repeat code in those circunstances. But anyway the problem is not related to the goto, I think, and yes with the initialization of 'b'. – Ayrosa Nov 11 '11 at 23:54
  • @jaayrosa: The problem *is* related to the goto, because if you got rid of the goto, you would also get rid of the problem. There are numerous ways to avoid this... RAII constructs, breaking the code into subfunctions, redesigning your logic. – Dennis Zickefoose Nov 12 '11 at 00:07
1

You must not skip an object's initialization with either goto or switch [*](that holds for user-defined types as well as primitive types like ints). In your case, you are not using the object whose initialization you've skipped, so the best solution would be to make this clear to the compiler by limiting the scope of b.

        if( !(hFont = CreateFontIndirect(&lf)) ) goto FreeDC;

        hFont = (HFONT)SelectObject(hDC, hFont);

        int j = 5;
        i = j;
    {
        A b;
        a = b(2);
        return 0;
    }
        FreeDC: ReleaseDC(hwnd, hDC);

[*] so these would be illegal:

switch(x) {
    case 1:
        int y=1;
    case 2:
        // y not initialized if x==2

and

if (x) goto l;
int y=1;
l: // y not initialized if x!=0

This particularly matters if y is a reference, a constant or a user-defined object with nontrivial constructor.

The standard says it in 6.7/3:

It is possible to transfer into a block, but not in a way that bypasses declarations with initialization. A program that jumps from a point where a local variable with automatic storage duration is not in scope to a point where it is in scope is ill-formed unless the variable has POD type (3.9) and is declared without an initializer (8.5).

jpalecek
  • 47,058
  • 7
  • 102
  • 144
  • But why the warning for the initialization of j doesn't happen ? – Ayrosa Nov 12 '11 at 00:19
  • 1
    @jaayrosa: Probably as a deviation from the standard, Microsoft people just want to give the programmers more freedom. After all, it is permitted in C (except for VLA variables). – jpalecek Nov 12 '11 at 00:41
1

Consider a smaller, trivial test case:

struct Object {
   Object(int i) : i(i) { }
   int i;
};

int main() {
    Object a(5);
    goto Label;
    Object b(6);
  Label:
    cout << a.i << " " << b.i << endl;
}

At that last line, a.i is obviously 5. But what is the value of b.i? When that object was created, it was supposed to be initialized to 6, but you explicitly told the program to skip that line. It could be anything.

Now, lets pretend Object is a more useful type:

struct Object {
  Object(int i) : p(new int(i)) { }
  ~Object() { delete p; }
  //insert copy/move constructors/assignment here
  int* p;
};

int main() {
    Object a(5);
    goto Label;
    Object b(6);
  Label:
    cout << *a.p << endl;
}

Now, you never actually use b.p, so it looks like the fact that you skipped the initialization is no big deal. a.p was properly initialized, so this will output 5, no problem. But then you return from main, and destructors start being called... including b.~Object(), which calls delete p;. But b.p was never initialized, so who knows what that line will do?

In these cases, I believe the code is actually ill-formed, and the compiler is required to reject it. It appears that instead of outright rejecting it, the compiler is chosing to warn you about the possible issues, so that you can decide for yourself if there is a concern.

Dennis Zickefoose
  • 10,791
  • 3
  • 29
  • 38