Enforce C11/C++11 on Clang, too. #998
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Otherwise, it faceplants on Darwin because of the lack of constexpr support.
Frenzie
reviewed
Oct 25, 2019
Makefile.defs
Outdated
| CSTD_FLAGS:=-std=c11 | ||
| CXXSTD_FLAGS:=-std=c++11 | ||
| else | ||
| # Keep GNU extensions enabled on Linux (mainly because the CI's Clang version sucks so very hard). |
There was a problem hiding this comment.
To be clear, that's Clang 8, the latest - 1. I'll probably update it to 9 sometime. Not some curious archeological find from the depths of Ubuntu 16.04. :-P
There was a problem hiding this comment.
Are you sure it's Clang 8? o_O.
FWIW, luajit builds just fine w/ c11 on Linux w/ Clang 9, and macOS w/ Xcode 11. -_-".
There was a problem hiding this comment.
Unless Ubuntu mangles LLVM/Clang beyond recognition, which wouldn't surprise me all that much ;).
There was a problem hiding this comment.
Quite sure:
$ docker run koreader/kobase-clang:0.1.2 clang --version
clang version 8.0.1-svn363027-1~exp1~20190611212422.76 (branches/release_80)
Target: x86_64-pc-linux-gnu
Thread model: posix
InstalledDir: /usr/bin
Also: #944
There was a problem hiding this comment.
Doesn't matter if Ubuntu does or doesn't; it comes straight from the horse's mouth:
There was a problem hiding this comment.
koreader/virdevenv#45 is ready to go
Just enforce current defaults. XCode works in mysterious ways, I guess. I'd be inclined to hide this behind a DARWIN check, but, it's currently harmless.
Appears to be smart enough not to downgrade a default/specified std, so, yay. c.f., https://cmake.org/cmake/help/latest/manual/cmake-compile-features.7.html
LLVM/CLang's doc status is... weird.
Frenzie
reviewed
Oct 25, 2019
| @@ -161,5 +155,7 @@ set (CRENGINE_SOURCES | |||
| ${CRE_DIR}/src/xxhash.c | |||
| ) | |||
| add_library(crengine SHARED ${CRENGINE_SOURCES}) | |||
| # Make sure we'll get C++11 support | |||
| target_compile_features(crengine PRIVATE cxx_constexpr) | |||
There was a problem hiding this comment.
Wow, that was the most helpful CMake man page I've ever seen.
Frenzie
reviewed
Oct 25, 2019
Makefile.defs
Outdated
| @@ -449,7 +449,7 @@ else | |||
| CSTD_FLAGS:=-std=c11 | |||
| CXXSTD_FLAGS:=-std=c++11 | |||
| else | |||
| # Keep GNU extensions enabled on Linux (mainly because the CI's Clang version sucks so very hard). | |||
| # Keep GNU extensions enabled on Linux, otherwise Clang < 9 fails to build LuaJIT. | |||
There was a problem hiding this comment.
But how do we know that's an accurate comment? :-P
There was a problem hiding this comment.
There was a problem hiding this comment.
Rather, I mean who says Clang 9 does?
There was a problem hiding this comment.
I do ;p.
clang -O2 -fomit-frame-pointer -Wall -std=c11 -march=native -pipe -O2 -fomit-frame-pointer -frename-registers -fweb -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -U_FORTIFY_SOURCE -DLUA_MULTILIB=\"lib\" -fno-stack-protector -c -o ljamalg.o ljamalg.c
clang-9: warning: optimization flag '-frename-registers' is not supported [-Wignored-optimization-argument]
clang-9: warning: optimization flag '-fweb' is not supported [-Wignored-optimization-argument]
gcc-ar rcus 2>/dev/null libluajit.a lj_vm.o ljamalg.o
clang -O2 -fomit-frame-pointer -Wall -std=c11 -march=native -pipe -O2 -fomit-frame-pointer -frename-registers -fweb -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -U_FORTIFY_SOURCE -DLUA_MULTILIB=\"lib\" -fno-stack-protector -Wl,-O1 -Wl,--sort-common -Wl,--hash-style=gnu -Wl,--as-needed -shared -fPIC -Wl,-soname,libluajit-5.1.so.2 -o libluajit.so lj_vm_dyn.o ljamalg_dyn.o -lm -ldl
gcc-ranlib libluajit.a
strip --strip-unneeded libluajit.so
clang -O2 -fomit-frame-pointer -Wall -std=c11 -march=native -pipe -O2 -fomit-frame-pointer -frename-registers -fweb -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -U_FORTIFY_SOURCE -DLUA_MULTILIB=\"lib\" -fno-stack-protector -Wl,-O1 -Wl,--sort-common -Wl,--hash-style=gnu -Wl,--as-needed -Wl,-E -o luajit luajit.o libluajit.a -lm -ldl
strip --strip-unneeded luajit
make[2]: Leaving directory '/var/tmp/niluje/luajit-2.0/src'
NiLuJe
added a commit
to NiLuJe/koreader
that referenced
this pull request
Oct 25, 2019
Pickup koreader/koreader-base#997 & koreader/koreader-base#998 The most user-visible fix being startup in non-standard orientations on the Libra no longer being broken ;).
NiLuJe
added a commit
to koreader/koreader
that referenced
this pull request
Oct 26, 2019
Pickup koreader/koreader-base#997 & koreader/koreader-base#998 The most user-visible fix being startup in non-standard orientations on the Libra no longer being broken ;).
mwoz123
pushed a commit
to mwoz123/koreader
that referenced
this pull request
Mar 29, 2020
Pickup koreader/koreader-base#997 & koreader/koreader-base#998 The most user-visible fix being startup in non-standard orientations on the Libra no longer being broken ;).
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.