kbobyrev created this revision.
kbobyrev added a reviewer: sammccall.
Herald added subscribers: usaxena95, arphaman.
kbobyrev requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov.
Herald added a project: clang-tools-extra.

This will allow the IncludeCleaner to suppress warnings on the lines with "IWYU
pragma: keep".

Clang APIs are not very convinient, so the code has to navigate around it.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D114072

Files:
  clang-tools-extra/clangd/Headers.cpp
  clang-tools-extra/clangd/Headers.h
  clang-tools-extra/clangd/ParsedAST.cpp
  clang-tools-extra/clangd/Preamble.cpp
  clang-tools-extra/clangd/unittests/HeadersTests.cpp

Index: clang-tools-extra/clangd/unittests/HeadersTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/HeadersTests.cpp
+++ clang-tools-extra/clangd/unittests/HeadersTests.cpp
@@ -80,8 +80,9 @@
     EXPECT_TRUE(
         Action.BeginSourceFile(*Clang, Clang->getFrontendOpts().Inputs[0]));
     IncludeStructure Includes;
-    Clang->getPreprocessor().addPPCallbacks(
-        Includes.collect(Clang->getSourceManager()));
+    auto Collector = Includes.collect(Clang->getSourceManager());
+    Clang->getPreprocessor().addCommentHandler(Collector.get());
+    Clang->getPreprocessor().addPPCallbacks(move(Collector));
     EXPECT_FALSE(Action.Execute());
     Action.EndSourceFile();
     return Includes;
@@ -143,6 +144,7 @@
 MATCHER_P(Resolved, Name, "") { return arg.Resolved == Name; }
 MATCHER_P(IncludeLine, N, "") { return arg.HashLine == N; }
 MATCHER_P(Directive, D, "") { return arg.Directive == D; }
+MATCHER_P(HasPragmaKeep, H, "") { return arg.BehindPragmaKeep == H; }
 
 MATCHER_P2(Distance, File, D, "") {
   if (arg.getFirst() != File)
@@ -258,6 +260,18 @@
                                    Directive(tok::pp_include_next)));
 }
 
+TEST_F(HeadersTest, IWYUPragmaKeep) {
+  FS.Files[MainFile] = R"cpp(
+#include "bar.h" // IWYU pragma: keep
+#include "foo.h"
+)cpp";
+
+  EXPECT_THAT(
+      collectIncludes().MainFileIncludes,
+      UnorderedElementsAre(AllOf(Written("\"foo.h\""), HasPragmaKeep(false)),
+                           AllOf(Written("\"bar.h\""), HasPragmaKeep(true))));
+}
+
 TEST_F(HeadersTest, InsertInclude) {
   std::string Path = testPath("sub/bar.h");
   FS.Files[Path] = "";
Index: clang-tools-extra/clangd/Preamble.cpp
===================================================================
--- clang-tools-extra/clangd/Preamble.cpp
+++ clang-tools-extra/clangd/Preamble.cpp
@@ -286,7 +286,9 @@
   const auto &SM = Clang->getSourceManager();
   Preprocessor &PP = Clang->getPreprocessor();
   IncludeStructure Includes;
-  PP.addPPCallbacks(Includes.collect(SM));
+  auto Collector = Includes.collect(SM);
+  PP.addCommentHandler(Collector.get());
+  PP.addPPCallbacks(move(Collector));
   ScannedPreamble SP;
   SP.Bounds = Bounds;
   PP.addPPCallbacks(
Index: clang-tools-extra/clangd/ParsedAST.cpp
===================================================================
--- clang-tools-extra/clangd/ParsedAST.cpp
+++ clang-tools-extra/clangd/ParsedAST.cpp
@@ -442,8 +442,9 @@
   // Important: collectIncludeStructure is registered *after* ReplayPreamble!
   // Otherwise we would collect the replayed includes again...
   // (We can't *just* use the replayed includes, they don't have Resolved path).
-  Clang->getPreprocessor().addPPCallbacks(
-      Includes.collect(Clang->getSourceManager()));
+  auto Collector = Includes.collect(Clang->getSourceManager());
+  Clang->getPreprocessor().addCommentHandler(Collector.get());
+  Clang->getPreprocessor().addPPCallbacks(move(Collector));
   // Copy over the macros in the preamble region of the main file, and combine
   // with non-preamble macros below.
   MainFileMacros Macros;
Index: clang-tools-extra/clangd/Headers.h
===================================================================
--- clang-tools-extra/clangd/Headers.h
+++ clang-tools-extra/clangd/Headers.h
@@ -19,6 +19,7 @@
 #include "clang/Format/Format.h"
 #include "clang/Lex/HeaderSearch.h"
 #include "clang/Lex/PPCallbacks.h"
+#include "clang/Lex/Preprocessor.h"
 #include "clang/Tooling/Inclusions/HeaderIncludes.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/StringRef.h"
@@ -62,6 +63,7 @@
   int HashLine = 0;        // Line number containing the directive, 0-indexed.
   SrcMgr::CharacteristicKind FileKind = SrcMgr::C_User;
   llvm::Optional<unsigned> HeaderID;
+  bool BehindPragmaKeep = false; // Has IWYU pragma: keep right after.
 };
 llvm::raw_ostream &operator<<(llvm::raw_ostream &, const Inclusion &);
 bool operator==(const Inclusion &LHS, const Inclusion &RHS);
@@ -121,9 +123,21 @@
     RealPathNames.emplace_back();
   }
 
