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

[Jan Svoboda via Phabricator via cfe-commits]

jansvoboda11 added a comment.

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



================
Comment at: clang/unittests/Tooling/ToolingTest.cpp:238-245
   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");
+  // Invalid argument (non-existent sysroot path) that will result in a warning.
+  Args.push_back("-isysroot");
+  Args.push_back("/dev/null/sysroot");
+  // Argument that will suppress the warning above.
+  Args.push_back("-Wno-missing-sysroot");
   Args.push_back("-fsyntax-only");
----------------
dexonsmith wrote:
> 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?
Good point, I switched to a platform-agnostic flags.


================
Comment at: clang/unittests/Tooling/ToolingTest.cpp:280-281
   InMemoryFileSystem->addFile(
-      "test.cpp", 0, llvm::MemoryBuffer::getMemBuffer("int a() {}\n"));
-  ErrorCountingDiagnosticConsumer Consumer;
+      "test.cpp", 0, llvm::MemoryBuffer::getMemBuffer("void a() {}\n"));
+  TextDiagnosticBuffer Consumer;
   Invocation.setDiagnosticConsumer(&Consumer);
----------------
dexonsmith wrote:
> Can this just be empty and use `-E`? But if you switch to another error maybe you don't need to modify this anyway?
Yes, that would be a nice simplification.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110160