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

Improve documentation of Rust version related variables #123

Merged
merged 3 commits into from
Jan 17, 2022

Conversation

jschwe
Copy link
Collaborator

@jschwe jschwe commented Jan 7, 2022

  • Document the version related variables that corrosion (FindRust) provides
  • Improve the Rust_IS_NIGHTLY variable by always defining it (allows testing wether corrosion is new enough to set this variable)
  • Add variables to expose the LLVM version used by rust. Useful e.g. for cross-language lto, where you want to be sure that the llvm version on both sides matches.

jschwe added 2 commits January 7, 2022 14:09
It may be useful to know the LLVM version in CMake, e.g. if one wants to
use cross-language LTO to ensure that the same version of LLVM is used
on both sides.

Signed-off-by: Jonathan Schwender <[email protected]>
Explicitly setting the variable to 0 allows users to check if corrosion
defines the variable at all. This can be used to test if the corrosion
version in use supports Rust_IS_NIGHTLY or needs to be updated.

Signed-off-by: Jonathan Schwender <[email protected]>

Improve documentation

Signed-off-by: Jonathan Schwender <[email protected]>
@jschwe jschwe requested a review from ogoffart January 7, 2022 13:16
Conditionally enabling some code depending on the rust version is pretty
 common, so it makes sense to document these variables.

Signed-off-by: Jonathan Schwender <[email protected]>
@jschwe jschwe force-pushed the jschwe/llvm-version branch from f1cfd33 to 81213a0 Compare January 7, 2022 13:32
@@ -231,6 +231,8 @@ if (_RUSTC_VERSION_RAW MATCHES "rustc ([0-9]+)\\.([0-9]+)\\.([0-9]+)(-nightly)?"
set(Rust_VERSION "${Rust_VERSION_MAJOR}.${Rust_VERSION_MINOR}.${Rust_VERSION_PATCH}")
if(CMAKE_MATCH_4)
set(Rust_IS_NIGHTLY 1)
else()
set(Rust_IS_NIGHTLY 0)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will that cause problem for people doing
if(Rust_IS_NIGHTLY) or is 0 falsy?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, 0 evaluates to false. From the documentation

False if the constant is 0, OFF, NO, FALSE, N, IGNORE, NOTFOUND, the empty string, or ends in the suffix -NOTFOUND

@@ -248,6 +250,18 @@ else()
)
endif()

if (_RUSTC_VERSION_RAW MATCHES "LLVM version: ([0-9]+)\\.([0-9]+)\\.([0-9]+)")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do users need to care about the LLVM version.
I guess you added that because you have an usecase.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My usecase is cross-language LTO. It requires that the LLVM version on both sides matches, so I need to know the version of LLVM used by rust.

@jschwe
Copy link
Collaborator Author

jschwe commented Jan 17, 2022

@ogoffart Are there still any open points from you, or could I go ahead and merge this?

@ogoffart ogoffart merged commit 85ef7e9 into corrosion-rs:master Jan 17, 2022
@ogoffart
Copy link
Collaborator

No more open points. Thanks for the patch

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants