Skip redefinition of bool if running GCC >= 15. #511
Conversation
|
Checking for GCC explicitly isn't the right way to do this, as this problem will affect any compiler in C23 mode. You can use |
|
I agree with Adam. The other reason not to make it a GCC version check is that it would do the workaround in new GCC even if the compile is done with an older C version requested. |
|
Thanks, @atsampson, @pkoning2. My approach was rather crude - I was not sure what the extension of the issue would be so I tried to keep the blast radius minimal. On the bug report, @markpizz pointed out this only happens when using the cmake scripts and SimH compiles correctly with GCC 15 on the makefile flow. |
|
@rbanffy @pkoning2 @atsampson Look at PR #485. I think I've already fixed this issue in a prior PR. |
|
@rbanffy: It's not an issue specific to GCC 15. A slightly better test is |
|
@rbanffy @atsampson @pkoning2 It works on the |
It looks like it solves it (I'm no expert in CMake), but it is a considerably large PR. It might be worth to extract just the CMake C99 fix into a small PR that's easier to validate. Can you do it @bscottm? |
|
As stated earlier in this thread by @pkoning2 the cmake build (i.e. a change of the build tool chain which uses the same compilers) should not require changes to the codebase. The problem is in the cmake build logic. Burying such a fix in the middle of a ton of other changes to the codebase, which by themselves need review and critical justification is not a reasonable approach. Precisely in PR #485 you haven't demonstrated any case where the proposed changes actually fixes a bug or where the original code didn't produce the results original author intended. You just list changes to some 41 source code across the many simulators you want to make because if you were writing it today, you'd do it differently. |
@pkoning2 @markpizz: It's not the build logic and it isn't a CMake issue, per se. It's a configuration management issue: I specifically do not set dialect-specific flags. I do not nail the configuration to C99, C11 or C23. The codebase should flexibly adapt to the dialect. PR #485 adapts the codebase so that open-simh compiles with C11 and C23 compilers natively. No funky workarounds in the build system. No burning a case of red and blank candles in the presnce of a 3D pentagram or requiring animal sacrifices. Natively. |
@rbanffy This isn't a CMake issue. It was a deliberate choice on my part to not limit the C dialect so that C11 and C23 features could be used in the future. The choice I made was for maximal flexibility, not restraint. The There is no C99-specific part to the patch. What PR #485 does is determine how to The patch is large for two reasons. First, there were places where |
You admit to deliberately choosing to have the cmake build not tolerate the C language standard that the simh codebase is specifically using. Therefore, your many rewrites of fully working code will then be fantastic. I'm not personally wallowing in the newer details of C11 and C23, but, curiously, which of these newly standardized code constructs (absolutely not used in the simh codebase, only in your rewrites of working code) actually fail to compile with the standard is specified as C99?
Once again, you argue that code changes need to be made to conform to the new standard you want to force, but you absolutely never provide any demonstration of code you want to change which (in it's unchanged form) does not perform exactly how the many original authors intended each case to work. That is your changes don't fix anything that is broken. |
|
I absolutely agree we might want to use more recent standards, but that
should be something that is done intentionally, on all build processes,
first as a change that only fixes code that breaks with the new C standard,
then, incrementally, to migrate code that still compiles under the new
standard and that would be more readable under the new one and, it'd be
nice, add unit tests for anything that will change and that might be
missing so we are sure the code is correct. There are some broken things on
the Github Actions part I can take a look at unless someone already knows
what the problem is.
At that point, it might be worth adding explicit target standards,
supported compilers and build systems to the README doc. At least, and it's
my guess here, we want to run under Windows, Linux and macOS and compile
under GCC and clang.
…On Thu, Oct 30, 2025 at 10:40 PM Mark Pizzolato ***@***.***> wrote:
*markpizz* left a comment (open-simh/simh#511)
<#511 (comment)>
@rbanffy <https://github.com/rbanffy> This isn't a CMake issue. It was a
deliberate choice on my part to not limit the C dialect so that C11 and C23
features could be used in the future. The choice I made was for maximal
flexibility, not restraint. The makefile specifically sets -std=gnu99 or
-std=c99.
You admit to deliberately choosing to have the cmake build not tolerate
the C language standard that the simh codebase is specifically using.
Therefore, your many rewrites of fully working code will then be fantastic.
I'm not personally wallowing in the newer details of C11 and C23, but,
curiously, which of these newly standardized code constructs (absolutely
not used in the simh codebase, only in your rewrites of working code)
actually fail to compile with the standard is specified as C99?
The patch is large for two reasons. First, there were places where t_bool,
int and int32 were used interchangeably and the code had to be fixed to use
the correct type. Second, where t_bool is used, I ensure that the
assignment expressions are Boolean expressions so that if I did introduce a
new issue, I reduce the risk that it was actually related to t_bool changes.
Once again, you argue that code changes need to be made to conform to the
new standard you want to force, but you absolutely never provide any
demonstration of code you want to change which (in it's unchanged form)
does not perform exactly how the many original authors intended each case
to work. That is your changes don't fix anything that is broken.
—
Reply to this email directly, view it on GitHub
<#511 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AABNDCWY4OHFCQPEU7Z3HIL32KHU5AVCNFSM6AAAAACKEEEAW6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZTINZQGUYDQNJUGA>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
--
Ricardo Bánffy
http://about.me/rbanffy
|
|
@rbanffy I've submitted a PR for the current CI/CD issue with macOS 13 and the windows-latest image that no longer support NSIS "exe" installers (I'm not sure that NSIS ".exe" installers are a great idea, but it was an option.) Also, there's a conflicting typedef with Windows in the LINC simulator code. Maintenance and modernization help is always welcome. My perspective is that open-simh should accommodate the newer C standards, providing backward compatibility insofar as that is feasible. I'm not in favor of locking the code to some configuration because I've seen this backfire spectacularly. Like PCRE2, this comes down to a "sooner or later" choice. |
5 participants
For c23,
boolis a keyword and can't be redefined. This skips the redefinition if we are compiling with GCC >= 15 (default on Fedora 42).