From c9445f041caab06d4aa184cb0e0b15a5d27feeaf Mon Sep 17 00:00:00 2001 From: Vedant Kumar Date: Mon, 11 Sep 2017 20:47:42 +0000 Subject: [PATCH] [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 git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@312947 91177308-0d34-0410-b5e6-96231b3b80d8 --- include/clang/Lex/PPCallbacks.h | 11 +++-- include/clang/Lex/PreprocessingRecord.h | 3 +- include/clang/Lex/Preprocessor.h | 14 +++--- lib/CodeGen/CoverageMappingGen.cpp | 2 +- lib/CodeGen/CoverageMappingGen.h | 2 +- lib/Lex/PPDirectives.cpp | 59 ++++++++++++++----------- lib/Lex/PreprocessingRecord.cpp | 5 ++- test/CoverageMapping/preprocessor.c | 53 +++++++++++++++++----- test/Index/skipped-ranges.c | 6 +-- tools/libclang/Indexing.cpp | 3 +- 10 files changed, 104 insertions(+), 54 deletions(-) diff --git a/include/clang/Lex/PPCallbacks.h b/include/clang/Lex/PPCallbacks.h index c1e1a54960..0e0a6c1406 100644 --- a/include/clang/Lex/PPCallbacks.h +++ b/include/clang/Lex/PPCallbacks.h @@ -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. diff --git a/include/clang/Lex/PreprocessingRecord.h b/include/clang/Lex/PreprocessingRecord.h index fc2c507004..882b8aedf7 100644 --- a/include/clang/Lex/PreprocessingRecord.h +++ b/include/clang/Lex/PreprocessingRecord.h @@ -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); diff --git a/include/clang/Lex/Preprocessor.h b/include/clang/Lex/Preprocessor.h index 3dd84d8cfb..ae6f201830 100644 --- a/include/clang/Lex/Preprocessor.h +++ b/include/clang/Lex/Preprocessor.h @@ -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, diff --git a/lib/CodeGen/CoverageMappingGen.cpp b/lib/CodeGen/CoverageMappingGen.cpp index f47dd07f0c..cf736a484e 100644 --- a/lib/CodeGen/CoverageMappingGen.cpp +++ b/lib/CodeGen/CoverageMappingGen.cpp @@ -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); } diff --git a/lib/CodeGen/CoverageMappingGen.h b/lib/CodeGen/CoverageMappingGen.h index b6789c2a79..d07ed5ebcf 100644 --- a/lib/CodeGen/CoverageMappingGen.h +++ b/lib/CodeGen/CoverageMappingGen.h @@ -39,7 +39,7 @@ class CoverageSourceInfo : public PPCallbacks { public: ArrayRef getSkippedRanges() const { return SkippedRanges; } - void SourceRangeSkipped(SourceRange Range) override; + void SourceRangeSkipped(SourceRange Range, SourceLocation EndifLoc) override; }; namespace CodeGen { diff --git a/lib/Lex/PPDirectives.cpp b/lib/Lex/PPDirectives.cpp index 6b09398e46..556e76356d 100644 --- a/lib/Lex/PPDirectives.cpp +++ b/lib/Lex/PPDirectives.cpp @@ -79,7 +79,8 @@ Preprocessor::AllocateVisibilityMacroDirective(SourceLocation Loc, } /// \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(const char *DirType, bool EnableMacros) { /// 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::SkipExcludedConditionalBlock(SourceLocation IfTokenLoc, // 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 &Result) { 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(Token &Result, bool isIfndef, 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(Token &Result, bool isIfndef, /*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(Token &IfToken, /*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(Token &EndifToken) { /// 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(Token &Result) { } // 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(Token &ElifToken) { } // 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()); } diff --git a/lib/Lex/PreprocessingRecord.cpp b/lib/Lex/PreprocessingRecord.cpp index 03c4cbe589..954b569bb0 100644 --- a/lib/Lex/PreprocessingRecord.cpp +++ b/lib/Lex/PreprocessingRecord.cpp @@ -400,8 +400,9 @@ void PreprocessingRecord::Defined(const Token &MacroNameTok, 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, diff --git a/test/CoverageMapping/preprocessor.c b/test/CoverageMapping/preprocessor.c index bd82b3939e..cce8b67999 100644 --- a/test/CoverageMapping/preprocessor.c +++ b/test/CoverageMapping/preprocessor.c @@ -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; } diff --git a/test/Index/skipped-ranges.c b/test/Index/skipped-ranges.c index bd16fb3856..f6c6c7f638 100644 --- a/test/Index/skipped-ranges.c +++ b/test/Index/skipped-ranges.c @@ -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] diff --git a/tools/libclang/Indexing.cpp b/tools/libclang/Indexing.cpp index 5312b7c016..2a136242ef 100644 --- a/tools/libclang/Indexing.cpp +++ b/tools/libclang/Indexing.cpp @@ -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 { + } }; //===----------------------------------------------------------------------===// -- 2.40.0