[include-cleaner] Handle files with unnamed buffers

[kadircet]

Summary

Some tools can register virtual buffers without identifiers into the
filemanager. Make sure we can handle pragmas in such cases.

[lei]

This is causing a ld.lld link failure in https://lab.llvm.org/buildbot/#/builders/57

FAILED: tools/clang/tools/extra/include-cleaner/unittests/ClangIncludeCleanerTests 
: && /home/buildbots/clang.15.0.4/bin/clang++ --gcc-toolchain=/usr -fPIC -fno-semantic-interposition -fvisibility-inlines-hidden -Werror -Werror=date-time -Werror=unguarded-availability-new -Wall -Wextra -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wmissing-field-initializers -pedantic -Wno-long-long -Wc++98-compat-extra-semi -Wimplicit-fallthrough -Wcovered-switch-default -Wno-noexcept-type -Wnon-virtual-dtor -Wdelete-non-virtual-dtor -Wsuggest-override -Wstring-conversion -Wmisleading-indentation -Wctad-maybe-unsupported -fdiagnostics-color -ffunction-sections -fdata-sections -fno-common -Woverloaded-virtual -Wno-nested-anon-types -O3 -DNDEBUG -Wl,--color-diagnostics     -Wl,--gc-sections tools/clang/tools/extra/include-cleaner/unittests/CMakeFiles/ClangIncludeCleanerTests.dir/AnalysisTest.cpp.o tools/clang/tools/extra/include-cleaner/unittests/CMakeFiles/ClangIncludeCleanerTests.dir/FindHeadersTest.cpp.o tools/clang/tools/extra/include-cleaner/unittests/CMakeFiles/ClangIncludeCleanerTests.dir/IncludeSpellerTest.cpp.o tools/clang/tools/extra/include-cleaner/unittests/CMakeFiles/ClangIncludeCleanerTests.dir/LocateSymbolTest.cpp.o tools/clang/tools/extra/include-cleaner/unittests/CMakeFiles/ClangIncludeCleanerTests.dir/RecordTest.cpp.o tools/clang/tools/extra/include-cleaner/unittests/CMakeFiles/ClangIncludeCleanerTests.dir/TypesTest.cpp.o tools/clang/tools/extra/include-cleaner/unittests/CMakeFiles/ClangIncludeCleanerTests.dir/WalkASTTest.cpp.o -o tools/clang/tools/extra/include-cleaner/unittests/ClangIncludeCleanerTests  -Wl,-rpath,/home/buildbots/docker-RHEL84-buildbot/SetupBot/worker_env/ppc64le-clang-rhel-test/clang-ppc64le-rhel/build/lib  -lpthread  lib/libllvm_gtest_main.so.18git  -lpthread  lib/libclangIncludeCleaner.so.18git  lib/libclangTesting.so.18git  lib/libLLVMTestingAnnotations.so.18git  lib/libLLVMTestingSupport.so.18git  lib/libclangFormat.so.18git  lib/libclangToolingInclusionsStdlib.so.18git  lib/libllvm_gtest.so.18git  lib/libclangFrontend.so.18git  lib/libclangAST.so.18git  lib/libclangLex.so.18git  lib/libclangBasic.so.18git  lib/libLLVMSupport.so.18git  -Wl,-rpath-link,/home/buildbots/docker-RHEL84-buildbot/SetupBot/worker_env/ppc64le-clang-rhel-test/clang-ppc64le-rhel/build/lib && :
ld.lld: error: undefined symbol: clang::PCHContainerOperations::PCHContainerOperations()
>>> referenced by RecordTest.cpp
>>>               tools/clang/tools/extra/include-cleaner/unittests/CMakeFiles/ClangIncludeCleanerTests.dir/RecordTest.cpp.o:(clang::include_cleaner::(anonymous namespace)::PragmaIncludeTest_ExportInUnnamedBuffer_Test::TestBody())
clang-15: error: linker command failed with exit code 1 (use -v to see invocation)

Maybe a lib is needed?

[tylanphear]

