CMake update 2/3: lift platform restriction #758
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
With the module search path cleverly handled with different install.h configurations, the genstatic script inserted absolute paths into the generated clib.c file. This didn't fail on Windows CI as this is an in-source build. For out-of-source builds, it's crucial that clib.c can refer to both .c files in the source directory and those generated in the build directory. As a fix, the genstatic invocation now uses the -I flag. This patch also improves the handling of include paths to find the .c. files mentioned above by trimming down the scope of this property to clib.c only. Also, there is no need to manually tell the preprocessor where to look for generated .c, as they live relative to clib.c anyhow.
chibi/install.h is included in C source files, and providing a different install.h upon actual installation is inconsistent and dangerous. When working with a chibi executable within the build tree (i.e., not an installed executable), the CHIBI_MODULE_PATH environment tweak can be used to not always specify -I paths on the command line.
okuoku
reviewed
Jul 20, 2021
Yeah, sanitiser builds are interesting. This is what I get on MacOS for an ordinary startup - certainly some things that might be worth addressing. Should I raise an issue for this? Sanitizer output |
I'd like to defer to @ashinn . Overflows: In other Schemes, Chicken uses Unaligned access: I assume these are portability issue. ... but it's true it won't cause any issue on modern systems aside microcontrollers. |
|
Regarding the unaligned read warnings, this can be removed by setting SEXP_USE_ALIGNED_BYTECODE. It defaults to true on platforms where this is required (arm, sparc, etc.), but will be false on x86 where these aren't really valid warnings. @okuoku, I'll leave merging this to you when ready. |
That's true, no such warnings with this flag. I wasn't aware that x86 has fewer alignment rules compared to other architectures. Could you point me to some reference where this is explained? I'll raise an issue with the remaining warnings; this might be worth having a look at. |
okuoku
approved these changes
Jul 21, 2021
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.
Follow-up PR for the CMake modernization. This enables a CMake build for Unix-based systems. A particular emphasis lies on the seamless configuration of either statically or dynamically linked build artifacts: With these commands;
shared libraries are compiled, and the standard chibi executable is used to generate .c files from the stubs. With
a minimal bootstrap executable is built for the .stub to .c and the clib.c generation. This can be mixed with further common CMake options as -DCMAKE_BUILD_TYPE=Release or certain chibi options such as -DSEXP_USE_BOEHM=1.
There is not much else going on here. The main benefits of this are