code review

Author: MythreyaK

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() {

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 {

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>(

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();

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()));
 }