[PATCH] D15173: [Preprocessor] Fix assertion in AnnotatePreviousCachedTokens

[Bruno Cardoso Lopes via cfe-commits]

Hi Richard,

>> +  // The advance from '>>' to '>' in a ObjectiveC template argument list
>> needs
>> +  // to be properly reflected in the token cache to allow correct
>> interaction
>> +  // between annotation and backtracking.
>> +  if (ObjCGenericList && PrevTok.getKind() == tok::greatergreater &&
>
> Why do you only do this in one special case, rather than in the general case
> where we split a token?

Mostly because I don't know how other cases might come up here nor I
have such testcases.

Given the possible tokens in the context of this function, I tried
several variations of ">>>>..." but when dumping the tokens it's
always a combination of tok::greater and tok::greatergreater.

>> +      RemainingToken == tok::greater &&
>> +      PrevTok.getLocation().getRawEncoding() <=
>> +          PP.getLastCachedTokenLocation().getRawEncoding()) {
>
> A <= check on a source location raw encoding is pretty much meaningless if
> you don't know they have the same FileID (and even then, you should ask
> SourceManager to compare the locations).

Ok

> Can you push the "is this a cached token" check down into the preprocessor
> instead? (If we're doing token caching, how can it not be?)

Ok

>> +
>> +void Preprocessor::ReplaceCachedToken(const Token &Tok,
>> +                                      SmallVectorImpl<Token> &NewToks) {
>
> It seems like this could always assume that it's replacing the most recent
> token (the one at CachedLexPos-1).

Ok, gonna rewrite to reflect that.

Thanks,

-- 
Bruno Cardoso Lopes
http://www.brunocardoso.cc