peter-winter / ctpg

Compile Time Parser Generator is a C++ single header library which takes a language description as a C++ code and turns it into a LR1 table parser with a deterministic finite automaton lexical analyzer, all in compile time.
MIT License
456 stars 24 forks source link

Fix up CMake build and CI #33

Closed alexreinking closed 2 years ago

alexreinking commented 2 years ago

There were a number of issues with the CMake build before. This PR fixes all of them. They are:

  1. There was no way to disable warnings-as-errors.
  2. The standard BUILD_TESTING variable wasn't honored.
  3. FetchContent and add_subdirectory users got the project's install rules by default.
  4. CI didn't build the examples (and they didn't have a CMakeLists.txt to begin with).
  5. The project searched for a C compiler it never used (C++ only)
  6. The path for installing the generated CMake package files wasn't independently configurable (important for Debian, vcpkg, and surely others)
  7. The project would incorrectly find Catch2 version 3, when only 2 is supported.
  8. The package set a variable for the include path.
  9. The language example unnecessarily used include_directories.
  10. The usage of #include <ctpg.hpp> and #include <ctpg/ctpg.hpp> was inconsistent.
  11. There was no ctpg::ctpg alias for FetchContent and add_subdirectory users.

Happy to work with you to make these changes more palatable.

peter-winter commented 2 years ago

Also a question: the examples are compiled with the increased warnings options? I'm asking because apparently some warnings pop out on windows build when you trned on examples build.

peter-winter commented 2 years ago

I fixed several warnings on windows compilation of the examples. Unfortunately the changes conflict with your PR, but these are easy fixes.

alexreinking commented 2 years ago

I rebased my PR on the latest master branch. I have also bumped the CMake version to 3.14 and enabled warnings on the examples. However, there are warnings on G++-10, which causes CI to fail. How would you like to proceed?

peter-winter commented 2 years ago

I'm preparing a commit to remove the warnings from the ctjs example. You'll rebase on that.

alexreinking commented 2 years ago

I'm preparing a commit to remove the warnings from the ctjs example. You'll rebase on that.

Sounds good. Ping me when it's ready.

peter-winter commented 2 years ago

GCC shouldbe fine, unfortunately MSVC gives some ridiculous warnings that some of the difinitions in library hide global ones in the examples, but in fact library is included first. So if I have a definition in some nested namespace called 'number' I cannot have a definition of 'number' in the example. See if you have the same issue on MSVC. If so I can't see any other solution than removing pedantic warnings from examples.

alexreinking commented 2 years ago

I rebased and now Clang 12 doesn't like an unused variable in ctjs

peter-winter commented 2 years ago

Made another fix. Sorry.

peter-winter commented 2 years ago

I think this shadowing warning fom msvc is a comiler bug. This would make sense if I shadowed some library definition by a definition in an example. This is the opposite, definition in library (which is compiler first) shadows a global in later code. Disabling the warning is a good idea.

peter-winter commented 2 years ago

Fixed the clang warning in custom_lexer.cpp. Please rebase.

alexreinking commented 2 years ago

Done. Looks like disabling the one warning was sufficient for the examples.

peter-winter commented 2 years ago

@JPenuchot can you update the AUR package?

JPenuchot commented 2 years ago

Done !