On Jan 7, 2016 7:13 PM, "Bruno Cardoso Lopes" <bruno.card...@gmail.com> wrote: > > bruno updated this revision to Diff 44306. > bruno added a comment. > > Hi Richard, > > 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. > > > http://reviews.llvm.org/D15173 > > Files: > include/clang/Lex/Preprocessor.h > lib/Lex/PPCaching.cpp > lib/Parse/ParseTemplate.cpp > test/Parser/objcxx11-protocol-in-template.mm > > Index: test/Parser/objcxx11-protocol-in-template.mm > =================================================================== > --- test/Parser/objcxx11-protocol-in-template.mm > +++ test/Parser/objcxx11-protocol-in-template.mm > @@ -8,3 +8,11 @@ > > vector<id<P>> v; > vector<vector<id<P>>> v2; > + > +@protocol PA; > +@protocol PB; > + > +@class NSArray<ObjectType>; > +typedef int some_t; > + > +id<PA> FA(NSArray<id<PB>> *h, some_t group); > Index: lib/Parse/ParseTemplate.cpp > =================================================================== > --- lib/Parse/ParseTemplate.cpp > +++ lib/Parse/ParseTemplate.cpp > @@ -827,6 +827,7 @@ > } > > // Strip the initial '>' from the token. > + Token PrevTok = Tok; > if (RemainingToken == tok::equal && Next.is(tok::equal) && > areTokensAdjacent(Tok, Next)) { > // Join two adjacent '=' tokens into one, for cases like: > @@ -843,6 +844,20 @@ > PP.getSourceManager(), > getLangOpts())); > > + // 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? > + 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). 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?) > + Token ReplTok = PrevTok; > + PrevTok.setKind(RemainingToken); > + PrevTok.setLength(1); > + SmallVector<Token, 2> NewToks = {PrevTok, Tok}; > + PP.ReplaceCachedToken(ReplTok, NewToks); > + } > + > if (!ConsumeLastToken) { > // Since we're not supposed to consume the '>' token, we need to push > // this token and revert the current token back to the '>'. > Index: lib/Lex/PPCaching.cpp > =================================================================== > --- lib/Lex/PPCaching.cpp > +++ lib/Lex/PPCaching.cpp > @@ -116,3 +116,19 @@ > } > } > } > + > +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). > + assert(CachedLexPos != 0 && "Expected to have some cached tokens"); > + for (auto i = CachedLexPos - 1; i != 0; --i) { > + const Token CurrCachedTok = CachedTokens[i]; > + if (CurrCachedTok.getKind() == Tok.getKind() && > + CurrCachedTok.getLocation() == Tok.getLocation()) { > + CachedTokens.insert(CachedTokens.begin() + i, NewToks.begin(), > + NewToks.end()); > + CachedTokens.erase(CachedTokens.begin() + i + NewToks.size()); > + CachedLexPos += NewToks.size() - 1; > + return; > + } > + } > +} > Index: include/clang/Lex/Preprocessor.h > =================================================================== > --- include/clang/Lex/Preprocessor.h > +++ include/clang/Lex/Preprocessor.h > @@ -1185,6 +1185,12 @@ > return CachedTokens[CachedLexPos-1].getLastLoc(); > } > > + /// \brief Replace token \p Tok in CachedTokens by the tokens in \p NewToks. > + /// > + /// Useful when a token needs to be split in smaller ones and CachedTokens > + /// must to be updated to reflect that. > + void ReplaceCachedToken(const Token &Tok, SmallVectorImpl<Token> &NewToks); > + > /// \brief Replace the last token with an annotation token. > /// > /// Like AnnotateCachedTokens(), this routine replaces an > >
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits