subject:"\[PATCH\] D157078\: \[include\-cleaner\] Handle files with unnamed buffers"

[PATCH] D157078: [include-cleaner] Handle files with unnamed buffers

2023-08-05 Thread Tyler Lanphear via Phabricator via cfe-commits

tylanphear added a comment.

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.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D157078/new/

https://reviews.llvm.org/D157078

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D157078: [include-cleaner] Handle files with unnamed buffers

2023-08-05 Thread Simon Pilgrim via Phabricator via cfe-commits

RKSimon added a comment.

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 



Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D157078/new/

https://reviews.llvm.org/D157078

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D157078: [include-cleaner] Handle files with unnamed buffers

2023-08-04 Thread Tyler Lanphear via Phabricator via cfe-commits

tylanphear added a comment.

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


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D157078/new/

https://reviews.llvm.org/D157078

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D157078: [include-cleaner] Handle files with unnamed buffers

2023-08-04 Thread Lei Huang via Phabricator via cfe-commits

lei added a comment.

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?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D157078/new/

https://reviews.llvm.org/D157078

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D157078: [include-cleaner] Handle files with unnamed buffers

2023-08-04 Thread Kadir Cetinkaya via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rG31873d3fca38: [include-cleaner] Handle files with unnamed 
buffers (authored by kadircet).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D157078/new/

https://reviews.llvm.org/D157078

Files:
  clang-tools-extra/include-cleaner/lib/Record.cpp
  clang-tools-extra/include-cleaner/unittests/RecordTest.cpp

Index: clang-tools-extra/include-cleaner/unittests/RecordTest.cpp
===
--- clang-tools-extra/include-cleaner/unittests/RecordTest.cpp
+++ clang-tools-extra/include-cleaner/unittests/RecordTest.cpp
@@ -7,18 +7,32 @@
 //===--===//
 
 #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 
+#include 
+#include 
+#include 
 
 namespace clang::include_cleaner {
 namespace {
@@ -509,5 +523,38 @@
   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(
+  std::make_shared());
+  Clang->createDiagnostics();
+
+  Clang->setInvocation(std::make_unique());
+  ASSERT_TRUE(CompilerInvocation::CreateFromArgs(
+  Clang->getInvocation(), {Filename.data()}, Clang->getDiagnostics(),
+  "clang"));
+
+  // Create unnamed memory buffers for all the files.
+  auto VFS = llvm::makeIntrusiveRefCnt();
+  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
Index: clang-tools-extra/include-cleaner/lib/Record.cpp
===
--- clang-tools-extra/include-cleaner/lib/Record.cpp
+++ clang-tools-extra/include-cleaner/lib/Record.cpp
@@ -279,20 +279,6 @@
 
 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")) {
@@ -307,8 +293,10 @@
 
 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"
+  // incl

[PATCH] D157078: [include-cleaner] Handle files with unnamed buffers

2023-08-04 Thread Kadir Cetinkaya via Phabricator via cfe-commits

kadircet created this revision.
kadircet added a reviewer: hokein.
Herald added a subscriber: ChuanqiXu.
Herald added a project: All.
kadircet requested review of this revision.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.

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


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D157078

Files:
  clang-tools-extra/include-cleaner/lib/Record.cpp
  clang-tools-extra/include-cleaner/unittests/RecordTest.cpp

Index: clang-tools-extra/include-cleaner/unittests/RecordTest.cpp
===
--- clang-tools-extra/include-cleaner/unittests/RecordTest.cpp
+++ clang-tools-extra/include-cleaner/unittests/RecordTest.cpp
@@ -7,18 +7,32 @@
 //===--===//
 
 #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 
+#include 
+#include 
+#include 
 
 namespace clang::include_cleaner {
 namespace {
@@ -509,5 +523,38 @@
   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(
+  std::make_shared());
+  Clang->createDiagnostics();
+
+  Clang->setInvocation(std::make_unique());
+  ASSERT_TRUE(CompilerInvocation::CreateFromArgs(
+  Clang->getInvocation(), {Filename.data()}, Clang->getDiagnostics(),
+  "clang"));
+
+  // Create unnamed memory buffers for all the files.
+  auto VFS = llvm::makeIntrusiveRefCnt();
+  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
Index: clang-tools-extra/include-cleaner/lib/Record.cpp
===
--- clang-tools-extra/include-cleaner/lib/Record.cpp
+++ clang-tools-extra/include-cleaner/lib/Record.cpp
@@ -279,20 +279,6 @@
 
 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")) {
@@ -307,8 +293,10 @@
 
 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 FileManag

[PATCH] D157078: [include-cleaner] Handle files with unnamed buffers

[PATCH] D157078: [include-cleaner] Handle files with unnamed buffers

[PATCH] D157078: [include-cleaner] Handle files with unnamed buffers

[PATCH] D157078: [include-cleaner] Handle files with unnamed buffers

[PATCH] D157078: [include-cleaner] Handle files with unnamed buffers

[PATCH] D157078: [include-cleaner] Handle files with unnamed buffers

6 matches

Site Navigation

Mail list logo

Footer information