+  class IncludeCollector : public PPCallbacks, public CommentHandler {};
   // Returns a PPCallback that visits all inclusions in the main file and
-  // populates the structure.
-  std::unique_ptr<PPCallbacks> collect(const SourceManager &SM);
+  // populates the structure. IncludeCollector can also scan the comments for
+  // IWYU pragmas but it needs to be explicitly added to as a preprocessor
+  // comment handler. PP wants to own the PPCallbacks, so the typical way to
+  // do both is:
+  //
+  // auto Collector = collect(SM);
+  // PP.addCommentHandler(Colletor.get());
+  // PP.addPPCallbacks(move(Collector));
+  //
+  // When IncludeCollector is not added as the CommentHandler, resulting
+  // include structure won't have the IWYU pragmas information.
+  std::unique_ptr<IncludeStructure::IncludeCollector>
+  collect(const SourceManager &SM);
 
   // HeaderID identifies file in the include graph. It corresponds to a
   // FileEntry rather than a FileID, but stays stable across preamble & main
Index: clang-tools-extra/clangd/Headers.cpp
===================================================================
--- clang-tools-extra/clangd/Headers.cpp
+++ clang-tools-extra/clangd/Headers.cpp
@@ -17,6 +17,7 @@
 #include "clang/Frontend/CompilerInvocation.h"
 #include "clang/Frontend/FrontendActions.h"
 #include "clang/Lex/HeaderSearch.h"
+#include "clang/Lex/Preprocessor.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/Path.h"
 
@@ -24,7 +25,9 @@
 namespace clangd {
 namespace {
 
-class RecordHeaders : public PPCallbacks {
+const char IWYUPragmaKeep[] = "// IWYU pragma: keep";
+
+class RecordHeaders : public IncludeStructure::IncludeCollector {
 public:
   RecordHeaders(const SourceManager &SM, IncludeStructure *Out)
       : SM(SM), Out(Out) {}
@@ -58,6 +61,8 @@
       Inc.Directive = IncludeTok.getIdentifierInfo()->getPPKeywordID();
       if (File)
         Inc.HeaderID = static_cast<unsigned>(Out->getOrCreateID(File));
+      if (PragmaKeepInMainFile == Inc.HashLine)
+        Inc.BehindPragmaKeep = true;
     }
 
     // Record include graph (not just for main-file includes)
@@ -95,6 +100,35 @@
     }
   }
 
+  // Because PP wants to own the PPCallbacks, CommentHandler will be added first
+  // and will also be called before handling the include directive. Example:
+  //
+  // #include "foo.h"
+  // #include "bar.h" // IWYU pragma: keep
+  //
+  // The order in which the callbacks will be triggered:
+  //
+  // 1. InclusionDirective("foo.h")
+  // 2. HandleComment("// IWYU pragma: keep")
+  // 3. InclusionDirective("bar.h")
+  //
+  // HandleComment will store the last location of "IWYU pragma: keep" comment
+  // in the main file, so that when InclusionDirective is called, it will know
+  // that the next inclusion is behind the IWYU pragma.
+  bool HandleComment(Preprocessor &PP, SourceRange Range) override {
+    llvm::StringRef Text =
+        Lexer::getSourceText(CharSourceRange::getCharRange(Range),
+                             PP.getSourceManager(), PP.getLangOpts());
+    if (!Text.consume_front(IWYUPragmaKeep))
+      return false;
+    if (!isInsideMainFile(Range.getBegin(), SM))
+      return false;
+    unsigned Offset = SM.getFileOffset(Range.getBegin());
+    PragmaKeepInMainFile =
+        SM.getLineNumber(SM.getFileID(Range.getBegin()), Offset) - 1;
+    return false;
+  }
+
 private:
   const SourceManager &SM;
   // Set after entering the <built-in> file.
@@ -103,6 +137,9 @@
   bool InBuiltinFile = false;
 
   IncludeStructure *Out;
+
+  // The last line "IWYU pragma: keep" was seen in the main file.
+  int PragmaKeepInMainFile = -1;
 };
 
 } // namespace
@@ -151,7 +188,7 @@
   return Headers;
 }
 
-std::unique_ptr<PPCallbacks>
+std::unique_ptr<IncludeStructure::IncludeCollector>
 IncludeStructure::collect(const SourceManager &SM) {
   MainFileEntry = SM.getFileEntryForID(SM.getMainFileID());
   return std::make_unique<RecordHeaders>(SM, this);
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to