[PATCH] D151320: [clang] Add `// expected-maybe-no-diagnostics` comment to VerifyDiagnosticConsumer

[Joel E. Denny via Phabricator via cfe-commits]

jdenny added a comment.

Thanks for working on this.  I've been wanting something like it too.  In downstream work, I've used a hack that seems to accomplish the same thing but probably shouldn't: `expected-error 0 {{}}`.

Anyway, please add documentation for the new directive here: https://clang.llvm.org/doxygen/classclang_1_1VerifyDiagnosticConsumer.html#details



================
Comment at: clang/lib/Frontend/VerifyDiagnosticConsumer.cpp:468
+        Status = VerifyDiagnosticConsumer::HasExpectedMaybeNoDiagnostics;
+      continue;
+    } else if (DToken.endswith(DType="-no-diagnostics")) {
----------------
This `continue` skips the prefix checking below, which is important when there are multiple prefixes active (e.g., `-verify=foo,bar`).  That is, any old `BOGUS-maybe-no-diagnostics` will be effective then.


================
Comment at: clang/test/Frontend/verify-maybe-no-diagnostics.c:38
+#endif
+
+#ifdef TEST_D1
----------------
Please test this case:

```
#ifdef TEST_C3
// expected-maybe-no-diagnostics
#error test_c3
#endif
```

That is, does Clang actually fail in this case as expected because there's an error and no corresponding `expected-error` directive?  Or does it ignore the error because of the `expected-maybe-no-diagnostics` directive?


================
Comment at: clang/test/Frontend/verify-maybe-no-diagnostics.c:51-52
+#ifdef TEST_D2
+// expected-maybe-no-diagnostics
+// expected-no-diagnostics
+#error test_d2
----------------
So `expected-no-diagnostics` overrides `expected-maybe-no-diagnostics`.  In your use case, omitting one or the other is not always possible?


================
Comment at: clang/test/Frontend/verify-maybe-no-diagnostics.c:104
+//      D6-CHECK: error: 'error' diagnostics seen but not expected:
+// D6-CHECK-NEXT:   {{.*}} 'expected-no-diagnostics' directive cannot follow other expected directives
+// D6-CHECK-NEXT: 1 error generated.
----------------
This diagnostic is confusing.  Should we add "except 'expected-maybe-no-diagnostics'"?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151320