[062] Visual Studio function glitch

I was going through the beginning of Day 62 and was baffled as to why my Sword.High entity was returning null pointer. Turns out I was baffled because there was nothing wrong with the code. Visual Studio is doing a weird thing where it's treating an if statement inside a function as scoped, so when the sword leaves that if block, it goes out of scope and the function can't return it.

When I moved
1
return(EntityHigh)
inside the glitching if statement, it worked just as it should. Fortunately, VS isn't doing this anywhere else in the code, just in that one spot. I still want to fix it, though, and reinstalling VS didn't help.

Does anyone have any ideas?
Are you saying this doesn't work:
 1
 2
 3
 4
 5
 6
 7
 8
 9
10
11
12
    high_entity *EntityHigh = 0;
                                    
    if(...)
    {
        EntityHigh = ...;
    }
    else
    {    
        EntityHigh = ...;
    }

    return(EntityHigh);


But this works:
 1
 2
 3
 4
 5
 6
 7
 8
 9
10
11
12
    high_entity *EntityHigh = 0;
                                    
    if(...)
    {
        EntityHigh = ...;
        return(EntityHigh);
    }
    else
    {    
        EntityHigh = ...;
        return(EntityHigh);
    }
??

I'm pretty this is not the issue and reinstalling VS will not help you here at all. There is something else wrong either with your changes or how are you using compiler or debugger.

Edited by Mārtiņš Možeiko on
Yes, that's exactly what's happening. The first example you gave doesn't work, but the second one does. The MakeEntityHighFrequency function is the only piece of code that has this problem, which makes me think it's not systemic. I've copy and pasted the entire function below;

 1
 2
 3
 4
 5
 6
 7
 8
 9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
inline high_entity *
MakeEntityHighFrequency(game_state *GameState, low_entity *EntityLow, uint32 LowIndex, v2 CameraSpaceP)
{
	high_entity *EntityHigh = {};

	Assert(EntityLow->HighEntityIndex == 0);
	if (EntityLow->HighEntityIndex == 0)
	{
		if (GameState->HighEntityCount < ArrayCount(GameState->HighEntities))
		{
			uint32 HighIndex = GameState->HighEntityCount++;
			high_entity *EntityHigh = &GameState->HighEntities[HighIndex];

			EntityHigh->P = CameraSpaceP;
			EntityHigh->dP = V2(0, 0);
			EntityHigh->ChunkZ = EntityLow->P.ChunkZ;
			EntityHigh->FacingDirection = 0;

			EntityHigh->LowEntityIndex = LowIndex;
			EntityLow->HighEntityIndex = HighIndex;

			return(EntityHigh);
		}
		else
		{
			InvalidCodePath;
		}	
	}	
	
}


Putting return(EntityHigh) at the very end of the function, as you would normally do, creates the problem with EntityHigh going out of scope. (I can actually see it go out of scope as I step through the code.)

Edited by Huntress77 on
There is a difference between:
1
2
3
4
5
6
7
    high_entity *EntityHigh = 0;
    if(...)
    {
        EntityHigh = ...;
        // ... other code
    }
    return(EntityHigh);


and this code:

1
2
3
4
5
6
7
    high_entity *EntityHigh = 0;
    if(...)
    {
        high_entity *EntityHigh = ...;
        // ... other code
    }
    return(EntityHigh);


Do you see the difference? Try to answer to yourself why there is a difference before reading further.

Your fragment of code has two EntityHigh variables. If you put return inside if statements, you will be returning the "inner" EntityHigh. If you have return statement at end of function (outside if statement), you will be returning the "outer" EntityHigh (the one which is defined at beginning of function.

Most likely you want to change "high_entity *EntityHigh = &GameState->HighEntities[HighIndex];" into "EntityHigh = &GameState->HighEntities[HighIndex];". This will assign new value to existing EntityHigh pointer, instead of defining new EntityHigh variable.

And yes, that inner EntityHigh variable really goes out of scope, when you are stepping over the "return" line if it is at bottom of function. This was just a bug in the code, no need to reinstall whole Visual Studio in these kind of situations. It's almost always bug in the code, and not a issue with VS installation.

Edited by Mārtiņš Možeiko on
(forehead smack) I can't believe how many times I went over that function and never spotted it! I do know what happens when you declare the same variable twice, but I just couldn't "see" the error. Thanks for pointing it out. I won't be so quick to assume VS is at fault in the future.

Edited by Huntress77 on
Visual Studio static analyzer can warn you about variable shadowing. Add /analyze to your compile switches to use it. But it will slow down your compile time, so use it only when you have a problem or want to check your code.
And if you are using VS2015 and up then you can enable either warning level 4 or just C4456 warning (+ make it as error), then compiler will warn you about these situations:
1
2
3
4
C:\test>cl /nologo /c /w14456 main.cpp
main.cpp
main.cpp(8): warning C4456: declaration of 'EntityHigh' hides previous local declaration
main.cpp(5): note: see declaration of 'EntityHigh'


Edited by Mārtiņš Možeiko on