Suspicious discards of a StringRef in RawCommentList.cpp

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:

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).

Richard Smith <richard@metafoo.co.uk> writes:

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.