vsk updated this revision to Diff 114433.
vsk added a comment.
Herald added subscribers: kbarton, nemanjai.

- Add an 'EndifLoc' parameter to the SourceRangeSkipped callback so that 
indexing clients can preserve their existing behavior.
- I'll submit a follow-up patch which updates the pp-trace tests.


https://reviews.llvm.org/D36642

Files:
  include/clang/Lex/PPCallbacks.h
  include/clang/Lex/PreprocessingRecord.h
  include/clang/Lex/Preprocessor.h
  lib/CodeGen/CoverageMappingGen.cpp
  lib/CodeGen/CoverageMappingGen.h
  lib/Lex/PPDirectives.cpp
  lib/Lex/PreprocessingRecord.cpp
  test/CoverageMapping/preprocessor.c
  test/Index/skipped-ranges.c
  tools/libclang/Indexing.cpp

Index: tools/libclang/Indexing.cpp
===================================================================
--- tools/libclang/Indexing.cpp
+++ tools/libclang/Indexing.cpp
@@ -272,7 +272,8 @@
   /// 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 {
+  }
 };
 
 //===----------------------------------------------------------------------===//
Index: test/Index/skipped-ranges.c
===================================================================
--- test/Index/skipped-ranges.c
+++ test/Index/skipped-ranges.c
@@ -20,6 +20,6 @@
 #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]
Index: test/CoverageMapping/preprocessor.c
===================================================================
--- test/CoverageMapping/preprocessor.c
+++ 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;
 }
Index: lib/Lex/PreprocessingRecord.cpp
===================================================================
--- lib/Lex/PreprocessingRecord.cpp
+++ lib/Lex/PreprocessingRecord.cpp
@@ -400,8 +400,9 @@
                       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,
Index: lib/Lex/PPDirectives.cpp
===================================================================
--- lib/Lex/PPDirectives.cpp
+++ lib/Lex/PPDirectives.cpp
@@ -79,7 +79,8 @@
 }
 
 /// \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 @@
 /// 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 @@
   // 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 @@
     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 @@
 /// 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 @@
   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 @@
                                      /*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 @@
                                    /*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 @@
 
 /// 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 @@
   }
 
   // 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 @@
   }
 
   // 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());
 }
Index: lib/CodeGen/CoverageMappingGen.h
===================================================================
--- lib/CodeGen/CoverageMappingGen.h
+++ lib/CodeGen/CoverageMappingGen.h
@@ -39,7 +39,7 @@
 public:
   ArrayRef<SourceRange> getSkippedRanges() const { return SkippedRanges; }
 
-  void SourceRangeSkipped(SourceRange Range) override;
+  void SourceRangeSkipped(SourceRange Range, SourceLocation EndifLoc) override;
 };
 
 namespace CodeGen {
Index: lib/CodeGen/CoverageMappingGen.cpp
===================================================================
--- lib/CodeGen/CoverageMappingGen.cpp
+++ lib/CodeGen/CoverageMappingGen.cpp
@@ -29,7 +29,7 @@
 using namespace CodeGen;
 using namespace llvm::coverage;
 
-void CoverageSourceInfo::SourceRangeSkipped(SourceRange Range) {
+void CoverageSourceInfo::SourceRangeSkipped(SourceRange Range, SourceLocation) {
   SkippedRanges.push_back(Range);
 }
 
Index: include/clang/Lex/Preprocessor.h
===================================================================
--- include/clang/Lex/Preprocessor.h
+++ include/clang/Lex/Preprocessor.h
@@ -1837,7 +1837,8 @@
   /// \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 @@
   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,
Index: include/clang/Lex/PreprocessingRecord.h
===================================================================
--- include/clang/Lex/PreprocessingRecord.h
+++ include/clang/Lex/PreprocessingRecord.h
@@ -504,7 +504,8 @@
     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);
Index: include/clang/Lex/PPCallbacks.h
===================================================================
--- include/clang/Lex/PPCallbacks.h
+++ include/clang/Lex/PPCallbacks.h
@@ -266,7 +266,10 @@
   /// \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 @@
     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.
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to