<div dir="ltr">I also noticed StringPool last month and had more or less the same thought: this looks dead, can we remove it?<div><br></div><div>+1 for replacing this usage with StringSet and removing StringPool.<br></div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Mon, Apr 13, 2020 at 11:03 AM Chris Lattner via cfe-dev <<a href="mailto:cfe-dev@lists.llvm.org">cfe-dev@lists.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Cc John Thompson, but I don’t know his email address.<br>
<br>
I recently ran across llvm::StringPool in llvm/lib/Support. It is an old class which isn’t ideal in several ways, and I would like to remove it in favor of direct uses of llvm::StringSet, which is nearly the same thing and a lot more efficient. The one difference is that in this one, each entry is individually reference counted, so they get deallocated as soon as the last reference is dropped.<br>
<br>
The only use of this class is in PreprocessorTrackerImpl in clang-tools-extra/modularize/PreprocessorTracker.cpp. Does anyone object to changing this to being a simple llvm::StringSet? Such a change would make this more efficient (and allow deleting a misleading class out of llvm/lib/Support), allow simplifying away the "StringHandle” logic (in favor of using StringRef directly) and the only cost that I can see is that the strings won’t get deallocated as eagerly.<br>
<br>
An alternative solution here is to move StringPool into clang-tools-extra as a private implementation detail. I’d personally rather remove it outright than preserve it.<br>
<br>
Any thoughts?<br>
<br>
-Chris<br>
_______________________________________________<br>
cfe-dev mailing list<br>
<a href="mailto:cfe-dev@lists.llvm.org" target="_blank">cfe-dev@lists.llvm.org</a><br>
<a href="https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev" rel="noreferrer" target="_blank">https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev</a><br>
</blockquote></div>