Seeing the link error downstream. I think clangSerialization needs to be added to the link libraries.

[RKSimon]

In D157078#4562788, @tylanphear wrote:

Seeing the link error downstream. I think clangSerialization needs to be added to the link libraries.

Should be fixed by 36daf3532d91bb

[tylanphear]

In D157078#4563212, @RKSimon wrote:

In D157078#4562788, @tylanphear wrote:

Seeing the link error downstream. I think clangSerialization needs to be added to the link libraries.

Should be fixed by 36daf3532d91bb

Perfect. Thanks for the quick fix.

clang-tools-extra/include-cleaner/lib/Record.cpp

bool HandleComment(Preprocessor &PP, SourceRange Range) override {
     auto &SM = PP.getSourceManager();
     auto Pragma =
         tooling::parseIWYUPragma(SM.getCharacterData(Range.getBegin()));
     if (!Pragma)
       return false;
 
     auto [CommentFID, CommentOffset] = SM.getDecomposedLoc(Range.getBegin());
     int CommentLine = SM.getLineNumber(CommentFID, CommentOffset);
-    auto Filename = SM.getBufferName(Range.getBegin());
-    // Record export pragma.
-    if (Pragma->startswith("export")) {
-      ExportStack.push_back({CommentLine, CommentFID, save(Filename), false});
-    } else if (Pragma->startswith("begin_exports")) {
-      ExportStack.push_back({CommentLine, CommentFID, save(Filename), true});
-    } else if (Pragma->startswith("end_exports")) {
-      // FIXME: be robust on unmatching cases. We should only pop the stack if
-      // the begin_exports and end_exports is in the same file.
-      if (!ExportStack.empty()) {
-        assert(ExportStack.back().Block);
-        ExportStack.pop_back();
-      }
-    }
 
     if (InMainFile) {
       if (Pragma->startswith("keep")) {
         KeepStack.push_back({CommentLine, false});
       } else if (Pragma->starts_with("begin_keep")) {
         KeepStack.push_back({CommentLine, true});
       } else if (Pragma->starts_with("end_keep") && !KeepStack.empty()) {
         assert(KeepStack.back().Block);
         KeepStack.pop_back();
       }
     }
 
     auto FE = SM.getFileEntryRefForID(CommentFID);
     if (!FE) {
-      // FIXME: Support IWYU pragmas in virtual files. Our mappings rely on
-      // "persistent" UniqueIDs and that is not the case for virtual files.
+      // This can only happen when the buffer was registered virtually into
+      // SourceManager and FileManager has no idea about it. In such a scenario,
+      // that file cannot be discovered by HeaderSearch, therefore no "explicit"
+      // includes for that file.
       return false;
     }
     auto CommentUID = FE->getUniqueID();
     if (Pragma->consume_front("private")) {
       StringRef PublicHeader;
       if (Pragma->consume_front(", include ")) {
         // We always insert using the spelling from the pragma.
         PublicHeader = save(Pragma->startswith("<") || Pragma->startswith("\"")
                                 ? (*Pragma)
                                 : ("\"" + *Pragma + "\"").str());
       }
       Out->IWYUPublic.insert({CommentUID, PublicHeader});
       return false;
     }
     if (Pragma->consume_front("always_keep")) {
       Out->ShouldKeep.insert(CommentUID);
       return false;
     }
+    auto Filename = FE->getName();
+    // Record export pragma.
+    if (Pragma->startswith("export")) {
+      ExportStack.push_back({CommentLine, CommentFID, save(Filename), false});
+    } else if (Pragma->startswith("begin_exports")) {
+      ExportStack.push_back({CommentLine, CommentFID, save(Filename), true});
+    } else if (Pragma->startswith("end_exports")) {
+      // FIXME: be robust on unmatching cases. We should only pop the stack if
+      // the begin_exports and end_exports is in the same file.
+      if (!ExportStack.empty()) {
+        assert(ExportStack.back().Block);
+        ExportStack.pop_back();
+      }
+    }
     return false;
   }
 
 private:
   StringRef save(llvm::StringRef S) { return UniqueStrings.save(S); }
 
   bool InMainFile = false;
   const SourceManager &SM;

