[cfe-dev] [ClangToolsExtra] PreprocessorTrackerImpl's use of llvm::StringPool

[Chris Lattner via cfe-dev]

Cc John Thompson, but I don’t know his email address.

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.

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.

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.

Any thoughts?

-Chris