[clang][tooling] NFC: Refactor command-line diagnostic tests

[jansvoboda11]

Summary

This patch uses a different command-line arguments to test clang::tooling::ToolInvocation that are not specific to Darwin.

[dexonsmith]

clang/unittests/Tooling/ToolingTest.cpp

238–244

I'm worried this is going to behave differently when the unit test isn't run on Darwin, since I think -isysroot is only valid for Darwin targets and there's no -arch or -target option. Can you confirm whether this should reproduce properly when run on other hosts as well?

[dexonsmith]

Stepping back, maybe it'd be better to use a more generic option that is unlikely to have special behaviour in any target (or downstream).

Best one I could find is triggering err_drv_unknown_language with -x this-is-an-unknown-language. WDYT?

clang/unittests/Tooling/ToolingTest.cpp

277–280

Can this just be empty and use -E? But if you switch to another error maybe you don't need to modify this anyway?

[jansvoboda11]

Use flags not specific to Darwin

[jansvoboda11]

I think we need a warning option to be able to suppress it (or conditionally promote to an error).

clang/unittests/Tooling/ToolingTest.cpp

238–244

Good point, I switched to a platform-agnostic flags.

277–280

Yes, that would be a nice simplification.

[jansvoboda11]

Use empty files and -E

[dexonsmith]

LGTM.

clang/unittests/Tooling/ToolingTest.cpp

 #include "clang/AST/DeclCXX.h"
 #include "clang/AST/DeclGroup.h"
 #include "clang/Driver/Compilation.h"
 #include "clang/Driver/Driver.h"
 #include "clang/Frontend/ASTUnit.h"
 #include "clang/Frontend/CompilerInstance.h"
 #include "clang/Frontend/FrontendAction.h"
 #include "clang/Frontend/FrontendActions.h"
+#include "clang/Frontend/TextDiagnosticBuffer.h"
 #include "clang/Tooling/ArgumentsAdjusters.h"
 #include "clang/Tooling/CompilationDatabase.h"
 #include "clang/Tooling/Tooling.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/Host.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/TargetRegistry.h"
InMemoryFileSystem->addFile("def/abc", 0,
                               llvm::MemoryBuffer::getMemBuffer("\n"));
   // Add a module.map file in the include directory of our header, so we trigger
   // the module.map header search logic.
   InMemoryFileSystem->addFile("def/module.map", 0,
                               llvm::MemoryBuffer::getMemBuffer("\n"));
   EXPECT_TRUE(Invocation.run());
 }
 