clang-tools-extra/include-cleaner/unittests/RecordTest.cpp

 //===-- RecordTest.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-include-cleaner/Record.h"
+#include "clang-include-cleaner/Types.h"
+#include "clang/AST/Decl.h"
+#include "clang/Basic/Diagnostic.h"
+#include "clang/Basic/LLVM.h"
 #include "clang/Basic/SourceLocation.h"
+#include "clang/Frontend/CompilerInvocation.h"
 #include "clang/Frontend/FrontendAction.h"
 #include "clang/Frontend/FrontendActions.h"
+#include "clang/Frontend/FrontendOptions.h"
+#include "clang/Serialization/PCHContainerOperations.h"
 #include "clang/Testing/TestAST.h"
 #include "clang/Tooling/Inclusions/StandardLibrary.h"
 #include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/IntrusiveRefCntPtr.h"
 #include "llvm/ADT/StringRef.h"
+#include "llvm/Support/Error.h"
+#include "llvm/Support/MemoryBuffer.h"
+#include "llvm/Support/VirtualFileSystem.h"
 #include "llvm/Support/raw_ostream.h"
 #include "llvm/Testing/Annotations/Annotations.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
 #include <cassert>
+#include <memory>
+#include <optional>
+#include <utility>
 
 namespace clang::include_cleaner {
 namespace {
 using testing::ElementsAreArray;
 using testing::IsEmpty;
 
 // Matches a Decl* if it is a NamedDecl with the given name.
 MATCHER_P(named, N, "") {
TEST_F(PragmaIncludeTest, AlwaysKeep) {
   // IWYU pragma: always_keep
   )cpp";
   Inputs.ExtraFiles["usual.h"] = "#pragma once";
   TestAST Processed = build();
   auto &FM = Processed.fileManager();
   EXPECT_TRUE(PI.shouldKeep(FM.getFile("always_keep.h").get()));
   EXPECT_FALSE(PI.shouldKeep(FM.getFile("usual.h").get()));
 }
+
+TEST_F(PragmaIncludeTest, ExportInUnnamedBuffer) {
+  llvm::StringLiteral Filename = "test.cpp";
+  auto Code = R"cpp(#include "exporter.h")cpp";
+  Inputs.ExtraFiles["exporter.h"] = R"cpp(
+  #pragma once
+  #include "foo.h" // IWYU pragma: export
+  )cpp";
+  Inputs.ExtraFiles["foo.h"] = "";
+
+  auto Clang = std::make_unique<CompilerInstance>(
+      std::make_shared<PCHContainerOperations>());
+  Clang->createDiagnostics();
+
+  Clang->setInvocation(std::make_unique<CompilerInvocation>());
+  ASSERT_TRUE(CompilerInvocation::CreateFromArgs(
+      Clang->getInvocation(), {Filename.data()}, Clang->getDiagnostics(),
+      "clang"));
+
+  // Create unnamed memory buffers for all the files.
+  auto VFS = llvm::makeIntrusiveRefCnt<llvm::vfs::InMemoryFileSystem>();
+  VFS->addFile(Filename, /*ModificationTime=*/0,
+               llvm::MemoryBuffer::getMemBufferCopy(Code, /*BufferName=*/""));
+  for (const auto &Extra : Inputs.ExtraFiles)
+    VFS->addFile(Extra.getKey(), /*ModificationTime=*/0,
+                 llvm::MemoryBuffer::getMemBufferCopy(Extra.getValue(),
+                                                      /*BufferName=*/""));
+  auto *FM = Clang->createFileManager(VFS);
+  ASSERT_TRUE(Clang->ExecuteAction(*Inputs.MakeAction()));
+  EXPECT_THAT(
+      PI.getExporters(llvm::cantFail(FM->getFileRef("foo.h")), *FM),
+      testing::ElementsAre(llvm::cantFail(FM->getFileRef("exporter.h"))));
+}
 } // namespace
 } // namespace clang::include_cleaner