This is an archive of the discontinued LLVM Phabricator instance.

[bindings/go] Add EraseFromParent
ClosedPublic

Authored by aykevl on Sep 29 2018, 9:12 AM.

Details

Reviewers
pcc
whitequark
Summary

After using ReplaceAllUsesWith on an instruction, it may be necessary to erase it even though it is dead.

Diff Detail

Repository
rL LLVM

Event Timeline

aykevl created this revision.Sep 29 2018, 9:12 AM
whitequark added inline comments.Sep 29 2018, 9:13 AM
bindings/go/llvm/ir.go
1213

Nit: I'd call this EraseInstructionFromParent. (Also I'd deprecate EraseFromParent and rename it to EraseBasicBlockFromParent, but I'm not the Go bindings code owner so it's not my call.)

aykevl added inline comments.Sep 29 2018, 9:20 AM
bindings/go/llvm/ir.go
1213

Yes, that sounds better but I've kept it consistent with EraseFromParentAsFunction and EraseFromParentAsGlobal. Should it be changed, then?

aykevl added inline comments.Oct 6 2018, 4:01 AM
bindings/go/llvm/ir.go
1213

ping?

whitequark accepted this revision.Oct 29 2018, 12:00 AM
whitequark added inline comments.
bindings/go/llvm/ir.go
1213

I'm not the Go bindings code owner so it seems better to go with the conservative naming.

This revision is now accepted and ready to land.Oct 29 2018, 12:00 AM
aykevl marked an inline comment as done.Jan 6 2019, 1:19 PM

Can someone merge this patch? I don't have commit access.

bindings/go/llvm/ir.go
1213

Do you know who the owner is, then? @pcc? I can't find anyone closer in the CODE_OWNERS.TXT file.

aykevl added a comment.Apr 4 2019, 3:29 PM

Ping?
I don't have commit access.

Herald added a project: Restricted Project. · View Herald TranscriptApr 4 2019, 3:30 PM

@aykevl I think you can ask for commit access, you've had more than a few patches merged.

aykevl closed this revision.EditedJun 8 2019, 2:59 PM

Asked for commit access and merged this change.