ksh93 / ksh

ksh 93u+m: KornShell lives! | Latest release: https://github.com/ksh93/ksh/releases
Eclipse Public License 2.0
171 stars 30 forks source link

[portability] Make AST code aliasing clean and use -fstrict-aliasing with ISO C17 aliasing rules #760

Open gisburn opened 5 days ago

gisburn commented 5 days ago

Make AST code aliasing clean and use -fstrict-aliasing + -Wstrict-aliasing=3. Many mondern compilers don't have something like -fno-strict-aliasing, and just use ISO C11/C17 aliasing rules.

McDutchie commented 5 days ago

But this code base is written in C90 (although it does conditionally use a couple of features from later standards, subject to iffe feature tests). I don't know what the "many" modern compilers are that don't support -fno-strict-aliasing but hopefully they will support -std=c90 which should also solve it.

By the way, I don't fully understand strict aliasing rules yet. I've tried to learn about them, but the information I can find via Google seems to consist mostly of a lot of hand waving and precious little in the way of actual information. If anyone has any good quality pointers for learning about them, I'd appreciate that.

However, one thing I have learned over the years of working with this code base is that, just because a compiler throws a warning, does not mean it's actually a problem. I have seen far too many spurious warnings to take them at face value. Many/most of the warnings that were thrown before @JohnoKing added the -fno-strict-aliasing flag said that this or that may violate strict aliasing. But I don't actually know if they really do or not.

I am not opposed to making this code compliant with strict aliasing, but I also don't want to spend a lot of time on it myself. All of my available time and energy is taken up fixing ksh bugs. I'd like to get around to developing some new features but even that is not happening, because I keep finding more bugs.

So, if you feel strongly that this should happen, I'm afraid you are going to have to put in the work, or get someone else to put in the work. I would be happy to review patches and PRs.