-
Notifications
You must be signed in to change notification settings - Fork 13k
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
Extend preprocessor LLVM version checks to support LLVM 4.x #36742
Conversation
r? @aturon (rust_highfive has picked a reviewer for you, use r? to override) |
cc #36295 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @shepmaster! Looks great to me.
@@ -45,7 +45,16 @@ | |||
#include "llvm-c/ExecutionEngine.h" | |||
#include "llvm-c/Object.h" | |||
|
|||
#if LLVM_VERSION_MINOR >= 7 | |||
#define LLVM_VERSION_GT_OR_EQ(major, minor) \ | |||
LLVM_VERSION_MAJOR > (major) || LLVM_VERSION_MAJOR == (major) && LLVM_VERSION_MINOR >= (minor) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could this entire clause also be wrapped in parens? (yay C macros)
same below as well
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done!
Someone should really work on creating a replacement for this C language thing.
@@ -45,7 +45,16 @@ | |||
#include "llvm-c/ExecutionEngine.h" | |||
#include "llvm-c/Object.h" | |||
|
|||
#if LLVM_VERSION_MINOR >= 7 | |||
#define LLVM_VERSION_GT_OR_EQ(major, minor) \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps this could be LLVM_VERSION_GE
?
Similar to ParialOrd::ge
naming at least
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done!
This doesn't actually do anything for LLVM 4.x yet, but sets the stage.
c7341e5
to
e6e117c
Compare
@bors: r+ |
📌 Commit e6e117c has been approved by |
…richton Extend preprocessor LLVM version checks to support LLVM 4.x This doesn't actually do anything for LLVM 4.x yet, but sets the stage.
…richton Extend preprocessor LLVM version checks to support LLVM 4.x This doesn't actually do anything for LLVM 4.x yet, but sets the stage.
This doesn't actually do anything for LLVM 4.x yet, but sets the stage.