r312947 - [Lexer] Report more precise skipped regions (PR34166)
Vedant Kumar via cfe-commits
cfe-commits at lists.llvm.org
Mon Sep 11 13:47:42 PDT 2017
Author: vedantk
Date: Mon Sep 11 13:47:42 2017
New Revision: 312947
URL: http://llvm.org/viewvc/llvm-project?rev=312947&view=rev
Log:
[Lexer] Report more precise skipped regions (PR34166)
This patch teaches the preprocessor to report more precise source ranges for
code that is skipped due to conditional directives.
The new behavior includes the '#' from the opening directive and the full text
of the line containing the closing directive in the skipped area. This matches
up clang's behavior (we don't IRGen the code between the closing "endif" and
the end of a line).
This also affects the code coverage implementation. See llvm.org/PR34166 (this
also happens to be rdar://problem/23224058).
The old behavior (report the end of the skipped range as the end
location of the 'endif' token) is preserved for indexing clients.
Differential Revision: https://reviews.llvm.org/D36642
Modified:
cfe/trunk/include/clang/Lex/PPCallbacks.h
cfe/trunk/include/clang/Lex/PreprocessingRecord.h
cfe/trunk/include/clang/Lex/Preprocessor.h
cfe/trunk/lib/CodeGen/CoverageMappingGen.cpp
cfe/trunk/lib/CodeGen/CoverageMappingGen.h
cfe/trunk/lib/Lex/PPDirectives.cpp
cfe/trunk/lib/Lex/PreprocessingRecord.cpp
cfe/trunk/test/CoverageMapping/preprocessor.c
cfe/trunk/test/Index/skipped-ranges.c
cfe/trunk/tools/libclang/Indexing.cpp
Modified: cfe/trunk/include/clang/Lex/PPCallbacks.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Lex/PPCallbacks.h?rev=312947&r1=312946&r2=312947&view=diff
==============================================================================
--- cfe/trunk/include/clang/Lex/PPCallbacks.h (original)
+++ cfe/trunk/include/clang/Lex/PPCallbacks.h Mon Sep 11 13:47:42 2017
@@ -266,7 +266,10 @@ public:
/// \brief Hook called when a source range is skipped.
/// \param Range The SourceRange that was skipped. The range begins at the
/// \#if/\#else directive and ends after the \#endif/\#else directive.
- virtual void SourceRangeSkipped(SourceRange Range) {
+ /// \param EndifLoc The end location of the 'endif' token, which may precede
+ /// the range skipped by the directive (e.g excluding comments after an
+ /// 'endif').
+ virtual void SourceRangeSkipped(SourceRange Range, SourceLocation EndifLoc) {
}
enum ConditionValueKind {
@@ -468,9 +471,9 @@ public:
Second->Defined(MacroNameTok, MD, Range);
}
- void SourceRangeSkipped(SourceRange Range) override {
- First->SourceRangeSkipped(Range);
- Second->SourceRangeSkipped(Range);
+ void SourceRangeSkipped(SourceRange Range, SourceLocation EndifLoc) override {
+ First->SourceRangeSkipped(Range, EndifLoc);
+ Second->SourceRangeSkipped(Range, EndifLoc);
}
/// \brief Hook called whenever an \#if is seen.
Modified: cfe/trunk/include/clang/Lex/PreprocessingRecord.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Lex/PreprocessingRecord.h?rev=312947&r1=312946&r2=312947&view=diff
==============================================================================
--- cfe/trunk/include/clang/Lex/PreprocessingRecord.h (original)
+++ cfe/trunk/include/clang/Lex/PreprocessingRecord.h Mon Sep 11 13:47:42 2017
@@ -504,7 +504,8 @@ namespace clang {
void Defined(const Token &MacroNameTok, const MacroDefinition &MD,
SourceRange Range) override;
- void SourceRangeSkipped(SourceRange Range) override;
+ void SourceRangeSkipped(SourceRange Range,
+ SourceLocation EndifLoc) override;
void addMacroExpansion(const Token &Id, const MacroInfo *MI,
SourceRange Range);
Modified: cfe/trunk/include/clang/Lex/Preprocessor.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Lex/Preprocessor.h?rev=312947&r1=312946&r2=312947&view=diff
==============================================================================
--- cfe/trunk/include/clang/Lex/Preprocessor.h (original)
+++ cfe/trunk/include/clang/Lex/Preprocessor.h Mon Sep 11 13:47:42 2017
@@ -1837,7 +1837,8 @@ private:
/// \p FoundElse is false, then \#else directives are ok, if not, then we have
/// already seen one so a \#else directive is a duplicate. When this returns,
/// the caller can lex the first valid token.
- void SkipExcludedConditionalBlock(SourceLocation IfTokenLoc,
+ void SkipExcludedConditionalBlock(const Token &HashToken,
+ SourceLocation IfTokenLoc,
bool FoundNonSkipPortion, bool FoundElse,
SourceLocation ElseLoc = SourceLocation());
@@ -2031,12 +2032,13 @@ private:
void HandleUndefDirective();
// Conditional Inclusion.
- void HandleIfdefDirective(Token &Tok, bool isIfndef,
- bool ReadAnyTokensBeforeDirective);
- void HandleIfDirective(Token &Tok, bool ReadAnyTokensBeforeDirective);
+ void HandleIfdefDirective(Token &Tok, const Token &HashToken,
+ bool isIfndef, bool ReadAnyTokensBeforeDirective);
+ void HandleIfDirective(Token &Tok, const Token &HashToken,
+ bool ReadAnyTokensBeforeDirective);
void HandleEndifDirective(Token &Tok);
- void HandleElseDirective(Token &Tok);
- void HandleElifDirective(Token &Tok);
+ void HandleElseDirective(Token &Tok, const Token &HashToken);
+ void HandleElifDirective(Token &Tok, const Token &HashToken);
// Pragmas.
void HandlePragmaDirective(SourceLocation IntroducerLoc,
Modified: cfe/trunk/lib/CodeGen/CoverageMappingGen.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CoverageMappingGen.cpp?rev=312947&r1=312946&r2=312947&view=diff
==============================================================================
--- cfe/trunk/lib/CodeGen/CoverageMappingGen.cpp (original)
+++ cfe/trunk/lib/CodeGen/CoverageMappingGen.cpp Mon Sep 11 13:47:42 2017
@@ -29,7 +29,7 @@ using namespace clang;
using namespace CodeGen;
using namespace llvm::coverage;
-void CoverageSourceInfo::SourceRangeSkipped(SourceRange Range) {
+void CoverageSourceInfo::SourceRangeSkipped(SourceRange Range, SourceLocation) {
SkippedRanges.push_back(Range);
}
Modified: cfe/trunk/lib/CodeGen/CoverageMappingGen.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CoverageMappingGen.h?rev=312947&r1=312946&r2=312947&view=diff
==============================================================================
--- cfe/trunk/lib/CodeGen/CoverageMappingGen.h (original)
+++ cfe/trunk/lib/CodeGen/CoverageMappingGen.h Mon Sep 11 13:47:42 2017
@@ -39,7 +39,7 @@ class CoverageSourceInfo : public PPCall
public:
ArrayRef<SourceRange> getSkippedRanges() const { return SkippedRanges; }
- void SourceRangeSkipped(SourceRange Range) override;
+ void SourceRangeSkipped(SourceRange Range, SourceLocation EndifLoc) override;
};
namespace CodeGen {
Modified: cfe/trunk/lib/Lex/PPDirectives.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Lex/PPDirectives.cpp?rev=312947&r1=312946&r2=312947&view=diff
==============================================================================
--- cfe/trunk/lib/Lex/PPDirectives.cpp (original)
+++ cfe/trunk/lib/Lex/PPDirectives.cpp Mon Sep 11 13:47:42 2017
@@ -79,7 +79,8 @@ Preprocessor::AllocateVisibilityMacroDir
}
/// \brief Read and discard all tokens remaining on the current line until
-/// the tok::eod token is found.
+/// the tok::eod token is found. If the discarded tokens are in a skipped range,
+/// complete the range and pass it to the \c SourceRangeSkipped callback.
void Preprocessor::DiscardUntilEndOfDirective() {
Token Tmp;
do {
@@ -350,7 +351,8 @@ void Preprocessor::CheckEndOfDirective(c
/// If ElseOk is true, then \#else directives are ok, if not, then we have
/// already seen one so a \#else directive is a duplicate. When this returns,
/// the caller can lex the first valid token.
-void Preprocessor::SkipExcludedConditionalBlock(SourceLocation IfTokenLoc,
+void Preprocessor::SkipExcludedConditionalBlock(const Token &HashToken,
+ SourceLocation IfTokenLoc,
bool FoundNonSkipPortion,
bool FoundElse,
SourceLocation ElseLoc) {
@@ -558,10 +560,10 @@ void Preprocessor::SkipExcludedCondition
// the #if block.
CurPPLexer->LexingRawMode = false;
- if (Callbacks) {
- SourceLocation BeginLoc = ElseLoc.isValid() ? ElseLoc : IfTokenLoc;
- Callbacks->SourceRangeSkipped(SourceRange(BeginLoc, Tok.getLocation()));
- }
+ if (Callbacks)
+ Callbacks->SourceRangeSkipped(
+ SourceRange(HashToken.getLocation(), CurPPLexer->getSourceLocation()),
+ Tok.getLocation());
}
void Preprocessor::PTHSkipExcludedConditionalBlock() {
@@ -949,15 +951,17 @@ void Preprocessor::HandleDirective(Token
default: break;
// C99 6.10.1 - Conditional Inclusion.
case tok::pp_if:
- return HandleIfDirective(Result, ReadAnyTokensBeforeDirective);
+ return HandleIfDirective(Result, SavedHash, ReadAnyTokensBeforeDirective);
case tok::pp_ifdef:
- return HandleIfdefDirective(Result, false, true/*not valid for miopt*/);
+ return HandleIfdefDirective(Result, SavedHash, false,
+ true /*not valid for miopt*/);
case tok::pp_ifndef:
- return HandleIfdefDirective(Result, true, ReadAnyTokensBeforeDirective);
+ return HandleIfdefDirective(Result, SavedHash, true,
+ ReadAnyTokensBeforeDirective);
case tok::pp_elif:
- return HandleElifDirective(Result);
+ return HandleElifDirective(Result, SavedHash);
case tok::pp_else:
- return HandleElseDirective(Result);
+ return HandleElseDirective(Result, SavedHash);
case tok::pp_endif:
return HandleEndifDirective(Result);
@@ -2613,7 +2617,9 @@ void Preprocessor::HandleUndefDirective(
/// true if any tokens have been returned or pp-directives activated before this
/// \#ifndef has been lexed.
///
-void Preprocessor::HandleIfdefDirective(Token &Result, bool isIfndef,
+void Preprocessor::HandleIfdefDirective(Token &Result,
+ const Token &HashToken,
+ bool isIfndef,
bool ReadAnyTokensBeforeDirective) {
++NumIf;
Token DirectiveTok = Result;
@@ -2625,8 +2631,8 @@ void Preprocessor::HandleIfdefDirective(
if (MacroNameTok.is(tok::eod)) {
// Skip code until we get to #endif. This helps with recovery by not
// emitting an error when the #endif is reached.
- SkipExcludedConditionalBlock(DirectiveTok.getLocation(),
- /*Foundnonskip*/false, /*FoundElse*/false);
+ SkipExcludedConditionalBlock(HashToken, DirectiveTok.getLocation(),
+ /*Foundnonskip*/ false, /*FoundElse*/ false);
return;
}
@@ -2674,15 +2680,16 @@ void Preprocessor::HandleIfdefDirective(
/*foundelse*/false);
} else {
// No, skip the contents of this block.
- SkipExcludedConditionalBlock(DirectiveTok.getLocation(),
- /*Foundnonskip*/false,
- /*FoundElse*/false);
+ SkipExcludedConditionalBlock(HashToken, DirectiveTok.getLocation(),
+ /*Foundnonskip*/ false,
+ /*FoundElse*/ false);
}
}
/// HandleIfDirective - Implements the \#if directive.
///
void Preprocessor::HandleIfDirective(Token &IfToken,
+ const Token &HashToken,
bool ReadAnyTokensBeforeDirective) {
++NumIf;
@@ -2720,8 +2727,9 @@ void Preprocessor::HandleIfDirective(Tok
/*foundnonskip*/true, /*foundelse*/false);
} else {
// No, skip the contents of this block.
- SkipExcludedConditionalBlock(IfToken.getLocation(), /*Foundnonskip*/false,
- /*FoundElse*/false);
+ SkipExcludedConditionalBlock(HashToken, IfToken.getLocation(),
+ /*Foundnonskip*/ false,
+ /*FoundElse*/ false);
}
}
@@ -2753,7 +2761,7 @@ void Preprocessor::HandleEndifDirective(
/// HandleElseDirective - Implements the \#else directive.
///
-void Preprocessor::HandleElseDirective(Token &Result) {
+void Preprocessor::HandleElseDirective(Token &Result, const Token &HashToken) {
++NumElse;
// #else directive in a non-skipping conditional... start skipping.
@@ -2784,13 +2792,14 @@ void Preprocessor::HandleElseDirective(T
}
// Finally, skip the rest of the contents of this block.
- SkipExcludedConditionalBlock(CI.IfLoc, /*Foundnonskip*/true,
- /*FoundElse*/true, Result.getLocation());
+ SkipExcludedConditionalBlock(HashToken, CI.IfLoc, /*Foundnonskip*/ true,
+ /*FoundElse*/ true, Result.getLocation());
}
/// HandleElifDirective - Implements the \#elif directive.
///
-void Preprocessor::HandleElifDirective(Token &ElifToken) {
+void Preprocessor::HandleElifDirective(Token &ElifToken,
+ const Token &HashToken) {
++NumElse;
// #elif directive in a non-skipping conditional... start skipping.
@@ -2827,7 +2836,7 @@ void Preprocessor::HandleElifDirective(T
}
// Finally, skip the rest of the contents of this block.
- SkipExcludedConditionalBlock(CI.IfLoc, /*Foundnonskip*/true,
- /*FoundElse*/CI.FoundElse,
+ SkipExcludedConditionalBlock(HashToken, CI.IfLoc, /*Foundnonskip*/ true,
+ /*FoundElse*/ CI.FoundElse,
ElifToken.getLocation());
}
Modified: cfe/trunk/lib/Lex/PreprocessingRecord.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Lex/PreprocessingRecord.cpp?rev=312947&r1=312946&r2=312947&view=diff
==============================================================================
--- cfe/trunk/lib/Lex/PreprocessingRecord.cpp (original)
+++ cfe/trunk/lib/Lex/PreprocessingRecord.cpp Mon Sep 11 13:47:42 2017
@@ -400,8 +400,9 @@ void PreprocessingRecord::Defined(const
MacroNameTok.getLocation());
}
-void PreprocessingRecord::SourceRangeSkipped(SourceRange Range) {
- SkippedRanges.push_back(Range);
+void PreprocessingRecord::SourceRangeSkipped(SourceRange Range,
+ SourceLocation EndifLoc) {
+ SkippedRanges.emplace_back(Range.getBegin(), EndifLoc);
}
void PreprocessingRecord::MacroExpands(const Token &Id,
Modified: cfe/trunk/test/CoverageMapping/preprocessor.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CoverageMapping/preprocessor.c?rev=312947&r1=312946&r2=312947&view=diff
==============================================================================
--- cfe/trunk/test/CoverageMapping/preprocessor.c (original)
+++ cfe/trunk/test/CoverageMapping/preprocessor.c Mon Sep 11 13:47:42 2017
@@ -3,36 +3,69 @@
// CHECK: func
void func() { // CHECK: File 0, [[@LINE]]:13 -> [[@LINE+5]]:2 = #0
int i = 0;
-#ifdef MACRO // CHECK-NEXT: Skipped,File 0, [[@LINE]]:2 -> [[@LINE+2]]:2 = 0
+#ifdef MACRO // CHECK-NEXT: Skipped,File 0, [[@LINE]]:1 -> [[@LINE+3]]:1 = 0
int x = i;
#endif
}
-#if 0
- int g = 0;
-
- void bar() { }
-#endif
-
// CHECK: main
int main() { // CHECK-NEXT: File 0, [[@LINE]]:12 -> {{[0-9]+}}:2 = #0
int i = 0;
-#if 0 // CHECK-NEXT: Skipped,File 0, [[@LINE]]:2 -> [[@LINE+4]]:2 = 0
+# if 0 // CHECK-NEXT: Skipped,File 0, [[@LINE]]:1 -> [[@LINE+5]]:1 = 0
if(i == 0) {
i = 1;
}
-#endif
+# endif // Mark me skipped!
#if 1
// CHECK-NEXT: File 0, [[@LINE+1]]:6 -> [[@LINE+1]]:12 = #0
if(i == 0) { // CHECK-NEXT: File 0, [[@LINE]]:14 -> [[@LINE+2]]:4 = #1
i = 1;
}
-#else // CHECK-NEXT: Skipped,File 0, [[@LINE]]:2 -> [[@LINE+5]]:2 = 0
+#else // CHECK-NEXT: Skipped,File 0, [[@LINE]]:1 -> [[@LINE+6]]:1 = 0
if(i == 1) {
i = 0;
}
}
#endif
+
+ // CHECK-NEXT: Skipped,File 0, [[@LINE+1]]:1 -> [[@LINE+5]]:1
+#\
+ if 0
+#\
+ endif // also skipped
+
+#if 1
+ // CHECK-NEXT: Skipped,File 0, [[@LINE+1]]:1 -> [[@LINE+4]]:1
+#\
+ elif 0
+#endif
+
+#if 1
+ // CHECK-NEXT: Skipped,File 0, [[@LINE+1]]:1 -> [[@LINE+4]]:1
+#\
+ else
+#endif
+
+ // CHECK-NEXT: Skipped,File 0, [[@LINE+1]]:1 -> [[@LINE+5]]:1
+#\
+ ifdef NOT_DEFINED
+#\
+ endif
+
+ // CHECK-NEXT: Skipped,File 0, [[@LINE+1]]:1 -> [[@LINE+5]]:1
+#\
+ ifndef __FILE__
+#\
+ endif
+
+ // CHECK-NEXT: Skipped,File 0, [[@LINE+1]]:1 -> [[@LINE+7]]:1
+#\
+ ifdef NOT_DEFINED
+#\
+ \
+ \
+ endif // also skipped
+
return 0;
}
Modified: cfe/trunk/test/Index/skipped-ranges.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Index/skipped-ranges.c?rev=312947&r1=312946&r2=312947&view=diff
==============================================================================
--- cfe/trunk/test/Index/skipped-ranges.c (original)
+++ cfe/trunk/test/Index/skipped-ranges.c Mon Sep 11 13:47:42 2017
@@ -20,6 +20,6 @@ int probably_hot = 1;
#endif // cool
// RUN: env CINDEXTEST_SHOW_SKIPPED_RANGES=1 c-index-test -test-annotate-tokens=%s:1:1:16:1 %s | FileCheck %s
-// CHECK: Skipping: [5:2 - 6:7]
-// CHECK: Skipping: [8:2 - 12:7]
-// CHECK: Skipping: [14:2 - 20:7]
+// CHECK: Skipping: [5:1 - 6:7]
+// CHECK: Skipping: [8:1 - 12:7]
+// CHECK: Skipping: [14:1 - 20:7]
Modified: cfe/trunk/tools/libclang/Indexing.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/tools/libclang/Indexing.cpp?rev=312947&r1=312946&r2=312947&view=diff
==============================================================================
--- cfe/trunk/tools/libclang/Indexing.cpp (original)
+++ cfe/trunk/tools/libclang/Indexing.cpp Mon Sep 11 13:47:42 2017
@@ -272,7 +272,8 @@ public:
/// SourceRangeSkipped - This hook is called when a source range is skipped.
/// \param Range The SourceRange that was skipped. The range begins at the
/// #if/#else directive and ends after the #endif/#else directive.
- void SourceRangeSkipped(SourceRange Range) override {}
+ void SourceRangeSkipped(SourceRange Range, SourceLocation EndifLoc) override {
+ }
};
//===----------------------------------------------------------------------===//
More information about the cfe-commits
mailing list