<p dir="ltr">On Jan 7, 2016 7:13 PM, "Bruno Cardoso Lopes" <<a href="mailto:bruno.cardoso@gmail.com">bruno.cardoso@gmail.com</a>> wrote:<br>
><br>
> bruno updated this revision to Diff 44306.<br>
> bruno added a comment.<br>
><br>
> Hi Richard,<br>
><br>
> Thanks for the detailed explanation, it now makes sense to me. I updated the patch with another approach! Let me know if it's the right direction.<br>
><br>
><br>
> <a href="http://reviews.llvm.org/D15173">http://reviews.llvm.org/D15173</a><br>
><br>
> Files:<br>
>   include/clang/Lex/Preprocessor.h<br>
>   lib/Lex/PPCaching.cpp<br>
>   lib/Parse/ParseTemplate.cpp<br>
>   test/Parser/<a href="http://objcxx11-protocol-in-template.mm">objcxx11-protocol-in-template.mm</a><br>
><br>
> Index: test/Parser/<a href="http://objcxx11-protocol-in-template.mm">objcxx11-protocol-in-template.mm</a><br>
> ===================================================================<br>
> --- test/Parser/<a href="http://objcxx11-protocol-in-template.mm">objcxx11-protocol-in-template.mm</a><br>
> +++ test/Parser/<a href="http://objcxx11-protocol-in-template.mm">objcxx11-protocol-in-template.mm</a><br>
> @@ -8,3 +8,11 @@<br>
><br>
>  vector<id<P>> v;<br>
>  vector<vector<id<P>>> v2;<br>
> +<br>
> +@protocol PA;<br>
> +@protocol PB;<br>
> +<br>
> +@class NSArray<ObjectType>;<br>
> +typedef int some_t;<br>
> +<br>
> +id<PA> FA(NSArray<id<PB>> *h, some_t group);<br>
> Index: lib/Parse/ParseTemplate.cpp<br>
> ===================================================================<br>
> --- lib/Parse/ParseTemplate.cpp<br>
> +++ lib/Parse/ParseTemplate.cpp<br>
> @@ -827,6 +827,7 @@<br>
>    }<br>
><br>
>    // Strip the initial '>' from the token.<br>
> +  Token PrevTok = Tok;<br>
>    if (RemainingToken == tok::equal && Next.is(tok::equal) &&<br>
>        areTokensAdjacent(Tok, Next)) {<br>
>      // Join two adjacent '=' tokens into one, for cases like:<br>
> @@ -843,6 +844,20 @@<br>
>                                                   PP.getSourceManager(),<br>
>                                                   getLangOpts()));<br>
><br>
> +  // The advance from '>>' to '>' in a ObjectiveC template argument list needs<br>
> +  // to be properly reflected in the token cache to allow correct interaction<br>
> +  // between annotation and backtracking.<br>
> +  if (ObjCGenericList && PrevTok.getKind() == tok::greatergreater &&</p>
<p dir="ltr">Why do you only do this in one special case, rather than in the general case where we split a token?</p>
<p dir="ltr">> +      RemainingToken == tok::greater &&<br>
> +      PrevTok.getLocation().getRawEncoding() <=<br>
> +          PP.getLastCachedTokenLocation().getRawEncoding()) {</p>
<p dir="ltr">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).</p>
<p dir="ltr">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?)</p>
<p dir="ltr">> +    Token ReplTok = PrevTok;<br>
> +    PrevTok.setKind(RemainingToken);<br>
> +    PrevTok.setLength(1);<br>
> +    SmallVector<Token, 2> NewToks = {PrevTok, Tok};<br>
> +    PP.ReplaceCachedToken(ReplTok, NewToks);<br>
> +  }<br>
> +<br>
>    if (!ConsumeLastToken) {<br>
>      // Since we're not supposed to consume the '>' token, we need to push<br>
>      // this token and revert the current token back to the '>'.<br>
> Index: lib/Lex/PPCaching.cpp<br>
> ===================================================================<br>
> --- lib/Lex/PPCaching.cpp<br>
> +++ lib/Lex/PPCaching.cpp<br>
> @@ -116,3 +116,19 @@<br>
>      }<br>
>    }<br>
>  }<br>
> +<br>
> +void Preprocessor::ReplaceCachedToken(const Token &Tok,<br>
> +                                      SmallVectorImpl<Token> &NewToks) {</p>
<p dir="ltr">It seems like this could always assume that it's replacing the most recent token (the one at CachedLexPos-1).</p>
<p dir="ltr">> +  assert(CachedLexPos != 0 && "Expected to have some cached tokens");<br>
> +  for (auto i = CachedLexPos - 1; i != 0; --i) {<br>
> +    const Token CurrCachedTok = CachedTokens[i];<br>
> +    if (CurrCachedTok.getKind() == Tok.getKind() &&<br>
> +        CurrCachedTok.getLocation() == Tok.getLocation()) {<br>
> +      CachedTokens.insert(CachedTokens.begin() + i, NewToks.begin(),<br>
> +                          NewToks.end());<br>
> +      CachedTokens.erase(CachedTokens.begin() + i + NewToks.size());<br>
> +      CachedLexPos += NewToks.size() - 1;<br>
> +      return;<br>
> +    }<br>
> +  }<br>
> +}<br>
> Index: include/clang/Lex/Preprocessor.h<br>
> ===================================================================<br>
> --- include/clang/Lex/Preprocessor.h<br>
> +++ include/clang/Lex/Preprocessor.h<br>
> @@ -1185,6 +1185,12 @@<br>
>      return CachedTokens[CachedLexPos-1].getLastLoc();<br>
>    }<br>
><br>
> +  /// \brief Replace token \p Tok in CachedTokens by the tokens in \p NewToks.<br>
> +  ///<br>
> +  /// Useful when a token needs to be split in smaller ones and CachedTokens<br>
> +  /// must to be updated to reflect that.<br>
> +  void ReplaceCachedToken(const Token &Tok, SmallVectorImpl<Token> &NewToks);<br>
> +<br>
>    /// \brief Replace the last token with an annotation token.<br>
>    ///<br>
>    /// Like AnnotateCachedTokens(), this routine replaces an<br>
><br>
><br>
</p>