This is an archive of the discontinued LLVM Phabricator instance.

[RS4GC] Cosmetic cleanup, NFC
ClosedPublic

Authored by sanjoy on Oct 1 2015, 6:36 PM.

Details

Summary

A series of cosmetic cleanup changes to RewriteStatepointsForGC:

  • Rename variables to LLVM style
  • Remove some redundant asserts
  • Remove an unsued Pass * parameter
  • Remove unnecessary variables
  • Use C++11 idioms where applicable
  • Pass CallSite by value, not reference

Diff Detail

Event Timeline

sanjoy updated this revision to Diff 36325.Oct 1 2015, 6:36 PM
sanjoy retitled this revision from to [RewriteStatepointsForGC] Cosmetic cleanup, NFC.
sanjoy updated this object.
sanjoy added reviewers: reames, swaroop.sridhar.
sanjoy added a subscriber: llvm-commits.

Why is CallSite passed by value?
Everything else looks good to me. Thanks.

sanjoy updated this revision to Diff 36571.Oct 5 2015, 5:18 PM
sanjoy edited edge metadata.
  • squash some more NFC cleanup work into this patch
sanjoy added a comment.Oct 5 2015, 5:31 PM

Why is CallSite passed by value?

Passing CallSites by value is really cheap (as cheap as copying a pointer), so there is no benefit in passing them by reference unless the CallSite is an out parameter. The downside of passing them by (non-const) reference is that it is harder to tell if the caller intends to modify the CallSite referred to, and passing by const reference is more typing. :)

From CallSite.h:

// NOTE: These classes are supposed to have "value semantics". So they should be
// passed by value, not by reference; they should not be "new"ed or "delete"d.
// They are efficiently copyable, assignable and constructable, with cost
// equivalent to copying a pointer (notice that they have only a single data
// member). The internal representation carries a flag which indicates which of

Everything else looks good to me. Thanks.

FYI: the updated version has some more (still NFC) cleanup work.

swaroop.sridhar accepted this revision.Oct 5 2015, 5:56 PM
swaroop.sridhar edited edge metadata.
This revision is now accepted and ready to land.Oct 5 2015, 5:56 PM
sanjoy retitled this revision from [RewriteStatepointsForGC] Cosmetic cleanup, NFC to [RS4GC] Cosmetic cleanup, NFC.Oct 6 2015, 5:15 PM
sanjoy edited edge metadata.
This revision was automatically updated to reflect the committed changes.