This is an archive of the discontinued LLVM Phabricator instance.

Use unique_ptr to hold AsmInfo,MRI,MII,STI
ClosedPublic

Authored by MaskRay on Sep 21 2018, 9:08 PM.

Diff Detail

Event Timeline

MaskRay created this revision.Sep 21 2018, 9:08 PM
dblaikie added inline comments.
include/llvm/Target/TargetMachine.h
87–90

Should these be unique_ptr<const T> like the original that was const T*?

lib/Target/TargetMachine.cpp
43

Could use = default here?

MaskRay updated this revision to Diff 166808.Sep 24 2018, 10:35 PM

Incorporate review suggestions by dblaikie

MaskRay marked 2 inline comments as done.Sep 24 2018, 10:37 PM
MaskRay added inline comments.
lib/Target/TargetMachine.cpp
43

In my understanding about an out-of-line destructor, it does not matter if {} or = default is used, either will make the destructor non-trivial. But I agree with you in that = default; looks better.

MaskRay updated this revision to Diff 166809.Sep 24 2018, 10:38 PM
MaskRay marked an inline comment as done.
MaskRay added a reviewer: dblaikie.
MaskRay removed a subscriber: dblaikie.

Update reviewers line with arc diff --verbatim (see if this command works)

dblaikie accepted this revision.Sep 24 2018, 10:55 PM

looks good - thanks!

(& yeah, for an out of line ctor, I don't think there's any difference in the codegen (or there shouldn't be) between = default and {}, but as you say, looks a bit better/makes it more explicit (to other readers and the compiler that there's nothing interesting going on, etc)

This revision is now accepted and ready to land.Sep 24 2018, 10:55 PM
This revision was automatically updated to reflect the committed changes.