Move the isSelfContained function from clangd to libtooling.

[hokein]

Summary

We plan to reuse it in the include-cleaner library, this patch moves
this functionality from clangd to libtooling, so that this piece of code can be
shared among all clang tools.

[kadircet]

clang/include/clang/Tooling/Inclusions/Header.h

1 ↗

(On Diff #474188)

rather than calling this Header can we call this HeaderAnalysis?

21 ↗

(On Diff #474188)

let's mention being #importd as well

clang/lib/Tooling/Inclusions/Header.cpp

64 ↗

(On Diff #474188)

translateFile actually does a linear scan over all the slocentries, so i think it's better for this API to be based on FileID. (later on we can easily get fileentry from fileid in constant time)

clang/unittests/Tooling/HeaderTest.cpp

21 ↗

(On Diff #474188)

can you also add a test case for #import ?

[hokein]

address comments:

• mention #import case
• rename file name
• API: FileEntry => FileID

[sammccall]

clang/lib/Tooling/Inclusions/Header.cpp

64 ↗

(On Diff #474188)

You can get content through the fileentry directly, no?

[kadircet]

clang/include/clang/Tooling/Inclusions/Header.h

21 ↗

(On Diff #474188)

again comment seems to be the same.

clang/lib/Tooling/Inclusions/Header.cpp

64 ↗

(On Diff #474188)

oh thanks for reminding that i forgot to mention it here, yes we can get that, but that would misbehave in the case of virtual/remapped files. so can you actually add a comment here about why we should get the contents from source manager?

clang/unittests/Tooling/HeaderTest.cpp

21 ↗

(On Diff #474188)

this is marked as done, but i can't see any #import statements here. am i missing something?

[sammccall]

clang/lib/Tooling/Inclusions/Header.cpp

64 ↗

(On Diff #474188)

But even when that remapping happens at the sourcemanager level, it happens on fileentries, not FileIDs (how would the latter even work?)

So SM.getMemoryBufferForFileOrNone(FileEntry)?

[kadircet]

clang/lib/Tooling/Inclusions/Header.cpp

64 ↗

(On Diff #474188)

oh i didn't know about getMemoryBufferForFileOrNone, looks like it should do.

But even when that remapping happens at the sourcemanager level, it happens on fileentries, not FileIDs (how would the latter even work?)

i was just looking at contentcache, which is a 1:1 mapping for each FileID and has fileentries, one for the original file and the other for providing the contents (or a virtual buffer). i didn't notice the overrideinfo map was also exposed.

[hokein]

getBufferData => getMemoryBufferForFileOrNone

clang/include/clang/Tooling/Inclusions/Header.h

21 ↗

(On Diff #474188)

you're looking at the old version, this file has been renamed, please see the https://reviews.llvm.org/D137697.

[kadircet]

thanks, lgtm!

[kadircet]

clang/include/clang/Tooling/Inclusions/HeaderAnalysis.h

29

oops sorry, missed this part. the idea with using getmemorybuffer was to use fileentries here.

[hokein]

use FileEntry in the API.

[kadircet]

thanks!

[nridge]

nit: isSelfContained --> isSelfContainedHeader in commit message

[Hahnfeld]

@hokein it seems the unit test wasn't updated for the last-minute API change, which breaks check-all for me. I've posted https://reviews.llvm.org/D137927, could you take a look if that change is correct?

[hokein]

In D137697#3924305, @Hahnfeld wrote:

@hokein it seems the unit test wasn't updated for the last-minute API change, which breaks check-all for me. I've posted https://reviews.llvm.org/D137927, could you take a look if that change is correct?

oops, thanks, and your analysis is right! (I landed a fix in 5c4ae8a86a865e622e2c663666c0c139334b5da2 before reading your email, sorry).

[Hahnfeld]

No worries, thanks for fixing so quickly! I just happened to pull in the wrong moment...

clang-tools-extra/clangd/Headers.cpp

 #include "Preamble.h"
 #include "SourceCode.h"
 #include "clang/Basic/SourceLocation.h"
 #include "clang/Basic/SourceManager.h"
 #include "clang/Frontend/CompilerInstance.h"
 #include "clang/Lex/HeaderSearch.h"
 #include "clang/Lex/PPCallbacks.h"
 #include "clang/Lex/Preprocessor.h"
+#include "clang/Tooling/Inclusions/HeaderAnalysis.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/Path.h"
 #include <cstring>
 
 namespace clang {
 namespace clangd {
 
 llvm::Optional<StringRef> parseIWYUPragma(const char *Text) {
case PPCallbacks::ExitFile: {
       if (PrevFID == BuiltinFile)
         InBuiltinFile = false;
       // At file exit time HeaderSearchInfo is valid and can be used to
       // determine whether the file was a self-contained header or not.
       if (const FileEntry *FE = SM.getFileEntryForID(PrevFID)) {
         // isSelfContainedHeader only returns true once the full header-guard
         // structure has been seen, i.e. when exiting the *outer* copy of the
         // file. So last result wins.
-        if (isSelfContainedHeader(FE, PrevFID, SM, HeaderInfo))
-          Out->NonSelfContained.erase(
-              *Out->getID(SM.getFileEntryForID(PrevFID)));
+        if (tooling::isSelfContainedHeader(FE, SM, HeaderInfo))
+          Out->NonSelfContained.erase(*Out->getID(FE));
         else
-          Out->NonSelfContained.insert(
-              *Out->getID(SM.getFileEntryForID(PrevFID)));
+          Out->NonSelfContained.insert(*Out->getID(FE));
       }
       break;
     }
     case PPCallbacks::RenameFile:
     case PPCallbacks::SystemHeaderPragma:
       break;
     }
   }

clang-tools-extra/clangd/SourceCode.h

 /// Infers whether this is a header from the FileName and LangOpts (if
 /// presents).
 bool isHeaderFile(llvm::StringRef FileName,
                   llvm::Optional<LangOptions> LangOpts = llvm::None);
 
 /// Returns true if the given location is in a generated protobuf file.
 bool isProtoFile(SourceLocation Loc, const SourceManager &SourceMgr);
 
-/// This scans source code, and should not be called when using a preamble.
-/// Prefer to access the cache in IncludeStructure::isSelfContained if you can.
-bool isSelfContainedHeader(const FileEntry *FE, FileID ID,
-                           const SourceManager &SM, HeaderSearch &HeaderInfo);
-
 /// Returns true if Name is reserved, like _Foo or __Vector_base.
 inline bool isReservedName(llvm::StringRef Name) {
   // This doesn't catch all cases, but the most common.
   return Name.size() >= 2 && Name[0] == '_' &&
          (isUppercase(Name[1]) || Name[1] == '_');
 }
 
 } // namespace clangd
 } // namespace clang
 #endif

clang-tools-extra/clangd/SourceCode.cpp

bool isProtoFile(SourceLocation Loc, const SourceManager &SM) {
   auto FID = SM.getFileID(Loc);
   // All proto generated headers should start with this line.
   static const char *ProtoHeaderComment =
       "// Generated by the protocol buffer compiler.  DO NOT EDIT!";
   // Double check that this is an actual protobuf header.
   return SM.getBufferData(FID).startswith(ProtoHeaderComment);
 }
 
-namespace {
-
-// Is Line an #if or #ifdef directive?
-// FIXME: This makes headers with #ifdef LINUX/WINDOWS/MACOS marked as non
-// self-contained and is probably not what we want.
-bool isIf(llvm::StringRef Line) {
-  Line = Line.ltrim();
-  if (!Line.consume_front("#"))
-    return false;
-  Line = Line.ltrim();
-  return Line.startswith("if");
-}
-
-// Is Line an #error directive mentioning includes?
-bool isErrorAboutInclude(llvm::StringRef Line) {
-  Line = Line.ltrim();
-  if (!Line.consume_front("#"))
-    return false;
-  Line = Line.ltrim();
-  if (!Line.startswith("error"))
-    return false;
-  return Line.contains_insensitive(
-      "includ"); // Matches "include" or "including".
-}
-
-// Heuristically headers that only want to be included via an umbrella.
-bool isDontIncludeMeHeader(llvm::StringRef Content) {
-  llvm::StringRef Line;
-  // Only sniff up to 100 lines or 10KB.
-  Content = Content.take_front(100 * 100);
-  for (unsigned I = 0; I < 100 && !Content.empty(); ++I) {
-    std::tie(Line, Content) = Content.split('\n');
-    if (isIf(Line) && isErrorAboutInclude(Content.split('\n').first))
-      return true;
-  }
-  return false;
-}
-
-} // namespace
-
-bool isSelfContainedHeader(const FileEntry *FE, FileID FID,
-                           const SourceManager &SM, HeaderSearch &HeaderInfo) {
-  // FIXME: Should files that have been #import'd be considered
-  // self-contained? That's really a property of the includer,
-  // not of the file.
-  if (!HeaderInfo.isFileMultipleIncludeGuarded(FE) &&
-      !HeaderInfo.hasFileBeenImported(FE))
-    return false;
-  // This pattern indicates that a header can't be used without
-  // particular preprocessor state, usually set up by another header.
-  return !isDontIncludeMeHeader(SM.getBufferData(FID));
-}
-
 } // namespace clangd
 } // namespace clang

clang-tools-extra/clangd/index/SymbolCollector.cpp

 #include "clang/AST/DeclTemplate.h"
 #include "clang/AST/DeclarationName.h"
 #include "clang/Basic/LangOptions.h"
 #include "clang/Basic/SourceLocation.h"
 #include "clang/Basic/SourceManager.h"
 #include "clang/Index/IndexSymbol.h"
 #include "clang/Lex/Preprocessor.h"
 #include "clang/Lex/Token.h"
+#include "clang/Tooling/Inclusions/HeaderAnalysis.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/Support/Casting.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/Path.h"
 
 namespace clang {
 namespace clangd {
 namespace {
llvm::StringRef getIncludeHeaderUncached(FileID FID) {
     // "path/FrameworkName.framework/Headers/Foo.h".
     auto &HS = PP->getHeaderSearchInfo();
     if (const auto *HFI = HS.getExistingFileInfo(FE, /*WantExternal*/ false))
       if (!HFI->Framework.empty())
         if (auto Spelling =
                 getFrameworkHeaderIncludeSpelling(FE, HFI->Framework, HS))
           return *Spelling;
 
-    if (!isSelfContainedHeader(FE, FID, PP->getSourceManager(),
+    if (!tooling::isSelfContainedHeader(FE, PP->getSourceManager(),
                                         PP->getHeaderSearchInfo())) {
       // A .inc or .def file is often included into a real header to define
       // symbols (e.g. LLVM tablegen files).
       if (Filename.endswith(".inc") || Filename.endswith(".def"))
         // Don't use cache reentrantly due to iterator invalidation.
         return getIncludeHeaderUncached(SM.getFileID(SM.getIncludeLoc(FID)));
       // Conservatively refuse to insert #includes to files without guards.
       return "";
     }

clang/include/clang/Tooling/Inclusions/HeaderAnalysis.h

+//===--- HeaderAnalysis.h -----------------------------------------*-C++-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_TOOLING_INCLUSIONS_HEADER_ANALYSIS_H
+#define LLVM_CLANG_TOOLING_INCLUSIONS_HEADER_ANALYSIS_H
+
+namespace clang {
+class FileEntry;
+class SourceManager;
+class HeaderSearch;
+
+namespace tooling {
+
+/// Returns true if the given physical file is a self-contained header.
+///
+/// A header is considered self-contained if
+//   - it has a proper header guard or has been #imported
+//   - *and* it doesn't have a dont-include-me pattern.
+///
+/// This function can be expensive as it may scan the source code to find out
+/// dont-include-me pattern heuristically.
+bool isSelfContainedHeader(const FileEntry *FE, const SourceManager &SM,
+                           HeaderSearch &HeaderInfo);
+
+} // namespace tooling
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLING_INCLUSIONS_HEADER_ANALYSIS_H

clang/lib/Tooling/Inclusions/CMakeLists.txt

 set(LLVM_LINK_COMPONENTS support)
 
 add_clang_library(clangToolingInclusions
+  HeaderAnalysis.cpp
   HeaderIncludes.cpp
   IncludeStyle.cpp
 
   LINK_LIBS
   clangBasic
   clangLex
   clangRewrite
   clangToolingCore
   )
 
   add_subdirectory(Stdlib)

clang/lib/Tooling/Inclusions/HeaderAnalysis.cpp

+//===--- HeaderAnalysis.cpp -------------------------------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "clang/Tooling/Inclusions/HeaderAnalysis.h"
+#include "clang/Basic/SourceLocation.h"
+#include "clang/Lex/HeaderSearch.h"
+
+namespace clang::tooling {
+namespace {
+
+// Is Line an #if or #ifdef directive?
+// FIXME: This makes headers with #ifdef LINUX/WINDOWS/MACOS marked as non
+// self-contained and is probably not what we want.
+bool isIf(llvm::StringRef Line) {
+  Line = Line.ltrim();
+  if (!Line.consume_front("#"))
+    return false;
+  Line = Line.ltrim();
+  return Line.startswith("if");
+}
+
+// Is Line an #error directive mentioning includes?
+bool isErrorAboutInclude(llvm::StringRef Line) {
+  Line = Line.ltrim();
+  if (!Line.consume_front("#"))
+    return false;
+  Line = Line.ltrim();
+  if (!Line.startswith("error"))
+    return false;
+  return Line.contains_insensitive(
+      "includ"); // Matches "include" or "including".
+}
+
+// Heuristically headers that only want to be included via an umbrella.
+bool isDontIncludeMeHeader(llvm::MemoryBufferRef Buffer) {
+  StringRef Content = Buffer.getBuffer();
+  llvm::StringRef Line;
+  // Only sniff up to 100 lines or 10KB.
+  Content = Content.take_front(100 * 100);
+  for (unsigned I = 0; I < 100 && !Content.empty(); ++I) {
+    std::tie(Line, Content) = Content.split('\n');
+    if (isIf(Line) && isErrorAboutInclude(Content.split('\n').first))
+      return true;
+  }
+  return false;
+}
+
+} // namespace
+
+bool isSelfContainedHeader(const FileEntry *FE, const SourceManager &SM,
+                           HeaderSearch &HeaderInfo) {
+  assert(FE);
+  if (!HeaderInfo.isFileMultipleIncludeGuarded(FE) &&
+      !HeaderInfo.hasFileBeenImported(FE))
+    return false;
+  // This pattern indicates that a header can't be used without
+  // particular preprocessor state, usually set up by another header.
+  return !isDontIncludeMeHeader(
+      const_cast<SourceManager &>(SM).getMemoryBufferForFileOrNone(FE).value_or(
+          llvm::MemoryBufferRef()));
+}
+} // namespace clang::tooling

clang/unittests/Tooling/CMakeLists.txt

add_clang_unittest(ToolingTests
   ASTSelectionTest.cpp
   CastExprTest.cpp
   CommentHandlerTest.cpp
   CompilationDatabaseTest.cpp
   DependencyScannerTest.cpp
   DiagnosticsYamlTest.cpp
   ExecutionTest.cpp
   FixItTest.cpp
+  HeaderAnalysisTest.cpp
   HeaderIncludesTest.cpp
   StandardLibraryTest.cpp
   LexicallyOrderedRecursiveASTVisitorTest.cpp
   LookupTest.cpp
   QualTypeNamesTest.cpp
   RangeSelectorTest.cpp
   RecursiveASTVisitorTests/Attr.cpp
   RecursiveASTVisitorTests/CallbacksLeaf.cpp

clang/unittests/Tooling/HeaderAnalysisTest.cpp

+//===- unittest/Tooling/HeaderAnalysisTest.cpp ----------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "clang/Tooling/Inclusions/HeaderAnalysis.h"
+#include "clang/Lex/Preprocessor.h"
+#include "clang/Testing/TestAST.h"
+#include "gtest/gtest.h"
+
+namespace clang {
+namespace tooling {
+namespace {
+
+TEST(HeaderAnalysisTest, IsSelfContained) {
+  TestInputs Inputs;
+  Inputs.Code = R"cpp(
+  #include "headerguard.h"
+  #include "pragmaonce.h"
+  #import "imported.h"
+
+  #include "bad.h"
+  #include "unguarded.h"
+  )cpp";
+
+  Inputs.ExtraFiles["headerguard.h"] = R"cpp(
+  #ifndef HEADER_H
+  #define HEADER_H
+
+  #endif HEADER_H
+  )cpp";
+  Inputs.ExtraFiles["pragmaonce.h"] = R"cpp(
+  #pragma once
+  )cpp";
+  Inputs.ExtraFiles["imported.h"] = "";
+
+  Inputs.ExtraFiles["unguarded.h"] = "";
+  Inputs.ExtraFiles["bad.h"] = R"cpp(
+  #pragma once
+
+  #if defined(INSIDE_H)
+  #error "Only ... can be included directly"
+  #endif
+  )cpp";
+
+  TestAST AST(Inputs);
+  const auto &SM = AST.sourceManager();
+  auto &FM = SM.getFileManager();
+  auto &HI = AST.preprocessor().getHeaderSearchInfo();
+  auto getFileID = [&](llvm::StringRef FileName) {
+    return SM.translateFile(FM.getFile(FileName).get());
+  };
+  EXPECT_TRUE(isSelfContainedHeader(getFileID("headerguard.h"), SM, HI));
+  EXPECT_TRUE(isSelfContainedHeader(getFileID("pragmaonce.h"), SM, HI));
+  EXPECT_TRUE(isSelfContainedHeader(getFileID("imported.h"), SM, HI));
+
+  EXPECT_FALSE(isSelfContainedHeader(getFileID("unguarded.h"), SM, HI));
+  EXPECT_FALSE(isSelfContainedHeader(getFileID("bad.h"), SM, HI));
+}
+
+} // namespace
+} // namespace tooling
+} // namespace clang