arphaman created this revision.
arphaman added reviewers: bruno, rsmith, akyrtzi.
arphaman added a subscriber: cfe-commits.
arphaman set the repository for this revision to rL LLVM.
Herald added a subscriber: nemanjai.
This patch fixes a token caching problem that currently occurs when clang is
skipping a function body (e.g. when looking for a code completion token) and at
the same time caching the tokens for _Pragma when lexing it in macro argument
pre-expansion mode.
When _Pragma is being lexed in macro argument pre-expansion mode, it caches the
tokens so that it can avoid interpreting the pragma immediately (as the macro
argument may not be used in the macro body), and then either backtracks over or
commits these tokens. The problem is that, when we're backtracking/committing
in such a scenario, there's already a previous backtracking position stored in
`BacktrackPositions` (as we're skipping the function body), and this leads to a
situation where the cached tokens from the pragma (like '(' 'string_literal'
and ')') will remain in the cached tokens array incorrectly even after they're
consumed (in the case of backtracking) or just ignored (in the case when
they're committed). Furthermore, what makes it even worse, is that because of a
previous backtracking position, the logic that deals with when should we call
`ExitCachingLexMode` in `CachingLex` no longer works for us in this situation,
and more tokens in the macro argument get cached, to the point where the EOF
token that corresponds to the macro argument EOF is cached. This problem leads
to all sorts of issues in code completion mode, where incorrect errors get
presented and code completion completely fails to produce completion results.
Thanks for taking a look
Repository:
rL LLVM
https://reviews.llvm.org/D28772
Files:
include/clang/Lex/Preprocessor.h
lib/Lex/PPCaching.cpp
test/CodeCompletion/pragma-macro-token-caching.c
Index: test/CodeCompletion/pragma-macro-token-caching.c
===================================================================
--- /dev/null
+++ test/CodeCompletion/pragma-macro-token-caching.c
@@ -0,0 +1,18 @@
+
+#define Outer(action) action
+
+void completeParam(int param) {
+ ;
+ Outer(__extension__({ _Pragma("clang diagnostic push") }));
+ param;
+}
+
+// RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:7:1 %s | FileCheck %s
+// CHECK: param : [#int#]param
+
+void completeParamPragmaError(int param) {
+ Outer(__extension__({ _Pragma(2) })); // expected-error {{_Pragma takes a parenthesized string literal}}
+ param;
+}
+
+// RUN: %clang_cc1 -fsyntax-only -verify -code-completion-at=%s:16:1 %s | FileCheck %s
Index: lib/Lex/PPCaching.cpp
===================================================================
--- lib/Lex/PPCaching.cpp
+++ lib/Lex/PPCaching.cpp
@@ -32,25 +32,58 @@
void Preprocessor::CommitBacktrackedTokens() {
assert(!BacktrackPositions.empty()
&& "EnableBacktrackAtThisPos was not called!");
+ auto PrevCachedLexPos = BacktrackPositions.back();
BacktrackPositions.pop_back();
+ // When committing in a macro argument pre-expansion when tokens are still
+ // being cached, we want to ensure that the tokens which were just committed
+ // won't remain in the cache and that the caching stops.
+ // Otherwise the caching will interfere with the way macro expansion works,
+ // because we will continue to cache tokens after consuming the backtracked
+ // tokens, which shouldn't happen when we're dealing with macro argument
+ // pre-expansion.
+ if (InMacroArgPreExpansion && !BacktrackPositions.empty()) {
+ CachedTokens.erase(CachedTokens.begin() + PrevCachedLexPos,
+ CachedTokens.begin() + CachedLexPos);
+ ExitCachingLexMode();
+ }
}
// Make Preprocessor re-lex the tokens that were lexed since
// EnableBacktrackAtThisPos() was previously called.
void Preprocessor::Backtrack() {
assert(!BacktrackPositions.empty()
&& "EnableBacktrackAtThisPos was not called!");
+ auto CurrentCachedLexPos = CachedLexPos;
CachedLexPos = BacktrackPositions.back();
BacktrackPositions.pop_back();
recomputeCurLexerKind();
+ // When backtracking in a macro argument pre-expansion when tokens are still
+ // being cached, we want to ensure that the tokens over which we just
+ // backtracked won't remain in the cache after they're consumed and and that
+ // the caching will stop after consuming them.
+ // Otherwise they will interfere with the way macro expansion works, because
+ // we will continue to cache tokens after consuming the backtracked tokens,
+ // which shouldn't happen when we're dealing with macro argument
+ // pre-expansion.
+ if (InMacroArgPreExpansion && !BacktrackPositions.empty())
+ ErasedBacktrackRange = CachedTokensRange{CachedLexPos, CurrentCachedLexPos};
}
void Preprocessor::CachingLex(Token &Result) {
if (!InCachingLexMode())
return;
if (CachedLexPos < CachedTokens.size()) {
Result = CachedTokens[CachedLexPos++];
+ // Erase the cached token range after the whole range is consumed if
+ // necessary.
+ if (ErasedBacktrackRange && ErasedBacktrackRange->End == CachedLexPos) {
+ CachedTokens.erase(CachedTokens.begin() + ErasedBacktrackRange->Begin,
+ CachedTokens.begin() + CachedLexPos);
+ CachedLexPos = ErasedBacktrackRange->Begin;
+ ErasedBacktrackRange = None;
+ ExitCachingLexMode();
+ }
return;
}
Index: include/clang/Lex/Preprocessor.h
===================================================================
--- include/clang/Lex/Preprocessor.h
+++ include/clang/Lex/Preprocessor.h
@@ -630,6 +630,13 @@
/// invoked (at which point the last position is popped).
std::vector<CachedTokensTy::size_type> BacktrackPositions;
+ struct CachedTokensRange {
+ CachedTokensTy::size_type Begin, End;
+ };
+ /// \brief A range of cached tokens that should be erased after lexing
+ /// when backtracking requires the erasure of such cached tokens.
+ Optional<CachedTokensRange> ErasedBacktrackRange;
+
struct MacroInfoChain {
MacroInfo MI;
MacroInfoChain *Next;
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
