Skip to content

Remove unnecessary LLVMRustPersonalityFn binding #39319

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

Merged
merged 1 commit into from
Feb 3, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/librustc_llvm/ffi.rs
Original file line number Diff line number Diff line change
Expand Up @@ -804,7 +804,7 @@ extern "C" {
Name: *const c_char)
-> ValueRef;
pub fn LLVMRustAddHandler(CatchSwitch: ValueRef, Handler: BasicBlockRef);
pub fn LLVMRustSetPersonalityFn(B: BuilderRef, Pers: ValueRef);
pub fn LLVMSetPersonalityFn(Func: ValueRef, Pers: ValueRef);

// Add a case to the switch instruction
pub fn LLVMAddCase(Switch: ValueRef, OnVal: ValueRef, Dest: BasicBlockRef);
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_trans/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1107,7 +1107,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {

pub fn set_personality_fn(&self, personality: ValueRef) {
unsafe {
llvm::LLVMRustSetPersonalityFn(self.llbuilder, personality);
llvm::LLVMSetPersonalityFn(self.llfn(), personality);
}
}

Expand Down
4 changes: 3 additions & 1 deletion src/librustc_trans/mir/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -319,7 +319,9 @@ pub fn trans_mir<'a, 'tcx: 'a>(
mircx.cleanup_kinds.iter_enumerated().map(|(bb, cleanup_kind)| {
if let CleanupKind::Funclet = *cleanup_kind {
let bcx = mircx.get_builder(bb);
bcx.set_personality_fn(mircx.ccx.eh_personality());
unsafe {
llvm::LLVMSetPersonalityFn(mircx.llfn, mircx.ccx.eh_personality());
}
Copy link
Member

Choose a reason for hiding this comment

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

... am I right in inferring from this change that bcx.llfn() is not the same as mircx.llfn? It seems surprising.

And If that is actually true... it probably could use a comment.

Copy link
Member Author

Choose a reason for hiding this comment

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

It is the same. It just avoids going through LLVM to get the information that’s already available here.

Copy link
Member

Choose a reason for hiding this comment

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

Okay. I would personally have left it alone, but I don't care enough to block this PR.

if base::wants_msvc_seh(ccx.sess()) {
return Some(Funclet::new(bcx.cleanup_pad(None, &[])));
}
Expand Down
8 changes: 0 additions & 8 deletions src/rustllvm/RustWrapper.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1082,14 +1082,6 @@ extern "C" void LLVMRustAddHandler(LLVMValueRef CatchSwitchRef,
#endif
}

extern "C" void LLVMRustSetPersonalityFn(LLVMBuilderRef B,
LLVMValueRef Personality) {
#if LLVM_VERSION_GE(3, 8)
unwrap(B)->GetInsertBlock()->getParent()->setPersonalityFn(
cast<Function>(unwrap(Personality)));
#endif
}

#if LLVM_VERSION_GE(3, 8)
extern "C" OperandBundleDef *LLVMRustBuildOperandBundleDef(const char *Name,
LLVMValueRef *Inputs,
Expand Down