-
Notifications
You must be signed in to change notification settings - Fork 12.9k
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
Update Rust LLVM bindings for LLVM 5.0 #43387
Conversation
(rust_highfive has picked a reviewer for you, use r? to override) |
src/librustc_trans/back/write.rs
Outdated
@@ -307,7 +307,7 @@ pub struct CodegenContext<'a> { | |||
} | |||
|
|||
struct HandlerFreeVars<'a> { | |||
llcx: ContextRef, | |||
_llcx: ContextRef, |
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.
This field is no longer used. I left it for the moment in case it is needed again in the future, however I can remove it if desired.
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.
Eh seems fine to me to go ahead and remove
SizeInBits, AlignInBits, Name)); | ||
SizeInBits, AlignInBits, | ||
#if LLVM_VERSION_GE(5, 0) | ||
/* DWARFAddressSpace */ None, |
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.
This is what the go bindings do, I don't know if we want the ability to set this from rust? Even if so I guess it currently isn't used so that functionality can be added later if desired.
When I helped with the last upgrade, we were able to drop the support for the oldest version of LLVM. I'm not sure what the policy is there, or if would even help, but just as a FYI. |
Mh, I think the oldest we still support is 3.9 and AFAIK we'll continue to support that for some time. |
Updated with the attributes changes from @alexcrichton from TimNN#1, thanks a lot! |
@TimNN want to throw in alexcrichton@b3e91ec which should fix archive creation as well? |
All looks great to me! Feel free to r=me once it's working locally for you |
@bors: r+ I believe this covers at least all the rustllvm stuff at least! |
📌 Commit fb217af has been approved by |
@bors: r+ |
📌 Commit 38e40ce has been approved by |
Update Rust LLVM bindings for LLVM 5.0 This is the initial set of changes to update the rust llvm bindings for 5.0. The llvm commits necessitating these changes are linked from the tracking issue, #43370.
☀️ Test successful - status-appveyor, status-travis |
This is the initial set of changes to update the rust llvm bindings for 5.0. The llvm commits necessitating these changes are linked from the tracking issue, #43370.