drop a few unsupported CFLAGS for clang #1090
Conversation
Tests passed ✓, Code: 17116 B (+0.0%), Stack: 1448 B (+0.0%), Structs: 812 B (+0.0%)
|
|
Hi @yamt, thanks for the PR, sorry about the late response. I'm not so sure on this one. When they break, It also breaks stack measurement, though there's no way around that... Thoughts on |
Actually, maybe NO_CALLGRAPH or NO_SU, NO_STACK could be interpreted weirdly (and NO_CI would be even worse!) |
on macOS, clang has a few names including "gcc" and "cc". spacetanuki% /usr/bin/gcc --version
Apple clang version 17.0.0 (clang-1700.0.13.3)
Target: x86_64-apple-darwin24.4.0
Thread model: posix
InstalledDir: /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin
spacetanuki% /usr/bin/cc --version
Apple clang version 17.0.0 (clang-1700.0.13.3)
Target: x86_64-apple-darwin24.4.0
Thread model: posix
InstalledDir: /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin
spacetanuki% /usr/bin/clang --version
Apple clang version 17.0.0 (clang-1700.0.13.3)
Target: x86_64-apple-darwin24.4.0
Thread model: posix
InstalledDir: /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin
spacetanuki% |
Ah... I get it, but if you force one compiler to cosplay as another you should probably expect some weird errors... I don't like it, but this does seem like the only way for |
earlier versions of xcode had a real gcc.
i don't like it either. |
Oh no! Anything but cmake!
After sleeping on it, I think you're right. This does seem like the only route for Will bring this in on the next patch release. Thanks for the PR! |
geky
added
next minor
needs minor version
|
Bumping to needs-minor. Not sure if Makefile-only changes need a minor release, but there will probably be a minor release soon. |
|
I've also added the NO_GCC variable. This should still be implicitly set if --version contains "clang", but allows other, non-gcc, users to explicitly disable the GCC-specific flag. Let me know if this tweak causes any problems on your end. |
Tests passed ✓, Code: 17116 B (+0.0%), Stack: 1448 B (+0.0%), Structs: 812 B (+0.0%)
|
Makefile
Outdated
| @@ -18,6 +18,12 @@ VALGRIND ?= valgrind | |||
| GDB ?= gdb | |||
| PERF ?= perf | |||
|
|
|||
| # guess clang or gcc (clang sometimes masquerades as gcc because of | |||
| # course it does) | |||
| ifeq ($(shell $(CC) --version | grep clang),clang) | |||
There was a problem hiding this comment.
i don't think this works because the line contains more characters than just clang.
spacetanuki% cc --version | grep clang
Apple clang version 17.0.0 (clang-1700.0.13.3)
spacetanuki% 
There was a problem hiding this comment.
AH, I was trying to avoid the double-negative and thought this was make's findstring. Reverted to ifneq. Good catch!
And actually tested with CC=clang this time..
This is the same as the implicit Clang => NO_GCC behavior introduced by yamt, but with an explicit variable that can be assigned by users using other, non-gcc, compilers: $ NO_GCC=1 make Note, stack measurements are currently GCC specific: $ NO_GCC=1 make stack ... snip ... FileNotFoundError: [Errno 2] No such file or directory: 'lfs.ci' make: *** [Makefile:494: lfs.stack.csv] Error 1
Thanks to yamt, GCC-specific flags should now be disabled if compiling with clang. Dropping the explicit flags also doubles as a test that the NO_GCC inference works.
|
I also just remembered we already have Clang in CI, to catch Clang-specific warnings, but it was implemented using explicit CFLAGS. I dropped the explicit CFLAGS so now this inference should actually be tested (which would've caught my above mistake). Let me know if anything else looks wrong. |
Tests passed ✓, Code: 17116 B (+0.0%), Stack: 1448 B (+0.0%), Structs: 812 B (+0.0%)
|
looks fine to me. thank you. |
No description provided.