[Fixed] [ZScript] uint max vals have only the MSB set, not all bits

Bugs that have been investigated and resolved somehow.

Moderator: GZDoom Developers

[ZScript] uint max vals have only the MSB set, not all bits

Postby phantombeta » Wed Jan 06, 2021 5:16 am

Max values ("type.max") for unsigned integer types are wrong. GZDoom is setting them to a constant with only the MSB set, not all bits (i.e., the highest value they can hold). Because of this line:
Code: Select allExpand view
Symbols.AddSymbol(Create<PSymbolConstNumeric>(NAME_Max, this, (1u << ((8 * size) - 1))));

The correct expression, as far as I know, would be:
Code: Select allExpand view
Symbols.AddSymbol(Create<PSymbolConstNumeric>(NAME_Max, this, (uint32_t) ((((uint64_t) 1u) << (8 * size)) - 1)));

(Of course, this won't work for unsigned 64-bit ints, if we ever add them... Not sure how to handle that.)

Quick edit: Thinking about it, this might work, as long as uint64_t right shift correctly performs an unsigned right shift (which it should, as far as I know):
Code: Select allExpand view
Symbols.AddSymbol(Create<PSymbolConstNumeric>(NAME_Max, this, (uint32_t) (~((uint64_t) 0u) >> (uint64_t) (64 - 8 * size))));


This leaves only the question of whether we should fix it or not. Mods could be relying on this and break if it gets changed, after all... But then again, you'd think someone would have noticed this, if unsigned ints were getting used much. So it's probably fine.

Edit: Fixed a copy-paste mistake on the second solution.
Last edited by phantombeta on Wed Jan 06, 2021 12:25 pm, edited 1 time in total.
User avatar
phantombeta
In the meadow of sinful thoughts, every flower's a perfect one
 
Joined: 02 May 2013
Location: Brazil, South America, Earth, Orion-Cygnus Arm, Milky Way
Discord: phantombeta#2461
Twitch ID: phantombeta_
Github ID: Doom2fan
Operating System: Windows 10/8.1/8/201x 64-bit
OS Test Version: No (Using Stable Public Version)
Graphics Processor: nVidia with Vulkan support

Re: [ZScript] uint max vals have only the MSB set, not all b

Postby Graf Zahl » Wed Jan 06, 2021 7:22 am

You have commit access to the repo, why don't you fix it yourself?
User avatar
Graf Zahl
Lead GZDoom+Raze Developer
Lead GZDoom+Raze Developer
 
Joined: 19 Jul 2003
Location: Germany

Re: [ZScript] uint max vals have only the MSB set, not all b

Postby phantombeta » Wed Jan 06, 2021 10:00 am

Because of this:
phantombeta wrote:This leaves only the question of whether we should fix it or not. Mods could be relying on this and break if it gets changed, after all... But then again, you'd think someone would have noticed this, if unsigned ints were getting used much. So it's probably fine.
User avatar
phantombeta
In the meadow of sinful thoughts, every flower's a perfect one
 
Joined: 02 May 2013
Location: Brazil, South America, Earth, Orion-Cygnus Arm, Milky Way
Discord: phantombeta#2461
Twitch ID: phantombeta_
Github ID: Doom2fan
Operating System: Windows 10/8.1/8/201x 64-bit
OS Test Version: No (Using Stable Public Version)
Graphics Processor: nVidia with Vulkan support

Re: [ZScript] uint max vals have only the MSB set, not all b

Postby Graf Zahl » Wed Jan 06, 2021 11:21 am

I cannot answer that - but the past has shown that every single bug that got sat out "for compatibility" turned into a bigger nightmare in the end,
User avatar
Graf Zahl
Lead GZDoom+Raze Developer
Lead GZDoom+Raze Developer
 
Joined: 19 Jul 2003
Location: Germany

Re: [ZScript] uint max vals have only the MSB set, not all b

Postby phantombeta » Wed Jan 06, 2021 12:53 pm

User avatar
phantombeta
In the meadow of sinful thoughts, every flower's a perfect one
 
Joined: 02 May 2013
Location: Brazil, South America, Earth, Orion-Cygnus Arm, Milky Way
Discord: phantombeta#2461
Twitch ID: phantombeta_
Github ID: Doom2fan
Operating System: Windows 10/8.1/8/201x 64-bit
OS Test Version: No (Using Stable Public Version)
Graphics Processor: nVidia with Vulkan support


Return to Closed Bugs

Who is online

Users browsing this forum: No registered users and 1 guest