This is an archive of the discontinued LLVM Phabricator instance.

Make SmallPtrSet count and find able to take const PtrType's
ClosedPublic

Authored by dberlin on Mar 4 2017, 6:37 AM.

Details

Summary

For our set/map types, count/find normally take const references.
This works well for non-pointer types, but can suck for pointer
types.

DenseSet<int *> foo;
const int *b = nullptr;
foo.count(b) does not work

but the equivalent reference version does work
(patch to fix DenseSet/DenseMap coming up)
For SmallPtrSet, you have no such option.

The following will not work right now:
SmallPtrSet<int *> foo;
const int *b = nullptr;
foo.count(b);

This makes const correctness hard in some cases.
Example:
SmallPtrSet<Instruction *> InstructionsToErase;

You can't make this SmallPtrSet<const Instruction *> because then you
can't erase the instruction. If I want to see if something is in the
set, I may only have a const Instruction *. Given that count and find
are non-mutating, this should just work.

The places in our code base that do this resort to const_cast :(.

This patch makes count and find able to be used with const Instruction

  • in the above SmallPtrSet examples.

This is a bit annoying because of where C++ applies the const, so we
have to remove the pointer type from the passed-in-type and rebuild it
with const.

Diff Detail

Repository
rL LLVM

Event Timeline

dberlin created this revision.Mar 4 2017, 6:37 AM
dberlin updated this revision to Diff 90579.Mar 4 2017, 6:45 AM
  • What do you mean we've already got one?
dberlin updated this revision to Diff 90581.Mar 4 2017, 8:46 AM
  • Add PointerLikeTypeTraits for const things, as long as there is one for the non-const version.

Clang and other users have a number of types they use as pointers and
put in SmallPtrSet, and this avoids having to define both const and
non-const versions of PointerLikeTraits for about 5 types.

(Note: You can't easily merge the const T and const T * versions of
these that i can see).

dblaikie accepted this revision.Mar 6 2017, 3:08 PM

Looks reasonable - Needs a unit test or two, though.

This revision is now accepted and ready to land.Mar 6 2017, 3:08 PM
This revision was automatically updated to reflect the committed changes.