-struct ErrorCountingDiagnosticConsumer : public DiagnosticConsumer {
-  ErrorCountingDiagnosticConsumer() : NumErrorsSeen(0) {}
-  void HandleDiagnostic(DiagnosticsEngine::Level DiagLevel,
-                        const Diagnostic &Info) override {
-    if (DiagLevel == DiagnosticsEngine::Level::Error)
-      ++NumErrorsSeen;
-  }
-  unsigned NumErrorsSeen;
-};
-
 TEST(ToolInvocation, DiagnosticsEngineProperlyInitializedForCC1Construction) {
   llvm::IntrusiveRefCntPtr<llvm::vfs::OverlayFileSystem> OverlayFileSystem(
       new llvm::vfs::OverlayFileSystem(llvm::vfs::getRealFileSystem()));
   llvm::IntrusiveRefCntPtr<llvm::vfs::InMemoryFileSystem> InMemoryFileSystem(
       new llvm::vfs::InMemoryFileSystem);
   OverlayFileSystem->pushOverlay(InMemoryFileSystem);
   llvm::IntrusiveRefCntPtr<FileManager> Files(
       new FileManager(FileSystemOptions(), OverlayFileSystem));
 
   std::vector<std::string> Args;
   Args.push_back("tool-executable");
-  Args.push_back("-target");
-  // Invalid argument that by default results in an error diagnostic:
-  Args.push_back("i386-apple-ios14.0-simulator");
-  // Argument that downgrades the error into a warning:
-  Args.push_back("-Wno-error=invalid-ios-deployment-target");
-  Args.push_back("-fsyntax-only");
+  // Unknown warning option will result in a warning.
+  Args.push_back("-fexpensive-optimizations");
+  // Argument that will suppress the warning above.
+  Args.push_back("-Wno-ignored-optimization-argument");
+  Args.push_back("-E");
   Args.push_back("test.cpp");
 
   clang::tooling::ToolInvocation Invocation(
       Args, std::make_unique<SyntaxOnlyAction>(), Files.get());
-  InMemoryFileSystem->addFile(
-      "test.cpp", 0, llvm::MemoryBuffer::getMemBuffer("int a() {}\n"));
-  ErrorCountingDiagnosticConsumer Consumer;
+  InMemoryFileSystem->addFile("test.cpp", 0,
+                              llvm::MemoryBuffer::getMemBuffer(""));
+  TextDiagnosticBuffer Consumer;
   Invocation.setDiagnosticConsumer(&Consumer);
   EXPECT_TRUE(Invocation.run());
-  // Check that `-Wno-error=invalid-ios-deployment-target` downgraded the error
-  // into a warning.
-  EXPECT_EQ(Consumer.NumErrorsSeen, 0u);
+  // Check that the warning was ignored due to the '-Wno-xxx' argument.
+  EXPECT_EQ(std::distance(Consumer.warn_begin(), Consumer.warn_end()), 0u);
 }
 
 TEST(ToolInvocation, CustomDiagnosticOptionsOverwriteParsedOnes) {
   llvm::IntrusiveRefCntPtr<llvm::vfs::OverlayFileSystem> OverlayFileSystem(
       new llvm::vfs::OverlayFileSystem(llvm::vfs::getRealFileSystem()));
   llvm::IntrusiveRefCntPtr<llvm::vfs::InMemoryFileSystem> InMemoryFileSystem(
       new llvm::vfs::InMemoryFileSystem);
   OverlayFileSystem->pushOverlay(InMemoryFileSystem);
   llvm::IntrusiveRefCntPtr<FileManager> Files(
       new FileManager(FileSystemOptions(), OverlayFileSystem));
 
   std::vector<std::string> Args;
   Args.push_back("tool-executable");
-  Args.push_back("-target");
-  // Invalid argument that by default results in an error diagnostic:
-  Args.push_back("i386-apple-ios14.0-simulator");
-  // Argument that downgrades the error into a warning:
-  Args.push_back("-Wno-error=invalid-ios-deployment-target");
-  Args.push_back("-fsyntax-only");
+  // Unknown warning option will result in a warning.
+  Args.push_back("-fexpensive-optimizations");
+  // Argument that will suppress the warning above.
+  Args.push_back("-Wno-ignored-optimization-argument");
+  Args.push_back("-E");
   Args.push_back("test.cpp");
 
   clang::tooling::ToolInvocation Invocation(
       Args, std::make_unique<SyntaxOnlyAction>(), Files.get());
-  InMemoryFileSystem->addFile(
-      "test.cpp", 0, llvm::MemoryBuffer::getMemBuffer("int a() {}\n"));
-  ErrorCountingDiagnosticConsumer Consumer;
+  InMemoryFileSystem->addFile("test.cpp", 0,
+                              llvm::MemoryBuffer::getMemBuffer(""));
+  TextDiagnosticBuffer Consumer;
   Invocation.setDiagnosticConsumer(&Consumer);
 
+  // Inject custom `DiagnosticOptions` for command-line parsing.
   auto DiagOpts = llvm::makeIntrusiveRefCnt<DiagnosticOptions>();
   Invocation.setDiagnosticOptions(&*DiagOpts);
 
   EXPECT_TRUE(Invocation.run());
-  // Check that `-Wno-error=invalid-ios-deployment-target` didn't downgrade the
-  // error into a warning due to being overwritten by custom diagnostic options.
-  EXPECT_EQ(Consumer.NumErrorsSeen, 1u);
+  // Check that the warning was issued during command-line parsing due to the
+  // custom `DiagnosticOptions` without '-Wno-xxx'.
+  EXPECT_EQ(std::distance(Consumer.warn_begin(), Consumer.warn_end()), 1u);
 }
 
 struct DiagnosticConsumerExpectingSourceManager : public DiagnosticConsumer {
   bool SawSourceManager;
 
   DiagnosticConsumerExpectingSourceManager() : SawSourceManager(false) {}
 
   void HandleDiagnostic(clang::DiagnosticsEngine::Level,