curiousdannii-testing
commented
2 years ago 557058:4c095ffd-6d6f-47ce-9e73-77c613347b86:
Comment by zarf :
Touching this much code is scary, so I did a test run of seven compiles:
inform +i6lib-611 Advent.inf
inform -G +i6lib-611 Advent.inf
inform -D +i6lib-611 Advent.inf
inform -G -D +i6lib-611 Advent.inf
inform -Dw i7gentest.inf
inform -G -Dw i7gentest.inf
inform -G glulxercise.inf
(i7gentext.inf is the I6 generated from a one-line I7 game.)
MD5 checksums before-and-after show that these code changes do not change the output of the I6 compiler. It's possible that I screwed up some obscure case (perhaps an I6 statement which is no longer used) but I'm reasonably confident.
The Xcode analyzer still shows a couple of problems, but I decided they're not possible in real life and I'm not going to fix them.
Compiler warnings that I turned off because there were just too many of them:
- code that is never executed
- variable written but never read
- suspicious conversion between int types of different sizes
Reported by : zarf
Description :
I compiled the source with Xcode's fancy static analysis feature. It turned up lots of warnings about possibly uninitialized variables. I believe that none of these were causing problems, but it's polite to squash compiler warnings, so I went through and did it.
For local variables, this was just changing "int x;" to "int x=0;" in many places. (Not every single variable, just the ones that Xcode reported could possibly get used initialized.) (Xcode is pretty smart but errs on the conservative side, of course.)
That was easy. The harder case was the assembly_operand structure.
There are many, many places in the code that use the idiom
assembly_operand AO;
AO.type = foo; AO.value = bar; AO.marker = baz;
But the assembly_operand structure has five fields, including the (rarely-used) symtype and symflags. This means we do a lot of work on partially-initialized structures.
Again, I believe this has never caused problems. But it's not provably safe. So I added a macro INITAO(&AO) which zeroes all fields of the structure. Also INITAOT(&AO, type) and INITAOTV(&AO, type, value) which initialize one or two fields to given values. (The marker field is almost always zero, so I didn't bother with a third macro.)
Applying these macros touched a whole lot of code. You'll note that I have about twenty checkins covering this stuff. It made the source code a bit more compact, though – I was replacing an idiom of three statements with one or two statements.
While I was in there, I made a couple of other changes:
The old ASSERT_GLULX and ASSERT_ZCODE macros now compile to nothing. (They were safeguards I used during the original Glulx development process, but they're not really helpful today.)
I added a magic "_attribute((noreturn_))" to the error functions which call exit(). This is done through a NORETURN macro which is defined only for GCC and Clang. These compilers can error-check a bit better knowing that these calls are non-returning.
Steps to reproduce :
Additional information :
imported from: [Mantis 1707] Clean up lots of uninitialized structure and variable warnings