Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cpp: cmake improvements #3996

Merged
merged 3 commits into from
Feb 19, 2023
Merged

Cpp: cmake improvements #3996

merged 3 commits into from
Feb 19, 2023

Conversation

Krzmbrzl
Copy link
Contributor

@Krzmbrzl Krzmbrzl commented Nov 30, 2022

Please see the individual commit messages for what changes were performed (and why)

Fixes #3993

In the top-level CMakeLists.txt, a minimum cmake version of 3.15 is
required. Therefore, all checks that were checking whether the cmake
version is >= 3.0 or 3.1 (etc.) are redundant as those will always be
true due to above requirement.

Signed-off-by: Robert Adam <[email protected]>
Setting any policy to the OLD behavior is almost always a bad idea in
the long run as the OLD behavior of cmake policies are deprecated by
definition and may be completely removed in a future cmake version.

The changes in cmake behavior introduced by this are
- Only auto-expand variables in if-expressions, if they are unquoted.
  Doing this differently just opens you up for hell to break loose.
  As far as I can tell, we don't make use of this behavior anyway.
- If referring to a non-existing target with get_target_properties,
  cmake will now produce an error. This is what one usually wants as
  referencing non-existing targets is simply a bug in your setup.
- cmake won't recognize "DEFINITIONS" as a built-in directory property.
  Instead, one has to use the (proper) "COMPILE_DEFINITIONS". Again,
  I was unable to find a location in which we use "DEFINITIONS", so the
  change should have no effect on us.
- "@rpath" can now be used in a target's install name on macOS. With the
  OLD behavior, this had to explicitly be enabled, while the new
  behavior is to enable this by default. In order to still remain at the
  old behavior, without depending on deprecated functionality, we now
  instead set CMAKE_MACOSX_RPATH to OFF, which should have the same
  effect.

Besides removing deprecated behavior, this has the nice side effect of
removing the warnings that were emitted when building the cpp runtime
with cmake.

Signed-off-by: Robert Adam <[email protected]>
The default behavior is left unchanged (build both) but now users can
choose to optionally only build one of the two variants.
Due to macro-magic, both variants had to be compiled separately and
therefore building both variants really does compile everything twice.
Therefore, disabling the version that one is not interested in can cut
down compilation time significantly.

Fixes antlr#3993

Signed-off-by: Robert Adam <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants