llvm-project icon indicating copy to clipboard operation
llvm-project copied to clipboard

"No expected directives found" error message is confusing if `-verify=foo` was passed

Open tbaederr opened this issue 3 years ago • 3 comments

If one passes -verify=foo, the error message when no foo-* directives are found is:

error: no expected directives found: consider use of 'expected-no-diagnostics'

however, what's meant here is not expected-no-diagnostics but of course foo-no-diagnostics. This is especially confusing if multiple compiler invocations with different -verify= values are present.

The error message should use the proper prefix.

tbaederr avatar Oct 11 '22 13:10 tbaederr

@llvm/issue-subscribers-good-first-issue

llvmbot avatar Oct 11 '22 13:10 llvmbot

@llvm/issue-subscribers-clang-driver

llvmbot avatar Oct 11 '22 13:10 llvmbot

Hi! Let me resolve this issue!

diohabara avatar Oct 14 '22 19:10 diohabara

I'm still working on it! I successfully reproduced the issue, and I'll fix it.

diohabara avatar Oct 18 '22 15:10 diohabara

@tbaederr According to this commit( 0fea04509a6416f27b8237cf7df5705678686cc3 ), it seems that the behavior is expected. Should we fix it to err with foo-no-diagnostics?

diohabara avatar Oct 25 '22 15:10 diohabara

It's fine that a "no-diagnostics" comment is required, but if I pass -verify=foo, I must use "foo-no-diagnostics" and as such, it would be nice if error messages about the usage of such "expected" messages would use the proper prefix and tell me that I should be adding a "foo-no-diagnostics" comment, not an "expected-no-diagnostics" one.

tbaederr avatar Oct 25 '22 16:10 tbaederr

It's fine that a "no-diagnostics" comment is required, but if I pass -verify=foo, I must use "foo-no-diagnostics" and as such, it would be nice if error messages about the usage of such "expected" messages would use the proper prefix and tell me that I should be adding a "foo-no-diagnostics" comment, not an "expected-no-diagnostics" one.

Thanks, I see! Since the commit I mentioned said that expected-no-diagnostics was expected behavior, I was wondering if I were in the right direction.

diohabara avatar Oct 25 '22 19:10 diohabara

I don't think "expected-no-diagnostics" is to be taken literally; it's just the default when no -verify= is passed.

tbaederr avatar Oct 26 '22 05:10 tbaederr

Hello, has this issue been fixed and @diohabara are you still working on this, I will like to work on it.

Unique-Usman avatar Apr 10 '23 18:04 Unique-Usman

@Unique-Usman

Yes, you can do it.

diohabara avatar Apr 10 '23 20:04 diohabara

Hello, I think that this issue has not been resolved yet. @tbaederr , I would like to work on this issue, can you please assign it to me.

Sh0g0-1758 avatar Jan 09 '24 06:01 Sh0g0-1758

I reproduced the error by reading the following documentation : docs. Will rectify it now.

Sh0g0-1758 avatar Jan 10 '24 04:01 Sh0g0-1758

I have made the necessary changes in the DiagnosticFrontendKinds.td file as :

def err_verify_no_directives : Error<
    "no expected directives found: consider use of '%0'-no-diagnostics">;

and in VerifyDiagnosticConsumer.cpp as :

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).AddString(TO DO : ADD DIAGNOSTIC NAME).setForceEmit();
  ++NumErrors;
  Status = HasNoDirectivesReported;
}

can you please confirm whether or not I can get this by SM.getDiagnostics() ?

@tbaederr

Sh0g0-1758 avatar Jan 10 '24 13:01 Sh0g0-1758

You might wanna look at b4e0589b2cd98a93aad449486bb2a52ab8790781, which did something similar.

tbaederr avatar Jan 11 '24 08:01 tbaederr

@tbaederr, created a PR for the same. Please take a look at it and suggest changes when you get free. Also really sorry for the delay, was busy the past couple of days.

Sh0g0-1758 avatar Jan 16 '24 20:01 Sh0g0-1758