-
Notifications
You must be signed in to change notification settings - Fork 13.4k
Migrate to LLVM{Get,Set}ValueName2 #67033
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Conversation
The deprecated `LLVM{Get,Set}ValueName` only work with NUL-terminated strings, but the `2` variants use explicit lengths, which fits better with Rust strings and slices. We now use these in new helper functions `llvm::{get,set}_value_name` that convert to/from `&[u8]`.
@@ -774,7 +774,8 @@ extern "C" { | |||
pub fn LLVMIsAGlobalVariable(GlobalVar: &Value) -> Option<&Value>; | |||
pub fn LLVMAddGlobal(M: &'a Module, Ty: &'a Type, Name: *const c_char) -> &'a Value; | |||
pub fn LLVMGetNamedGlobal(M: &Module, Name: *const c_char) -> Option<&Value>; | |||
pub fn LLVMRustGetOrInsertGlobal(M: &'a Module, Name: *const c_char, T: &'a Type) -> &'a Value; | |||
pub fn LLVMRustGetOrInsertGlobal(M: &'a Module, Name: *const c_char, NameLen: size_t, | |||
T: &'a Type) -> &'a Value; |
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.
FWIW, I think there's an opportunity to convert a lot more of these LLVMRust
wrappers to pointer+len strings, using StringRef
on the actual C++ LLVM call, but for now I only changed a couple that were involved with the ValueName
calls.
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.
Agreed. I also wouldn't object if you did the rest too but it's not necessary.
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.
I'll probably work on those incrementally, in separate PRs.
Thanks! @bors r+ |
📌 Commit 16d2178 has been approved by |
Migrate to LLVM{Get,Set}ValueName2 The deprecated `LLVM{Get,Set}ValueName` only work with NUL-terminated strings, but the `2` variants use explicit lengths, which fits better with Rust strings and slices. We now use these in new helper functions `llvm::{get,set}_value_name` that convert to/from `&[u8]`. Closes rust-lang#64223. r? @rkruppe
Rollup of 11 pull requests Successful merges: - #66846 (Make try_mark_previous_green aware of cycles.) - #66959 (Remove potential cfgs duplicates) - #66988 (Fix angle bracket formatting when dumping MIR debug vars) - #66998 (Modified the testcases for VxWorks) - #67008 (rustdoc: Add test for fixed issue) - #67023 (SGX: Fix target linker used by bootstrap) - #67033 (Migrate to LLVM{Get,Set}ValueName2) - #67049 (Simplify {IoSlice, IoSliceMut}::advance examples and tests) - #67054 (codegen "unreachable" for invalid SetDiscriminant) - #67081 (Fix Query type docs) - #67085 (Remove boxed closures in address parser.) Failed merges: r? @ghost
The deprecated
LLVM{Get,Set}ValueName
only work with NUL-terminatedstrings, but the
2
variants use explicit lengths, which fits betterwith Rust strings and slices. We now use these in new helper functions
llvm::{get,set}_value_name
that convert to/from&[u8]
.Closes #64223.
r? @rkruppe