[Clang] Handle Error message to output proper Prefix

[Unique_Usman]

Summary

Made err_verify_no_directives output error message to be generic.
The error output is alway default message for all value of -verify which
"no expected directives found: consider use of 'expected-no-diagnostics'"

Now, if you have -verify=foo, the error message will
"no expected directives found: consider use of 'foo-no-diagnostics'"

Resolves issue #58290

[tbaeder]

I am not 100% sure about the semantics of passing multiple prefixes, i.e. if the error is emitted for all prefixes individually or if it's only emitted if no expected line for any of the prefixes is found. In the latter case we should probably add all the prefixes to the error message.

clang/lib/Frontend/VerifyDiagnosticConsumer.cpp

1097

Can move this line inside the if statement block.

[Unique_Usman]

In D148601#4279334, @tbaeder wrote:

I am not 100% sure about the semantics of passing multiple prefixes, i.e. if the error is emitted for all prefixes individually or if it's only emitted if no expected line for any of the prefixes is found. In the latter case we should probably add all the prefixes to the error message.

I tested different scenerios e.g added more than one RUN lines with different value of -verify, what I concluded on is that if we have multiple RUN lines with each of them having no directive, the prefixes generated is always of the first occurence with no expected directive so, the error is always generated for the first occurence with no expected directive.

[tbaeder]

In D148601#4279604, @Unique_Usman wrote:

In D148601#4279334, @tbaeder wrote:

I am not 100% sure about the semantics of passing multiple prefixes, i.e. if the error is emitted for all prefixes individually or if it's only emitted if no expected line for any of the prefixes is found. In the latter case we should probably add all the prefixes to the error message.

I tested different scenerios e.g added more than one RUN lines with different value of -verify, what I concluded on is that if we have multiple RUN lines with each of them having no directive, the prefixes generated is always of the first occurence with no expected directive so, the error is always generated for the first occurence with no expected directive.

Yeah but I think you can do -verify=foo,bar(?) in which case the list f prefixes would actually have more than one item.

[Unique_Usman]

In D148601#4295719, @tbaeder wrote:

In D148601#4279604, @Unique_Usman wrote:

In D148601#4279334, @tbaeder wrote:

I am not 100% sure about the semantics of passing multiple prefixes, i.e. if the error is emitted for all prefixes individually or if it's only emitted if no expected line for any of the prefixes is found. In the latter case we should probably add all the prefixes to the error message.

I tested different scenerios e.g added more than one RUN lines with different value of -verify, what I concluded on is that if we have multiple RUN lines with each of them having no directive, the prefixes generated is always of the first occurence with no expected directive so, the error is always generated for the first occurence with no expected directive.

Yeah but I think you can do -verify=foo,bar(?) in which case the list f prefixes would actually have more than one item.

I used -verify=foo,bar but, the prefixes still have just only one item, in the case bar. Does this the implementation of getting the prefixes is faulty?

[tbaeder]

When I run with -verify=foo,barI only get the output for bar-no-diagnostics. Adding bar-no-diagnostics, I get no output at all and the test succeeds.

I didn't know how passing multiple prefixes works in practice, seems like it matches any of the given ones. So the new output is still better than the old one.

LGTM but I'll wait a while for someone else to chime in.

In the meantime, please state a name and email address to use for the git commit, in case you don't have commit access.

[Unique_Usman]

Thanks @tbaeder, I really appreciate your time.

Name is Unique_Usman and my email is [e-mail]

As you mentioned, if anyone later chime in, I will be glad to update the issue back.
Thank you.

[aaron.ballman]

Thank you for working on this! These changes should come with test coverage (we have a handful of verifier tests in clang/test/Frontend -- you can add a new test file there), but I don't think a release note is required because this is a fix for internal functionality. The test should cover the simple case of -verify=something as well as a more complex test with -verify=something, something_else.

clang/include/clang/Basic/DiagnosticFrontendKinds.td

175–176

Should we be handling this situation at the same time, as it's effectively the same concern? e.g., https://godbolt.org/z/4Mn1rW9oa

[Unique_Usman]

In D148601#4301977, @aaron.ballman wrote:

Thank you for working on this! These changes should come with test coverage (we have a handful of verifier tests in clang/test/Frontend -- you can add a new test file there), but I don't think a release note is required because this is a fix for internal functionality. The test should cover the simple case of -verify=something as well as a more complex test with -verify=something, something_else.

Thank you, will work on this.

[Unique_Usman]

In D148601#4301977, @aaron.ballman wrote:

Thank you for working on this! These changes should come with test coverage (we have a handful of verifier tests in clang/test/Frontend -- you can add a new test file there), but I don't think a release note is required because this is a fix for internal functionality. The test should cover the simple case of -verify=something as well as a more complex test with -verify=something, something_else.

Hello, I have been going through the other verfiier test, so basicallhy the new test file will test if the error output generated by -verify=something and -verify=something, something_else is the expected output right?

clang/include/clang/Basic/DiagnosticFrontendKinds.td

175–176

Hello, do you meant if we should handle when value is passed to -verify and when it is not together?

I will say yes, the solution handle the situation whenever the value is passed to verify and at the same time was able to handle whenever value is not passed(it output the default which is 'expected-no-diagnostics',).

Do you think otherwise?

clang/include/clang/Basic/DiagnosticFrontendKinds.td

 def err_verify_invalid_content : Error<
     "invalid expected %0: %1">;
 def err_verify_missing_regex : Error<
     "cannot find start of regex ('{{') in %0">;
 def err_verify_inconsistent_diags : Error<
     "'%0' diagnostics %select{expected|seen}1 but not %select{seen|expected}1: "
     "%2">;
 def err_verify_invalid_no_diags : Error<
     "%select{expected|'expected-no-diagnostics'}0 directive cannot follow "
     "%select{'expected-no-diagnostics' directive|other expected directives}0">;
 def err_verify_no_directives : Error<
-    "no expected directives found: consider use of 'expected-no-diagnostics'">;
+    "no expected directives found: consider use of '%0-no-diagnostics'">;
 def err_verify_nonconst_addrspace : Error<
   "qualifier 'const' is needed for variables in address space '%0'">;
 
 def note_fixit_applied : Note<"FIX-IT applied suggested code changes">;
 def note_fixit_in_macro : Note<
     "FIX-IT unable to apply suggested code changes in a macro">;
 def note_fixit_failed : Note<
     "FIX-IT unable to apply suggested code changes">;

clang/lib/Frontend/VerifyDiagnosticConsumer.cpp

if (!UnparsedFiles.empty()) {
     // UnparsedFiles has been processed now, so clear it.
     UnparsedFiles.clear();
   }
 #endif // !NDEBUG
 
   if (SrcManager) {
     // Produce an error if no expected-* directives could be found in the
     // source file(s) processed.
     if (Status == HasNoDirectives) {
-      Diags.Report(diag::err_verify_no_directives).setForceEmit();
+      const auto &Prefixes = Diags.getDiagnosticOptions().VerifyPrefixes;
+      Diags.Report(diag::err_verify_no_directives).setForceEmit() << *Prefixes.begin();
       ++NumErrors;
       Status = HasNoDirectivesReported;
     }
 
     // Check that the expected diagnostics occurred.
     NumErrors += CheckResults(Diags, *SrcManager, *Buffer, ED);
   } else {
     const DiagnosticLevelMask DiagMask =