bruno updated this revision to Diff 44180.
bruno added a comment.
Hi Richard,
Thanks for the comments. Updated the patch!
In http://reviews.llvm.org/D15173#313235, @rsmith wrote:
> I think that this will leave us with a broken token stream. In your example,
> the cached token stream starts as
>
> `NSArray` `<` `id` `<` `PB` `>>` `*` [...]
>
>
> ... and we try to annotate the `id<PB>` with our `CachedLexPos` pointing at
> the `*` token. That leaves `CachedTokens` containing:
>
> `NSArray` `<` `(type annotation)` `*` [...]
>
>
> ... which is wrong. We need to actually convert the `tok::greatergreater` in
> `CachedTokens` into a `tok::greater` and update its location and length in
> order for the cached token stream to be correctly updated. Otherwise if the
> parser backtracks it will see the wrong token stream.
I don't think this happens, the type annotation starts at 7:11 and ends at 7:24:
identifier 'NSArray' Loc=<testcase.mm:7:11>
greatergreater '>>' Loc=<testcase.mm:7:24>
The code that follows the assert then patches the CachedTokens the correct way,
see the CachedTokens before and after:
- Before:
(clang::Preprocessor::CachedTokensTy) $32 = {
[0] = (Loc = 89, UintData = 1, PtrData = 0x0000000000000000, Kind = l_paren,
Flags = 0)
[1] = (Loc = 90, UintData = 7, PtrData = 0x000000010d82fba0, Kind =
identifier, Flags = 0)
[2] = (Loc = 97, UintData = 1, PtrData = 0x0000000000000000, Kind = less,
Flags = 0)
[3] = (Loc = 98, UintData = 2, PtrData = 0x000000010d01da58, Kind =
identifier, Flags = 0)
[4] = (Loc = 100, UintData = 1, PtrData = 0x0000000000000000, Kind = less,
Flags = 0)
[5] = (Loc = 101, UintData = 2, PtrData = 0x000000010d82fb70, Kind =
identifier, Flags = 0)
[6] = (Loc = 103, UintData = 2, PtrData = 0x0000000000000000, Kind =
greatergreater, Flags = 0)
[7] = (Loc = 106, UintData = 1, PtrData = 0x0000000000000000, Kind = star,
Flags = 2)
}
- After:
(clang::Preprocessor::CachedTokensTy) $34 = {
[0] = (Loc = 89, UintData = 1, PtrData = 0x0000000000000000, Kind = l_paren,
Flags = 0)
[1] = (Loc = 90, UintData = 104, PtrData = 0x000000010d820660, Kind =
annot_typename, Flags = 0)
[2] = (Loc = 106, UintData = 1, PtrData = 0x0000000000000000, Kind = star,
Flags = 2)
}
http://reviews.llvm.org/D15173
Files:
lib/Lex/PPCaching.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/Lex/PPCaching.cpp
===================================================================
--- lib/Lex/PPCaching.cpp
+++ lib/Lex/PPCaching.cpp
@@ -97,13 +97,33 @@
void Preprocessor::AnnotatePreviousCachedTokens(const Token &Tok) {
assert(Tok.isAnnotation() && "Expected annotation token");
assert(CachedLexPos != 0 && "Expected to have some cached tokens");
- assert(CachedTokens[CachedLexPos-1].getLastLoc() == Tok.getAnnotationEndLoc()
- && "The annotation should be until the most recent cached token");
+
+#ifndef NDEBUG
+ {
+ Token CachedLastTok = CachedTokens[CachedLexPos - 1];
+ SourceLocation CachedLastTokLoc = CachedLastTok.getLastLoc();
+ SourceLocation TokAnnEndLoc = Tok.getAnnotationEndLoc();
+
+ // The annotation should be until the most recent cached token. Since
+ // `Tok` length could be bigger than one (e.g. greatergreater '>>'),
account
+ // for that case before checking the assertion.
+ if (CachedLastTokLoc != TokAnnEndLoc && !CachedLastTok.isAnnotation()) {
+ CachedLastTokLoc =
+ CachedLastTokLoc.getLocWithOffset(CachedLastTok.getLength());
+ unsigned TokAnnEndLocSize = Lexer::MeasureTokenLength(
+ SourceMgr.getSpellingLoc(TokAnnEndLoc), SourceMgr, LangOpts);
+ TokAnnEndLoc = TokAnnEndLoc.getLocWithOffset(TokAnnEndLocSize);
+ }
+
+ assert(CachedLastTokLoc == TokAnnEndLoc &&
+ "The annotation should be until the most recent cached token");
+ }
+#endif
// Start from the end of the cached tokens list and look for the token
// that is the beginning of the annotation token.
for (CachedTokensTy::size_type i = CachedLexPos; i != 0; --i) {
- CachedTokensTy::iterator AnnotBegin = CachedTokens.begin() + i-1;
+ CachedTokensTy::iterator AnnotBegin = CachedTokens.begin() + i - 1;
if (AnnotBegin->getLocation() == Tok.getLocation()) {
assert((BacktrackPositions.empty() || BacktrackPositions.back() < i) &&
"The backtrack pos points inside the annotated tokens!");
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/Lex/PPCaching.cpp
===================================================================
--- lib/Lex/PPCaching.cpp
+++ lib/Lex/PPCaching.cpp
@@ -97,13 +97,33 @@
void Preprocessor::AnnotatePreviousCachedTokens(const Token &Tok) {
assert(Tok.isAnnotation() && "Expected annotation token");
assert(CachedLexPos != 0 && "Expected to have some cached tokens");
- assert(CachedTokens[CachedLexPos-1].getLastLoc() == Tok.getAnnotationEndLoc()
- && "The annotation should be until the most recent cached token");
+
+#ifndef NDEBUG
+ {
+ Token CachedLastTok = CachedTokens[CachedLexPos - 1];
+ SourceLocation CachedLastTokLoc = CachedLastTok.getLastLoc();
+ SourceLocation TokAnnEndLoc = Tok.getAnnotationEndLoc();
+
+ // The annotation should be until the most recent cached token. Since
+ // `Tok` length could be bigger than one (e.g. greatergreater '>>'), account
+ // for that case before checking the assertion.
+ if (CachedLastTokLoc != TokAnnEndLoc && !CachedLastTok.isAnnotation()) {
+ CachedLastTokLoc =
+ CachedLastTokLoc.getLocWithOffset(CachedLastTok.getLength());
+ unsigned TokAnnEndLocSize = Lexer::MeasureTokenLength(
+ SourceMgr.getSpellingLoc(TokAnnEndLoc), SourceMgr, LangOpts);
+ TokAnnEndLoc = TokAnnEndLoc.getLocWithOffset(TokAnnEndLocSize);
+ }
+
+ assert(CachedLastTokLoc == TokAnnEndLoc &&
+ "The annotation should be until the most recent cached token");
+ }
+#endif
// Start from the end of the cached tokens list and look for the token
// that is the beginning of the annotation token.
for (CachedTokensTy::size_type i = CachedLexPos; i != 0; --i) {
- CachedTokensTy::iterator AnnotBegin = CachedTokens.begin() + i-1;
+ CachedTokensTy::iterator AnnotBegin = CachedTokens.begin() + i - 1;
if (AnnotBegin->getLocation() == Tok.getLocation()) {
assert((BacktrackPositions.empty() || BacktrackPositions.back() < i) &&
"The backtrack pos points inside the annotated tokens!");
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
