| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
There was a problem hiding this comment.
LGTM, thanks. Can you check if there is anything else thats stale in this MakeFile?
Sorry, something went wrong.
|
@aaronj0 I cannot see anything taking a quick look at the ci. If you update the reusable action here https://github.com/compiler-research/ci-workflows/blob/main/actions/build-and-test-cppjit/action.yml to do the cmake build with CMAKE_COMPILE_WARNING_AS_ERROR=ON you should catch any leftover warnings (if there is any) when building cppjit. To catch them when building the tests you'll want to add CXXFLAGS="-Werror" to the make command (and maybe a linker flag since I don't think this catches linker warnings). To match Clad, xeus-cpp, CppInterOp I think it would make sense to remove the handwritten makefile and replace it with a cmake file which the main cmake can call, but that is just personal choice. You could also change it so the tests are built with Wall like the cppjit library for some extra checks. You will need to merge this PR if happy with it, since I don't have permissions to do anything in this repo. |
Sorry, something went wrong.
That sounds like a good idea, but we should probably first inject the flag in the CMakeLists.txt and test MakeFile here and see what fails. We can then address the remaining issues as a part of this PR, before making that default in the ci-workflows yml.
Regarding moving to a CMake file, that sounds like a good idea, but perhaps the simplicity of this MakeFile is desired :) |
Sorry, something went wrong.
|
I have added Werror and Wextra to the build of cppjit, and Wall, Werror and Wextra to the tests. These have raised some new warnings. I will make my way through them, but I might not start until next week. |
Sorry, something went wrong.
Sounds good, thank you! |
Sorry, something went wrong.
| Back | FazBrowse Home | New Git URL |
If you look at the nightly builds (here for example https://github.com/compiler-research/cppjit/actions/runs/32926841974/job/98051372290#step:8:695), you'll see that the single_module is labelled as obsolete, so this PR removes the flag.