[GEN][ZH] Change TheGlobalData macro to variable in debug mode for debugging convenience

[Caball009]

Preprocessor macros aren't supported in the debugger. For instance, if a constant VALUE is declared as: #define VALUE 3, you can't use VALUE in the Watch window.

The value of a macro cannot be inspected with the VS22 debugger: https://learn.microsoft.com/en-us/visualstudio/debugger/expressions-in-the-debugger

This PR makes it more convenient to see the values of the TheGlobalData macro when _DEBUG, RTS_DEBUG or RTS_INTERNAL is defined. The second pointer should not be used in release mode because of the potential for missed optimizations. (not relevant for the version with static union)

[xezon]

If this works in Debug, then can we have this in Release too?

[Caball009]

Frankly, I'm not a great fan of this solution even though it seems quite neat. C++ has quite strict rules about unions:

It is undefined behavior to read from the member of the union that wasn't most recently written. Many compilers implement, as a non-standard language extension, the ability to read inactive members of a union.

https://www.cppreference.com/w/cpp/language/union.html

Now maybe in our case there are mitigating circumstances considering that both pointers are of the same type and only differ in const-ness, but I don't know. The idea was that the debugging convenience might outweigh the potential for UB, but I wouldn't want to do it this way in release mode.

[Caball009]

FWIW, my first commit is a setup with a second pointer like you suggested initially.

[xezon]

Hmm unfortunate. Can we put

extern GlobalData* TheWritableGlobalData;
extern const GlobalData* TheGlobalData;

on the same cache line somehow?

[Caball009]

I think putting their declarations right under each other, as they are, is the best we can do in that regard.

What's the relevance of cache lines here?

[helmutbuhler]

How about we replace TheWritableGlobalData with TheGlobalData and make it writeable. The whole const thing is stupid, let's not make it even more stupid.

[xezon]

Yes sure. We can also protect against accidental writes by writing expression like so:

if (0 == TheGlobalData->m_value)

[Caball009]

Yoda notation isn't great for readability imo and modern compilers can detect and warn for accidental assignments in if statements.

I think there's value in applying const as much as possible and doubly so when accessing a singleton, so I wouldn't be in favor of removing it entirely. I'm on the fence with regards to the change to a template. One fewer macro is a good thing, having to change 800+ cases of TheWritableGlobalData less so. I'd probably just stick to the first commit implementation.

[helmutbuhler]

Never heard it called Yoda notation before :)

[xezon]

I'd probably just stick to the first commit implementation.

It is mostly useless though. I almost always test in Release.

[helmutbuhler]

Btw, the recent commandline PR changed some things around GlobalData. I suggest updating before continuing on this.