[clang-tools-extra] [llvm] [clang] [Clang] Fix : More Detailed "No expected directives found" (PR #78338)

[Shourya Goel via cfe-commits]

@@ -179,7 +179,7 @@ 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">;

Sh0g0-1758 wrote:

Yes I understand. I made a beginners mistake of not making a branch for the 
changes in this PR. Will surely keep that in mind from now on. Also I hope you 
can overlook my enthu, was not trying to nag any reviewers. 

https://github.com/llvm/llvm-project/pull/78338
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] [llvm] [clang] [Clang] Fix : More Detailed "No expected directives found" (PR #78338)

[Vlad Serebrennikov via cfe-commits]

Endilll wrote:

> gentle ping. Please check the mergeability of this PR.

Friendly reminder of one ping per week policy.

https://github.com/llvm/llvm-project/pull/78338
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] [llvm] [clang] [Clang] Fix : More Detailed "No expected directives found" (PR #78338)

[Aaron Ballman via cfe-commits]

@@ -179,7 +179,7 @@ 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">;

AaronBallman wrote:

I think @jrtc27 is saying that this diagnostic should also change to use `%0` 
similar to what you've done for `err_verify_no_directives` because it has the 
same issue with saying `'expected-no-diagnostics'`.

https://github.com/llvm/llvm-project/pull/78338
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] [llvm] [clang] [Clang] Fix : More Detailed "No expected directives found" (PR #78338)

[Jessica Clarke via cfe-commits]

@@ -179,7 +179,7 @@ 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">;

jrtc27 wrote:

This looks like it needs templating?

https://github.com/llvm/llvm-project/pull/78338
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] [llvm] [clang] [Clang] Fix : More Detailed "No expected directives found" (PR #78338)

[Jessica Clarke via cfe-commits]

@@ -179,7 +179,7 @@ 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'">;

jrtc27 wrote:

I guess elsewhere we refer to them as expected diagnostics, it's only in single 
quotes that we intend it to match the source?

https://github.com/llvm/llvm-project/pull/78338
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] [llvm] [clang] [Clang] Fix : More Detailed "No expected directives found" (PR #78338)

[Timm Baeder via cfe-commits]

@@ -1098,7 +1098,13 @@ void VerifyDiagnosticConsumer::CheckDiagnostics() {
 // 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();
+  std::string directives;
+  if (Diags.getDiagnosticOptions().VerifyPrefixes.empty()) {
+directives = "expected";

tbaederr wrote:

I guess that's fine then.

https://github.com/llvm/llvm-project/pull/78338
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] [llvm] [clang] [Clang] Fix : More Detailed "No expected directives found" (PR #78338)

[Shourya Goel via cfe-commits]

https://github.com/Sh0g0-1758 edited 
https://github.com/llvm/llvm-project/pull/78338
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] [llvm] [clang] [Clang] Fix : More Detailed "No expected directives found" (PR #78338)

[Shourya Goel via cfe-commits]

@@ -1098,7 +1098,16 @@ void VerifyDiagnosticConsumer::CheckDiagnostics() {
 // 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();
+  std::string directives;
+  for (auto  : Diags.getDiagnosticOptions().VerifyPrefixes) {
+directives = directives + Prefix + ",";
+  }

Sh0g0-1758 wrote:

I see your point. I will revert to emitting just one error then with the first 
of the diagnostics that were not found. 

https://github.com/llvm/llvm-project/pull/78338
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] [llvm] [clang] [Clang] Fix : More Detailed "No expected directives found" (PR #78338)

[Shourya Goel via cfe-commits]

https://github.com/Sh0g0-1758 deleted 
https://github.com/llvm/llvm-project/pull/78338
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] [llvm] [clang] [Clang] Fix : More Detailed "No expected directives found" (PR #78338)

[Shourya Goel via cfe-commits]

@@ -1098,7 +1098,16 @@ void VerifyDiagnosticConsumer::CheckDiagnostics() {
 // 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();
+  std::string directives;
+  for (auto  : Diags.getDiagnosticOptions().VerifyPrefixes) {
+directives = directives + Prefix + ",";
+  }

Sh0g0-1758 wrote:

I think the one I implemented is a better option as it batched the same types 
of errors into one and gave the programmer a concise view of the error. 

https://github.com/llvm/llvm-project/pull/78338
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] [llvm] [clang] [Clang] Fix : More Detailed "No expected directives found" (PR #78338)

[Shourya Goel via cfe-commits]

@@ -1098,7 +1098,16 @@ void VerifyDiagnosticConsumer::CheckDiagnostics() {
 // 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();
+  std::string directives;
+  for (auto  : Diags.getDiagnosticOptions().VerifyPrefixes) {
+directives = directives + Prefix + ",";
+  }

Sh0g0-1758 wrote:

yes suppose multiple diagnostics were not found like given in the third case 
here : 

> ![Screenshot from 2024-01-20 
> 00-23-12](https://private-user-images.githubusercontent.com/114918019/298168762-c8c7dcca-a1eb-4ee5-8fe7-53c7c2ac99ca.png?jwt=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJnaXRodWIuY29tIiwiYXVkIjoicmF3LmdpdGh1YnVzZXJjb250ZW50LmNvbSIsImtleSI6ImtleTUiLCJleHAiOjE3MDU5NDU2NTIsIm5iZiI6MTcwNTk0NTM1MiwicGF0aCI6Ii8xMTQ5MTgwMTkvMjk4MTY4NzYyLWM4YzdkY2NhLWExZWItNGVlNS04ZmU3LTUzYzdjMmFjOTljYS5wbmc_WC1BbXotQWxnb3JpdGhtPUFXUzQtSE1BQy1TSEEyNTYmWC1BbXotQ3JlZGVudGlhbD1BS0lBVkNPRFlMU0E1M1BRSzRaQSUyRjIwMjQwMTIyJTJGdXMtZWFzdC0xJTJGczMlMkZhd3M0X3JlcXVlc3QmWC1BbXotRGF0ZT0yMDI0MDEyMlQxNzQyMzJaJlgtQW16LUV4cGlyZXM9MzAwJlgtQW16LVNpZ25hdHVyZT03ZTRjNmIwYmU4MDhhMGNhZjc3NzE2NTgwYmUzNjRiZjEzNDE5YzhmNTJmYmNhZDM3ZDBmZTg5NTUwNGViNzc2JlgtQW16LVNpZ25lZEhlYWRlcnM9aG9zdCZhY3Rvcl9pZD0wJmtleV9pZD0wJnJlcG9faWQ9MCJ9.CEGvxZlT56vu6qZ73y9ZSxX4d2a2FhH5hYEW3Wp6sUM)

then in the error generated, I give all the diagnostics as ```consider use of 
'foo,alpha,bar,gamma-no-diagnostics'```. But if that is not the intended 
behavior or if multiple errors are expected to be emitted in response to 
multiple diagnostics not found, then that can also be done. Please specify 
which of the two is required. 

https://github.com/llvm/llvm-project/pull/78338
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] [llvm] [clang] [Clang] Fix : More Detailed "No expected directives found" (PR #78338)

[Timm Baeder via cfe-commits]

@@ -1098,7 +1098,16 @@ void VerifyDiagnosticConsumer::CheckDiagnostics() {
 // 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();
+  std::string directives;
+  for (auto  : Diags.getDiagnosticOptions().VerifyPrefixes) {
+directives = directives + Prefix + ",";
+  }

tbaederr wrote:

In `PrintUnexpected` we're simply using 
`Diags.getDiagnosticOptions().VerifyPrefixed.begin`.  How is this case 
different?

https://github.com/llvm/llvm-project/pull/78338
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] [llvm] [clang] [CLANG] Fix : More Detailed "No expected directives found" (PR #78338)

[Shourya Goel via cfe-commits]

@@ -11,7 +11,7 @@
 #error
 // expected-error@-1 {{}}
 
-//  CHECK: error: no expected directives found: consider use of 
'expected-no-diagnostics'
+//  CHECK: error: no expected directives found: consider use of 
'{{.*}}-no-diagnostics'

Sh0g0-1758 wrote:

the intended behavior is that when only `-verify` is passed and there are no 
diagnostics in the file then `expected-no-diagnostics` should be the error and 
when `-verify=foo` is passed and no such diagnostics is present in the file, 
then `foo-no-diagnostics` should be the error. 

https://github.com/llvm/llvm-project/pull/78338
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] [llvm] [clang] [CLANG] Fix : More Detailed "No expected directives found" (PR #78338)

[Timm Baeder via cfe-commits]

@@ -11,7 +11,7 @@
 #error
 // expected-error@-1 {{}}
 
-//  CHECK: error: no expected directives found: consider use of 
'expected-no-diagnostics'
+//  CHECK: error: no expected directives found: consider use of 
'{{.*}}-no-diagnostics'

tbaederr wrote:

And what is the output now? Both RUN lines only pass `-verify` so the prefix 
shouldn't change?

https://github.com/llvm/llvm-project/pull/78338
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] [llvm] [clang] [CLANG] Fix : More Detailed "No expected directives found" (PR #78338)

[Timm Baeder via cfe-commits]

@@ -11,7 +11,7 @@
 #error
 // expected-error@-1 {{}}
 
-//  CHECK: error: no expected directives found: consider use of 
'expected-no-diagnostics'
+//  CHECK: error: no expected directives found: consider use of 
'{{.*}}-no-diagnostics'

tbaederr wrote:

Do all these tests really fail if you leave "expected-no-diagnostics" in the 
expected output? Or can you just change that to a concrete value instead of the 
regex?

https://github.com/llvm/llvm-project/pull/78338
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] [llvm] [clang] [CLANG] Fix : More Detailed "No expected directives found" (PR #78338)

[Shourya Goel via cfe-commits]

Sh0g0-1758 wrote:

@tbaederr, please review the changes. 

https://github.com/llvm/llvm-project/pull/78338
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] [llvm] [clang] [CLANG] Fix : More Detailed "No expected directives found" (PR #78338)

[Shourya Goel via cfe-commits]

@@ -179,7 +179,7 @@ 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">;

Sh0g0-1758 wrote:

changed it to this norm and modified the test cases correspondingly. 

https://github.com/llvm/llvm-project/pull/78338
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] [llvm] [clang] [CLANG] Fix : More Detailed "No expected directives found" (PR #78338)

[Shourya Goel via cfe-commits]

@@ -5,5 +5,5 @@
 # `.ii` file and `-verify` triggers extra diagnostics generation. Clangd should
 # strip those.
 # RUN: clangd-indexer %t.cpp -- -Xclang -verify --save-temps -- 2>&1 | 
FileCheck %s
-# CHECK-NOT: error: no expected directives found: consider use of 
'expected-no-diagnostics'
+# CHECK-NOT: error: no expected directives found: consider use of 
{{.*}}-no-diagnostics

Sh0g0-1758 wrote:

ah yes, overlooked that. Will revert it. 

https://github.com/llvm/llvm-project/pull/78338
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] [llvm] [clang] [CLANG] Fix : More Detailed "No expected directives found" (PR #78338)

[Shourya Goel via cfe-commits]

https://github.com/Sh0g0-1758 updated 
https://github.com/llvm/llvm-project/pull/78338

>From b98f02d4c155b5be9bd4f5b2e4bf73720a81f39a Mon Sep 17 00:00:00 2001
From: Sh0g0-1758 
Date: Wed, 17 Jan 2024 01:24:17 +0530
Subject: [PATCH 1/6] Fix : more detailed no expected directive message

---
 clang-tools-extra/clangd/test/indexer.test   | 2 +-
 clang/include/clang/Basic/Diagnostic.h   | 4 +++-
 clang/include/clang/Basic/DiagnosticFrontendKinds.td | 2 +-
 clang/lib/AST/ASTContext.cpp | 2 +-
 clang/lib/Frontend/VerifyDiagnosticConsumer.cpp  | 4 +++-
 clang/test/ARCMT/verify.m| 2 +-
 clang/test/Frontend/verify.c | 2 +-
 clang/test/Frontend/verify2.c| 2 +-
 clang/test/Frontend/verify3.c| 4 ++--
 9 files changed, 14 insertions(+), 10 deletions(-)

diff --git a/clang-tools-extra/clangd/test/indexer.test 
b/clang-tools-extra/clangd/test/indexer.test
index 2f01f6c557a7de7..213d33f8e2d6a56 100644
--- a/clang-tools-extra/clangd/test/indexer.test
+++ b/clang-tools-extra/clangd/test/indexer.test
@@ -5,5 +5,5 @@
 # `.ii` file and `-verify` triggers extra diagnostics generation. Clangd should
 # strip those.
 # RUN: clangd-indexer %t.cpp -- -Xclang -verify --save-temps -- 2>&1 | 
FileCheck %s
-# CHECK-NOT: error: no expected directives found: consider use of 
'expected-no-diagnostics'
+# CHECK-NOT: error: no expected directives found: consider use of 
{{.*}}-no-diagnostics
 # RUN: not ls %t.ii
diff --git a/clang/include/clang/Basic/Diagnostic.h 
b/clang/include/clang/Basic/Diagnostic.h
index 0c7836c2ea569cc..a185c75e418b58b 100644
--- a/clang/include/clang/Basic/Diagnostic.h
+++ b/clang/include/clang/Basic/Diagnostic.h
@@ -9,7 +9,9 @@
 /// \file
 /// Defines the Diagnostic-related interfaces.
 //
-//===--===//
+//===--===//]
+
+// look into this file as well
 
 #ifndef LLVM_CLANG_BASIC_DIAGNOSTIC_H
 #define LLVM_CLANG_BASIC_DIAGNOSTIC_H
diff --git a/clang/include/clang/Basic/DiagnosticFrontendKinds.td 
b/clang/include/clang/Basic/DiagnosticFrontendKinds.td
index 568000106a84dc7..42d2767af6b7081 100644
--- a/clang/include/clang/Basic/DiagnosticFrontendKinds.td
+++ b/clang/include/clang/Basic/DiagnosticFrontendKinds.td
@@ -179,7 +179,7 @@ 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'">;
 
diff --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp
index b60dcfaabfd1a4b..0997ac9fa7df042 100644
--- a/clang/lib/AST/ASTContext.cpp
+++ b/clang/lib/AST/ASTContext.cpp
@@ -1404,7 +1404,7 @@ void ASTContext::InitBuiltinTypes(const TargetInfo 
,
 getTranslationUnitDecl()->addDecl(MSGuidTagDecl);
   }
 }
-
+// maybe change here also. 
 DiagnosticsEngine ::getDiagnostics() const {
   return SourceMgr.getDiagnostics();
 }
diff --git a/clang/lib/Frontend/VerifyDiagnosticConsumer.cpp 
b/clang/lib/Frontend/VerifyDiagnosticConsumer.cpp
index 8a3d2286cd168c3..9a26905a76d46bb 100644
--- a/clang/lib/Frontend/VerifyDiagnosticConsumer.cpp
+++ b/clang/lib/Frontend/VerifyDiagnosticConsumer.cpp
@@ -1098,7 +1098,9 @@ void VerifyDiagnosticConsumer::CheckDiagnostics() {
 // 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();
+  // change here
+  std::string directives = 
*Diags.getDiagnosticOptions().VerifyPrefixes.begin();
+  Diags.Report(diag::err_verify_no_directives).setForceEmit() << 
directives;
   ++NumErrors;
   Status = HasNoDirectivesReported;
 }
diff --git a/clang/test/ARCMT/verify.m b/clang/test/ARCMT/verify.m
index 7d245fe80575e5b..13fd599f530fbec 100644
--- a/clang/test/ARCMT/verify.m
+++ b/clang/test/ARCMT/verify.m
@@ -11,7 +11,7 @@
 #error
 // expected-error@-1 {{}}
 
-//  CHECK: error: no expected directives found: consider use of 
'expected-no-diagnostics'
+//  CHECK: error: no expected directives found: consider use of 
{{.*}}-no-diagnostics
 // CHECK-NEXT: error: 'expected-error' diagnostics seen but not expected:
 // CHECK-NEXT:   (frontend): error reading '{{.*}}verify.m.tmp.invalid'
 // CHECK-NEXT: 2 errors generated.
diff --git a/clang/test/Frontend/verify.c b/clang/test/Frontend/verify.c
index 221b715c19e416e..657a9d3f2bf6bb7 100644
--- a/clang/test/Frontend/verify.c
+++ 

[clang-tools-extra] [llvm] [clang] [CLANG] Fix : More Detailed "No expected directives found" (PR #78338)

[Timm Baeder via cfe-commits]

https://github.com/tbaederr edited 
https://github.com/llvm/llvm-project/pull/78338
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] [llvm] [clang] [CLANG] Fix : More Detailed "No expected directives found" (PR #78338)

[Anton Korobeynikov via cfe-commits]

https://github.com/asl edited https://github.com/llvm/llvm-project/pull/78338
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] [llvm] [clang] [CLANG] Fix : More Detailed "No expected directives found" (PR #78338)

[Anton Korobeynikov via cfe-commits]

@@ -9,7 +9,9 @@
 /// \file
 /// Defines the Diagnostic-related interfaces.
 //
-//===--===//
+//===--===//]
+
+// look into this file as well

asl wrote:

Remove

https://github.com/llvm/llvm-project/pull/78338
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits