[cfe-dev] Suspicious discards of a StringRef in RawCommentList.cpp

[Justin Bogner via cfe-dev]

Richard Smith <richard at metafoo.co.uk> writes:
> On Fri, Oct 14, 2016 at 5:52 PM, Justin Bogner via cfe-dev <
> cfe-dev at lists.llvm.org> wrote:
>
>> There are a couple of calls to getRawText in RawCommentList.cpp that
>> have the comment "Make sure that RawText is valid" but don't actually
>> use the result of getRawText, like this one:
> ...
>> As far as I can tell this doesn't make sure anything is valid. It
>> *might* hit the assert in getRawTextSlow
>
> I don't think even that can happen -- we already called it once in the
> RawComment constructor.
>
>> but it's probably more likely to hit the case that says "if (Invalid)"
>> and returns an empty string.
>>
>> Should this be an assert? Is the comment just wrong and this is
>> pre-populating a cache or something? Should this just be deleted?
>
> It looks like it should just be deleted, along with the RawTextValid flag
> (which stores the same value as Kind != RCK_Invalid) and the lazy on-demand
> RawText computation in getRawText (it's always called on construction).

On further inspection, that's not entirely true - the "Constructor for
AST deserialization" at about RawCommentList:147 and called by
ASTReader::ReadComments doesn't eagerly call this.

So I guess the comment is really trying to say "Lazily initialize
RawText", and it might still be necessary. I'll just improve the
comments for now and explicitly cast the result of getRawText to
void. r284341.