rnk accepted this revision.
rnk added a comment.
This revision is now accepted and ready to land.
lgtm
================
Comment at: clang-tools-extra/modularize/PreprocessorTracker.cpp:466
-bool operator<(const StringHandle &H1, const StringHandle &H2) {
- const char *S1 = (H1 ? *H1 : "");
----------------
Nice.
================
Comment at: clang-tools-extra/modularize/PreprocessorTracker.cpp:912
// Lookup/add string.
- StringHandle addString(llvm::StringRef Str) { return Strings.intern(Str); }
+ StringHandle addString(llvm::StringRef Str) {
+ return Strings.insert(Str).first->first();
----------------
MaskRay wrote:
> Is it well-known that a StringSet (= `StringMap<NoneType, ...>`) returned
> StringRef is stable? Is that property something we can reliably depend on?
>
> If not, StringSaver.h:UniqueStringSaver may be a better choice.
I am only one data point, but yes, I knew that before this came up. :)
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D78273/new/
https://reviews.llvm.org/D78273
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
