[PATCH] D49596: [Support] Add a UniqueStringSaver: like StringSaver, but deduplicating.

[Haojian Wu via Phabricator via llvm-commits]

hokein accepted this revision.
hokein added a comment.
This revision is now accepted and ready to land.

LGTM.



================
Comment at: lib/Support/StringSaver.cpp:26
+  S = Strings.save(S);
+  if (!Unique.insert(S).second)
+    llvm_unreachable("Failed to insert into unique table - race?");
----------------
sammccall wrote:
> hokein wrote:
> > While we have nice assertion message here, but in return we pay an extra lookup cost. I think we should avoid it (this function is likely to be used in hot path).
> > 
> > Maybe just
> > 
> > ```
> > auto It = Unique.insert(S);
> > if (It.second) {
> >   *It.first = Strings.save(V);
> > }
> > return *It.first.
> > ```
> I was all ready to explain why this doesn't work (keys in a map/set are const) but... it does.
> I think this means `DenseMap` is unsafe (at least less safe than `unordered_map`) but at least this usage is safe. Left a comment.
Yeah, it is tricky.


Repository:
  rL LLVM

https://reviews.llvm.org/D49596