https://github.com/MythreyaK updated https://github.com/llvm/llvm-project/pull/155561
>From b0c182fc416a5fa1cd64ef3a95cd4fd05b0c8eef Mon Sep 17 00:00:00 2001 From: Mythreya Kuricheti <[email protected]> Date: Thu, 28 Aug 2025 01:27:22 -0700 Subject: [PATCH 1/4] Add `CaptureDiagsKind` to `buildASTFromCodeWithArgs` --- clang/include/clang/Tooling/Tooling.h | 2 ++ clang/lib/Tooling/Tooling.cpp | 2 +- clang/unittests/Tooling/ToolingTest.cpp | 2 +- 3 files changed, 4 insertions(+), 2 deletions(-) diff --git a/clang/include/clang/Tooling/Tooling.h b/clang/include/clang/Tooling/Tooling.h index 200fb30839a95..fd7a1de589a6f 100644 --- a/clang/include/clang/Tooling/Tooling.h +++ b/clang/include/clang/Tooling/Tooling.h @@ -32,6 +32,7 @@ #include "clang/AST/ASTConsumer.h" #include "clang/Basic/FileManager.h" #include "clang/Basic/LLVM.h" +#include "clang/Frontend/ASTUnit.h" #include "clang/Frontend/FrontendAction.h" #include "clang/Frontend/PCHContainerOperations.h" #include "clang/Tooling/ArgumentsAdjusters.h" @@ -238,6 +239,7 @@ std::unique_ptr<ASTUnit> buildASTFromCodeWithArgs( ArgumentsAdjuster Adjuster = getClangStripDependencyFileAdjuster(), const FileContentMappings &VirtualMappedFiles = FileContentMappings(), DiagnosticConsumer *DiagConsumer = nullptr, + CaptureDiagsKind CaptureKind = CaptureDiagsKind::None, IntrusiveRefCntPtr<llvm::vfs::FileSystem> BaseFS = llvm::vfs::getRealFileSystem()); diff --git a/clang/lib/Tooling/Tooling.cpp b/clang/lib/Tooling/Tooling.cpp index 45dfdf4360253..97061be8e300f 100644 --- a/clang/lib/Tooling/Tooling.cpp +++ b/clang/lib/Tooling/Tooling.cpp @@ -692,7 +692,7 @@ std::unique_ptr<ASTUnit> buildASTFromCodeWithArgs( StringRef Code, const std::vector<std::string> &Args, StringRef FileName, StringRef ToolName, std::shared_ptr<PCHContainerOperations> PCHContainerOps, ArgumentsAdjuster Adjuster, const FileContentMappings &VirtualMappedFiles, - DiagnosticConsumer *DiagConsumer, + DiagnosticConsumer *DiagConsumer, CaptureDiagsKind CaptureKind, IntrusiveRefCntPtr<llvm::vfs::FileSystem> BaseFS) { std::vector<std::unique_ptr<ASTUnit>> ASTs; ASTBuilderAction Action(ASTs); diff --git a/clang/unittests/Tooling/ToolingTest.cpp b/clang/unittests/Tooling/ToolingTest.cpp index 25e1d67eb2294..67572cbf2cf50 100644 --- a/clang/unittests/Tooling/ToolingTest.cpp +++ b/clang/unittests/Tooling/ToolingTest.cpp @@ -161,7 +161,7 @@ TEST(buildASTFromCode, FileSystem) { R"(#include "included_file.h")", {}, "input.cc", "clang-tool", std::make_shared<PCHContainerOperations>(), getClangStripDependencyFileAdjuster(), FileContentMappings(), nullptr, - InMemoryFileSystem); + CaptureDiagsKind::None, InMemoryFileSystem); ASSERT_TRUE(AST.get()); EXPECT_TRUE(FindClassDeclX(AST.get())); } >From b1a30c882b141583bea0ab791e64fa77715f53f3 Mon Sep 17 00:00:00 2001 From: Mythreya Kuricheti <[email protected]> Date: Thu, 28 Aug 2025 02:32:24 -0700 Subject: [PATCH 2/4] Add error check in `HeuristicResolverTest.cpp` --- clang/lib/Tooling/Tooling.cpp | 7 ++++- .../unittests/Sema/HeuristicResolverTest.cpp | 29 +++++++++++++++++++ 2 files changed, 35 insertions(+), 1 deletion(-) diff --git a/clang/lib/Tooling/Tooling.cpp b/clang/lib/Tooling/Tooling.cpp index 97061be8e300f..015ec8633b62d 100644 --- a/clang/lib/Tooling/Tooling.cpp +++ b/clang/lib/Tooling/Tooling.cpp @@ -658,13 +658,15 @@ class ASTBuilderAction : public ToolAction { Invocation->getDiagnosticOpts(), DiagConsumer, /*ShouldOwnClient=*/false), - Files); + Files, false, CaptureKinds); if (!AST) return false; ASTs.push_back(std::move(AST)); return true; } + + CaptureDiagsKind CaptureKinds{CaptureDiagsKind::None}; }; } // namespace @@ -695,7 +697,10 @@ std::unique_ptr<ASTUnit> buildASTFromCodeWithArgs( DiagnosticConsumer *DiagConsumer, CaptureDiagsKind CaptureKind, IntrusiveRefCntPtr<llvm::vfs::FileSystem> BaseFS) { std::vector<std::unique_ptr<ASTUnit>> ASTs; + ASTBuilderAction Action(ASTs); + Action.CaptureKinds = CaptureDiagsKind::All; + auto OverlayFileSystem = llvm::makeIntrusiveRefCnt<llvm::vfs::OverlayFileSystem>( std::move(BaseFS)); diff --git a/clang/unittests/Sema/HeuristicResolverTest.cpp b/clang/unittests/Sema/HeuristicResolverTest.cpp index 21aca7a3489b8..5d4048d787390 100644 --- a/clang/unittests/Sema/HeuristicResolverTest.cpp +++ b/clang/unittests/Sema/HeuristicResolverTest.cpp @@ -8,7 +8,9 @@ #include "clang/Sema/HeuristicResolver.h" #include "clang/ASTMatchers/ASTMatchFinder.h" #include "clang/ASTMatchers/ASTMatchers.h" +#include "clang/Basic/Diagnostic.h" #include "clang/Tooling/Tooling.h" +#include "llvm/ADT/SmallSet.h" #include "gmock/gmock-matchers.h" #include "gtest/gtest.h" @@ -31,6 +33,24 @@ template <typename InputNode> using ResolveFnT = std::function<std::vector<const NamedDecl *>( const HeuristicResolver *, InputNode)>; +std::string format_error(const clang::StoredDiagnostic *D) { + std::ostringstream Msg{}; + if (D->getLevel() == DiagnosticsEngine::Level::Ignored) + Msg << "Ignored: "; + if (D->getLevel() == DiagnosticsEngine::Level::Note) + Msg << "Note: "; + if (D->getLevel() == DiagnosticsEngine::Level::Remark) + Msg << "Remark: "; + if (D->getLevel() == DiagnosticsEngine::Level::Warning) + Msg << "Warning: "; + if (D->getLevel() == DiagnosticsEngine::Level::Error) + Msg << "Error: "; + if (D->getLevel() == DiagnosticsEngine::Level::Fatal) + Msg << "Fatal: "; + Msg << D->getID() << ": " << D->getMessage().str(); + return Msg.str(); +} + // Test heuristic resolution on `Code` using the resolution procedure // `ResolveFn`, which takes a `HeuristicResolver` and an input AST node of type // `InputNode` and returns a `std::vector<const NamedDecl *>`. @@ -41,7 +61,16 @@ template <typename InputNode, typename ParamT, typename InputMatcher, typename... OutputMatchers> void expectResolution(llvm::StringRef Code, ResolveFnT<ParamT> ResolveFn, const InputMatcher &IM, const OutputMatchers &...OMS) { + llvm::SmallSet<unsigned int, 16> IgnoredDiagnostics{}; auto TU = tooling::buildASTFromCodeWithArgs(Code, {"-std=c++23"}); + + for (auto D = TU->stored_diag_begin(), DEnd = TU->stored_diag_end(); + D != DEnd; ++D) { + EXPECT_TRUE(D->getLevel() < DiagnosticsEngine::Warning || + IgnoredDiagnostics.contains(D->getID())) + << format_error(D); + } + auto &Ctx = TU->getASTContext(); auto InputMatches = match(IM, Ctx); ASSERT_EQ(1u, InputMatches.size()); >From d4ceb236d4fea3d30799051f5181add32e9cce37 Mon Sep 17 00:00:00 2001 From: Mythreya Kuricheti <[email protected]> Date: Fri, 29 Aug 2025 17:57:13 -0700 Subject: [PATCH 3/4] code review --- clang/include/clang/Frontend/ASTUnit.h | 11 +++++++++ clang/include/clang/Tooling/Tooling.h | 4 ++-- clang/lib/Tooling/Tooling.cpp | 16 +++++++------ .../unittests/Sema/HeuristicResolverTest.cpp | 24 ++++++++----------- clang/unittests/Tooling/ToolingTest.cpp | 2 +- 5 files changed, 33 insertions(+), 24 deletions(-) diff --git a/clang/include/clang/Frontend/ASTUnit.h b/clang/include/clang/Frontend/ASTUnit.h index ad54016da11d2..27bba8e64859d 100644 --- a/clang/include/clang/Frontend/ASTUnit.h +++ b/clang/include/clang/Frontend/ASTUnit.h @@ -629,6 +629,17 @@ class ASTUnit { return StoredDiagnostics.end(); } + using diags_range = llvm::iterator_range<stored_diag_iterator>; + using const_diags_range = llvm::iterator_range<stored_diag_const_iterator>; + + diags_range storedDiagnostics() { + return {stored_diag_begin(), stored_diag_end()}; + } + + const_diags_range storedDiagnostics() const { + return {stored_diag_begin(), stored_diag_end()}; + } + unsigned stored_diag_size() const { return StoredDiagnostics.size(); } stored_diag_iterator stored_diag_afterDriver_begin() { diff --git a/clang/include/clang/Tooling/Tooling.h b/clang/include/clang/Tooling/Tooling.h index fd7a1de589a6f..9909394495496 100644 --- a/clang/include/clang/Tooling/Tooling.h +++ b/clang/include/clang/Tooling/Tooling.h @@ -239,9 +239,9 @@ std::unique_ptr<ASTUnit> buildASTFromCodeWithArgs( ArgumentsAdjuster Adjuster = getClangStripDependencyFileAdjuster(), const FileContentMappings &VirtualMappedFiles = FileContentMappings(), DiagnosticConsumer *DiagConsumer = nullptr, - CaptureDiagsKind CaptureKind = CaptureDiagsKind::None, IntrusiveRefCntPtr<llvm::vfs::FileSystem> BaseFS = - llvm::vfs::getRealFileSystem()); + llvm::vfs::getRealFileSystem(), + CaptureDiagsKind CaptureKind = CaptureDiagsKind::None); /// Utility to run a FrontendAction in a single clang invocation. class ToolInvocation { diff --git a/clang/lib/Tooling/Tooling.cpp b/clang/lib/Tooling/Tooling.cpp index 015ec8633b62d..a05084dae267d 100644 --- a/clang/lib/Tooling/Tooling.cpp +++ b/clang/lib/Tooling/Tooling.cpp @@ -644,9 +644,13 @@ namespace { class ASTBuilderAction : public ToolAction { std::vector<std::unique_ptr<ASTUnit>> &ASTs; + CaptureDiagsKind CaptureKinds; public: - ASTBuilderAction(std::vector<std::unique_ptr<ASTUnit>> &ASTs) : ASTs(ASTs) {} + ASTBuilderAction( + std::vector<std::unique_ptr<ASTUnit>> &ASTs, + CaptureDiagsKind CaptureDiagnosticKinds = CaptureDiagsKind::None) + : ASTs(ASTs), CaptureKinds(CaptureDiagnosticKinds) {} bool runInvocation(std::shared_ptr<CompilerInvocation> Invocation, FileManager *Files, @@ -665,8 +669,6 @@ class ASTBuilderAction : public ToolAction { ASTs.push_back(std::move(AST)); return true; } - - CaptureDiagsKind CaptureKinds{CaptureDiagsKind::None}; }; } // namespace @@ -694,12 +696,12 @@ std::unique_ptr<ASTUnit> buildASTFromCodeWithArgs( StringRef Code, const std::vector<std::string> &Args, StringRef FileName, StringRef ToolName, std::shared_ptr<PCHContainerOperations> PCHContainerOps, ArgumentsAdjuster Adjuster, const FileContentMappings &VirtualMappedFiles, - DiagnosticConsumer *DiagConsumer, CaptureDiagsKind CaptureKind, - IntrusiveRefCntPtr<llvm::vfs::FileSystem> BaseFS) { + DiagnosticConsumer *DiagConsumer, + IntrusiveRefCntPtr<llvm::vfs::FileSystem> BaseFS, + CaptureDiagsKind CaptureKind) { std::vector<std::unique_ptr<ASTUnit>> ASTs; - ASTBuilderAction Action(ASTs); - Action.CaptureKinds = CaptureDiagsKind::All; + ASTBuilderAction Action(ASTs, CaptureKind); auto OverlayFileSystem = llvm::makeIntrusiveRefCnt<llvm::vfs::OverlayFileSystem>( diff --git a/clang/unittests/Sema/HeuristicResolverTest.cpp b/clang/unittests/Sema/HeuristicResolverTest.cpp index 5d4048d787390..3ac2a168063e3 100644 --- a/clang/unittests/Sema/HeuristicResolverTest.cpp +++ b/clang/unittests/Sema/HeuristicResolverTest.cpp @@ -33,21 +33,21 @@ template <typename InputNode> using ResolveFnT = std::function<std::vector<const NamedDecl *>( const HeuristicResolver *, InputNode)>; -std::string format_error(const clang::StoredDiagnostic *D) { +std::string format_error(const clang::StoredDiagnostic &D) { std::ostringstream Msg{}; - if (D->getLevel() == DiagnosticsEngine::Level::Ignored) + if (D.getLevel() == DiagnosticsEngine::Level::Ignored) Msg << "Ignored: "; - if (D->getLevel() == DiagnosticsEngine::Level::Note) + if (D.getLevel() == DiagnosticsEngine::Level::Note) Msg << "Note: "; - if (D->getLevel() == DiagnosticsEngine::Level::Remark) + if (D.getLevel() == DiagnosticsEngine::Level::Remark) Msg << "Remark: "; - if (D->getLevel() == DiagnosticsEngine::Level::Warning) + if (D.getLevel() == DiagnosticsEngine::Level::Warning) Msg << "Warning: "; - if (D->getLevel() == DiagnosticsEngine::Level::Error) + if (D.getLevel() == DiagnosticsEngine::Level::Error) Msg << "Error: "; - if (D->getLevel() == DiagnosticsEngine::Level::Fatal) + if (D.getLevel() == DiagnosticsEngine::Level::Fatal) Msg << "Fatal: "; - Msg << D->getID() << ": " << D->getMessage().str(); + Msg << D.getID() << ": " << D.getMessage().str(); return Msg.str(); } @@ -61,14 +61,10 @@ template <typename InputNode, typename ParamT, typename InputMatcher, typename... OutputMatchers> void expectResolution(llvm::StringRef Code, ResolveFnT<ParamT> ResolveFn, const InputMatcher &IM, const OutputMatchers &...OMS) { - llvm::SmallSet<unsigned int, 16> IgnoredDiagnostics{}; auto TU = tooling::buildASTFromCodeWithArgs(Code, {"-std=c++23"}); - for (auto D = TU->stored_diag_begin(), DEnd = TU->stored_diag_end(); - D != DEnd; ++D) { - EXPECT_TRUE(D->getLevel() < DiagnosticsEngine::Warning || - IgnoredDiagnostics.contains(D->getID())) - << format_error(D); + for (const auto &D : TU->storedDiagnostics()) { + EXPECT_TRUE(D.getLevel() < DiagnosticsEngine::Error) << format_error(D); } auto &Ctx = TU->getASTContext(); diff --git a/clang/unittests/Tooling/ToolingTest.cpp b/clang/unittests/Tooling/ToolingTest.cpp index 67572cbf2cf50..25e1d67eb2294 100644 --- a/clang/unittests/Tooling/ToolingTest.cpp +++ b/clang/unittests/Tooling/ToolingTest.cpp @@ -161,7 +161,7 @@ TEST(buildASTFromCode, FileSystem) { R"(#include "included_file.h")", {}, "input.cc", "clang-tool", std::make_shared<PCHContainerOperations>(), getClangStripDependencyFileAdjuster(), FileContentMappings(), nullptr, - CaptureDiagsKind::None, InMemoryFileSystem); + InMemoryFileSystem); ASSERT_TRUE(AST.get()); EXPECT_TRUE(FindClassDeclX(AST.get())); } >From 1a9792195ad847761a96ca5144680f9887fe289a Mon Sep 17 00:00:00 2001 From: Mythreya Kuricheti <[email protected]> Date: Sat, 30 Aug 2025 16:25:42 -0700 Subject: [PATCH 4/4] code review --- clang/lib/Tooling/Tooling.cpp | 8 ++--- .../unittests/Sema/HeuristicResolverTest.cpp | 30 ++++++------------- 2 files changed, 13 insertions(+), 25 deletions(-) diff --git a/clang/lib/Tooling/Tooling.cpp b/clang/lib/Tooling/Tooling.cpp index a05084dae267d..1120e33b1f0c3 100644 --- a/clang/lib/Tooling/Tooling.cpp +++ b/clang/lib/Tooling/Tooling.cpp @@ -644,13 +644,13 @@ namespace { class ASTBuilderAction : public ToolAction { std::vector<std::unique_ptr<ASTUnit>> &ASTs; - CaptureDiagsKind CaptureKinds; + CaptureDiagsKind CaptureKind; public: ASTBuilderAction( std::vector<std::unique_ptr<ASTUnit>> &ASTs, - CaptureDiagsKind CaptureDiagnosticKinds = CaptureDiagsKind::None) - : ASTs(ASTs), CaptureKinds(CaptureDiagnosticKinds) {} + CaptureDiagsKind CaptureDiagnosticsKind = CaptureDiagsKind::None) + : ASTs(ASTs), CaptureKind(CaptureDiagnosticsKind) {} bool runInvocation(std::shared_ptr<CompilerInvocation> Invocation, FileManager *Files, @@ -662,7 +662,7 @@ class ASTBuilderAction : public ToolAction { Invocation->getDiagnosticOpts(), DiagConsumer, /*ShouldOwnClient=*/false), - Files, false, CaptureKinds); + Files, false, CaptureKind); if (!AST) return false; diff --git a/clang/unittests/Sema/HeuristicResolverTest.cpp b/clang/unittests/Sema/HeuristicResolverTest.cpp index 3ac2a168063e3..883a4e20e40a7 100644 --- a/clang/unittests/Sema/HeuristicResolverTest.cpp +++ b/clang/unittests/Sema/HeuristicResolverTest.cpp @@ -10,7 +10,6 @@ #include "clang/ASTMatchers/ASTMatchers.h" #include "clang/Basic/Diagnostic.h" #include "clang/Tooling/Tooling.h" -#include "llvm/ADT/SmallSet.h" #include "gmock/gmock-matchers.h" #include "gtest/gtest.h" @@ -33,24 +32,6 @@ template <typename InputNode> using ResolveFnT = std::function<std::vector<const NamedDecl *>( const HeuristicResolver *, InputNode)>; -std::string format_error(const clang::StoredDiagnostic &D) { - std::ostringstream Msg{}; - if (D.getLevel() == DiagnosticsEngine::Level::Ignored) - Msg << "Ignored: "; - if (D.getLevel() == DiagnosticsEngine::Level::Note) - Msg << "Note: "; - if (D.getLevel() == DiagnosticsEngine::Level::Remark) - Msg << "Remark: "; - if (D.getLevel() == DiagnosticsEngine::Level::Warning) - Msg << "Warning: "; - if (D.getLevel() == DiagnosticsEngine::Level::Error) - Msg << "Error: "; - if (D.getLevel() == DiagnosticsEngine::Level::Fatal) - Msg << "Fatal: "; - Msg << D.getID() << ": " << D.getMessage().str(); - return Msg.str(); -} - // Test heuristic resolution on `Code` using the resolution procedure // `ResolveFn`, which takes a `HeuristicResolver` and an input AST node of type // `InputNode` and returns a `std::vector<const NamedDecl *>`. @@ -61,10 +42,17 @@ template <typename InputNode, typename ParamT, typename InputMatcher, typename... OutputMatchers> void expectResolution(llvm::StringRef Code, ResolveFnT<ParamT> ResolveFn, const InputMatcher &IM, const OutputMatchers &...OMS) { - auto TU = tooling::buildASTFromCodeWithArgs(Code, {"-std=c++23"}); + auto TU = tooling::buildASTFromCodeWithArgs( + Code, {"-std=c++23"}, "input.cc", "clang-tool", + std::make_shared<PCHContainerOperations>(), + tooling::getClangStripDependencyFileAdjuster(), + tooling::FileContentMappings(), nullptr, llvm::vfs::getRealFileSystem(), + CaptureDiagsKind::All); for (const auto &D : TU->storedDiagnostics()) { - EXPECT_TRUE(D.getLevel() < DiagnosticsEngine::Error) << format_error(D); + EXPECT_TRUE(D.getLevel() < DiagnosticsEngine::Error) + << "Unexpected error diagnostic while building AST for test code: " + << D.getMessage(); } auto &Ctx = TU->getASTContext(); _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
