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

[Fangrui Song via cfe-dev]

On 2020-04-14, Reid Kleckner via cfe-dev wrote:
>I also noticed StringPool last month and had more or less the same thought:
>this looks dead, can we remove it?
>
>+1 for replacing this usage with StringSet and removing StringPool.

+1. llvm/Support/StringSaver.h:UniqueStringSaver does more or less the
same thing.

>On Mon, Apr 13, 2020 at 11:03 AM Chris Lattner via cfe-dev <
>cfe-dev at lists.llvm.org> wrote:
>
>> 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
>> _______________________________________________
>> cfe-dev mailing list
>> cfe-dev at lists.llvm.org
>> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
>>

>_______________________________________________
>cfe-dev mailing list
>cfe-dev at lists.llvm.org
>https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev