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

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

jdenny accepted this revision.
jdenny added a comment.
This revision is now accepted and ready to land.

Thanks for addressing all my concerns.

LGTM except for a test issue I just commented on.



================
Comment at: clang/lib/Frontend/VerifyDiagnosticConsumer.cpp:468
+        Status = VerifyDiagnosticConsumer::HasExpectedMaybeNoDiagnostics;
+      continue;
+    } else if (DToken.endswith(DType="-no-diagnostics")) {
----------------
Endill wrote:
> jdenny wrote:
> > Endill wrote:
> > > jdenny wrote:
> > > > 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.
> > > This should be fixed now. Thank you for spotting this!
> > Thanks for the fix.  Please add a test so this bug doesn't pop up again.
> Done as A2 test
Does A2 trigger the original bug?  I think there must be multiple prefixes for this fix to matter.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151320