[clang] [clang] print correct context for diagnostics suppressed by deduction (PR #125453)

2025-02-20 Thread Matheus Izvekov via cfe-commits 

https://github.com/mizvekov closed 
https://github.com/llvm/llvm-project/pull/125453
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] print correct context for diagnostics suppressed by deduction (PR #125453)

2025-02-20 Thread via cfe-commits 

https://github.com/cor3ntin approved this pull request.


https://github.com/llvm/llvm-project/pull/125453
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] print correct context for diagnostics suppressed by deduction (PR #125453)

2025-02-19 Thread Matheus Izvekov via cfe-commits 

mizvekov wrote:

@cor3ntin ping

https://github.com/llvm/llvm-project/pull/125453
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] print correct context for diagnostics suppressed by deduction (PR #125453)

2025-02-19 Thread Matheus Izvekov via cfe-commits 


@@ -1909,7 +1909,19 @@ class Sema final : public SemaBase {
   /// '\#pragma clang attribute push' directives to the given declaration.
   void AddPragmaAttributes(Scope *S, Decl *D);
 
-  void PrintPragmaAttributeInstantiationPoint();
+  using DiagFuncRef =

mizvekov wrote:

Sounds good, done.

https://github.com/llvm/llvm-project/pull/125453
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] print correct context for diagnostics suppressed by deduction (PR #125453)

2025-02-19 Thread Matheus Izvekov via cfe-commits 


@@ -1654,11 +1654,20 @@ void Sema::EmitDiagnostic(unsigned DiagID, const 
DiagnosticBuilder &DB) {
 }
 
 case DiagnosticIDs::SFINAE_Suppress:
+  if (DiagnosticsEngine::Level Level = getDiagnostics().getDiagnosticLevel(
+  DiagInfo.getID(), DiagInfo.getLocation());
+  Level == DiagnosticsEngine::Ignored)
+return;
   // Make a copy of this suppressed diagnostic and store it with the
   // template-deduction information;
   if (*Info) {
-(*Info)->addSuppressedDiagnostic(DiagInfo.getLocation(),
-   PartialDiagnostic(DiagInfo, 
Context.getDiagAllocator()));
+(*Info)->addSuppressedDiagnostic(
+DiagInfo.getLocation(),
+PartialDiagnostic(DiagInfo, Context.getDiagAllocator()));
+if (!Diags.getDiagnosticIDs()->isNote(DiagID))
+  PrintContextStack([Info](SourceLocation Loc, PartialDiagnostic PD) {
+(*Info)->addSuppressedDiagnostic(Loc, std::move(PD));
+  });

mizvekov wrote:

We weren't generating the context stack notes from the point the error was 
issued and saved, we would instead defer these notes to the later point where 
the saved error was emitted, which is when the declaration being deduced was 
actually used.

This patch saves the context note from the point the error was issued, and 
stops emitting the context notes from the point of decl use.

So I don't see how we could emit duplicate diagnostics here, other than 
pre-existing issues where the error itself or some of its manually emitted 
notes are redundant with some context notes.

https://github.com/llvm/llvm-project/pull/125453
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] print correct context for diagnostics suppressed by deduction (PR #125453)

2025-02-19 Thread Matheus Izvekov via cfe-commits 

https://github.com/mizvekov updated 
https://github.com/llvm/llvm-project/pull/125453

>From 228d525709e5ba2fd9c8f929dc90db1ccbc915bb Mon Sep 17 00:00:00 2001
From: Matheus Izvekov 
Date: Sun, 2 Feb 2025 23:47:15 -0300
Subject: [PATCH] [clang] print correct context for diagnostics suppressed by
 deduction

This patch makes it so the correct instantiation context is printed
for diagnostics suppessed by template argument deduction.

The context is saved along with the suppressed diagnostic, and
when the declaration they were attached to becomes used, we print
the correct context, instead of whatever context was at this point.
---
 clang/docs/ReleaseNotes.rst   |   4 +-
 clang/include/clang/Sema/Sema.h   |  30 +-
 clang/lib/Sema/Sema.cpp   |  13 +-
 clang/lib/Sema/SemaAttr.cpp   |   7 +-
 clang/lib/Sema/SemaExpr.cpp   |   7 +-
 clang/lib/Sema/SemaTemplateInstantiate.cpp| 271 +-
 clang/test/CXX/drs/cwg0xx.cpp |   5 +
 clang/test/CXX/drs/cwg4xx.cpp |   3 +-
 .../CXX/temp/temp.arg/temp.arg.type/p2.cpp|   9 +-
 clang/test/SemaCXX/anonymous-struct.cpp   |   5 +-
 clang/test/SemaCXX/bool-increment-SFINAE.cpp  |   4 +-
 clang/test/SemaCXX/cxx98-compat-flags.cpp |   2 +
 clang/test/SemaCXX/cxx98-compat.cpp   |   2 +
 clang/test/SemaCXX/deprecated.cpp |   4 +-
 clang/test/SemaCXX/lambda-expressions.cpp |   2 +
 clang/test/SemaCXX/undefined-internal.cpp |   6 +-
 clang/test/SemaTemplate/recovery-crash.cpp|   1 +
 clang/test/SemaTemplate/temp_arg_nontype.cpp  |   1 +
 18 files changed, 214 insertions(+), 162 deletions(-)

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index a91c764860ccd..6f4c69aef0db5 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -168,6 +168,8 @@ Bug Fixes to C++ Support
 
 
 - Clang is now better at keeping track of friend function template instance 
contexts. (#GH55509)
+- Clang now prints the correct instantiation context for diagnostics suppressed
+  by template argument deduction.
 - The initialization kind of elements of structured bindings
   direct-list-initialized from an array is corrected to direct-initialization.
 
@@ -271,7 +273,7 @@ Code Completion
 Static Analyzer
 ---
 
-- Clang currently support extending lifetime of object bound to 
+- Clang currently support extending lifetime of object bound to
   reference members of aggregates in CFG and ExprEngine, that are
   created from default member initializer.
 
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index c55b964650323..093e9a06b00ce 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -1908,7 +1908,23 @@ class Sema final : public SemaBase {
   /// '\#pragma clang attribute push' directives to the given declaration.
   void AddPragmaAttributes(Scope *S, Decl *D);
 
-  void PrintPragmaAttributeInstantiationPoint();
+  using InstantiationContextDiagFuncRef =
+  llvm::function_ref;
+  auto getDefaultDiagFunc() {
+return [this](SourceLocation Loc, PartialDiagnostic PD) {
+  // This bypasses a lot of the filters in the diag engine, as it's
+  // to be used to attach notes to diagnostics which have already
+  // been filtered through.
+  DiagnosticBuilder Builder(Diags.Report(Loc, PD.getDiagID()));
+  PD.Emit(Builder);
+};
+  }
+
+  void PrintPragmaAttributeInstantiationPoint(
+  InstantiationContextDiagFuncRef DiagFunc);
+  void PrintPragmaAttributeInstantiationPoint() {
+PrintPragmaAttributeInstantiationPoint(getDefaultDiagFunc());
+  }
 
   void DiagnoseUnterminatedPragmaAttribute();
 
@@ -13263,18 +13279,22 @@ class Sema final : public SemaBase {
   void pushCodeSynthesisContext(CodeSynthesisContext Ctx);
   void popCodeSynthesisContext();
 
-  void PrintContextStack() {
+  void PrintContextStack(InstantiationContextDiagFuncRef DiagFunc) {
 if (!CodeSynthesisContexts.empty() &&
 CodeSynthesisContexts.size() != LastEmittedCodeSynthesisContextDepth) {
-  PrintInstantiationStack();
+  PrintInstantiationStack(DiagFunc);
   LastEmittedCodeSynthesisContextDepth = CodeSynthesisContexts.size();
 }
 if (PragmaAttributeCurrentTargetDecl)
-  PrintPragmaAttributeInstantiationPoint();
+  PrintPragmaAttributeInstantiationPoint(DiagFunc);
   }
+  void PrintContextStack() { PrintContextStack(getDefaultDiagFunc()); }
   /// Prints the current instantiation stack through a series of
   /// notes.
-  void PrintInstantiationStack();
+  void PrintInstantiationStack(InstantiationContextDiagFuncRef DiagFunc);
+  void PrintInstantiationStack() {
+PrintInstantiationStack(getDefaultDiagFunc());
+  }
 
   /// Determines whether we are currently in a context where
   /// template argument substitution failures are not considered
diff --git a/clang/li

[clang] [clang] print correct context for diagnostics suppressed by deduction (PR #125453)

2025-02-19 Thread Matheus Izvekov via cfe-commits 

https://github.com/mizvekov updated 
https://github.com/llvm/llvm-project/pull/125453

>From f7e2964999d5410a1f954e5142741e929f10f43e Mon Sep 17 00:00:00 2001
From: Matheus Izvekov 
Date: Sun, 2 Feb 2025 23:47:15 -0300
Subject: [PATCH] [clang] print correct context for diagnostics suppressed by
 deduction

This patch makes it so the correct instantiation context is printed
for diagnostics suppessed by template argument deduction.

The context is saved along with the suppressed diagnostic, and
when the declaration they were attached to becomes used, we print
the correct context, instead of whatever context was at this point.
---
 clang/docs/ReleaseNotes.rst   |   4 +-
 clang/include/clang/Sema/Sema.h   |  30 +-
 clang/lib/Sema/Sema.cpp   |  13 +-
 clang/lib/Sema/SemaAttr.cpp   |   7 +-
 clang/lib/Sema/SemaExpr.cpp   |   7 +-
 clang/lib/Sema/SemaTemplateInstantiate.cpp| 271 +-
 clang/test/CXX/drs/cwg0xx.cpp |   5 +
 clang/test/CXX/drs/cwg4xx.cpp |   3 +-
 .../CXX/temp/temp.arg/temp.arg.type/p2.cpp|   9 +-
 clang/test/SemaCXX/anonymous-struct.cpp   |   5 +-
 clang/test/SemaCXX/bool-increment-SFINAE.cpp  |   4 +-
 clang/test/SemaCXX/cxx98-compat-flags.cpp |   2 +
 clang/test/SemaCXX/cxx98-compat.cpp   |   2 +
 clang/test/SemaCXX/deprecated.cpp |   4 +-
 clang/test/SemaCXX/lambda-expressions.cpp |   2 +
 clang/test/SemaCXX/undefined-internal.cpp |   6 +-
 clang/test/SemaTemplate/recovery-crash.cpp|   1 +
 clang/test/SemaTemplate/temp_arg_nontype.cpp  |   1 +
 18 files changed, 214 insertions(+), 162 deletions(-)

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index a91c764860ccd..6f4c69aef0db5 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -168,6 +168,8 @@ Bug Fixes to C++ Support
 
 
 - Clang is now better at keeping track of friend function template instance 
contexts. (#GH55509)
+- Clang now prints the correct instantiation context for diagnostics suppressed
+  by template argument deduction.
 - The initialization kind of elements of structured bindings
   direct-list-initialized from an array is corrected to direct-initialization.
 
@@ -271,7 +273,7 @@ Code Completion
 Static Analyzer
 ---
 
-- Clang currently support extending lifetime of object bound to 
+- Clang currently support extending lifetime of object bound to
   reference members of aggregates in CFG and ExprEngine, that are
   created from default member initializer.
 
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index c55b964650323..d2c43cf407d2d 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -1908,7 +1908,23 @@ class Sema final : public SemaBase {
   /// '\#pragma clang attribute push' directives to the given declaration.
   void AddPragmaAttributes(Scope *S, Decl *D);
 
-  void PrintPragmaAttributeInstantiationPoint();
+  using InstantiationContextDiagFuncRef =
+  llvm::function_ref;
+  auto getDefaultDiagFunc() {
+return [this](SourceLocation Loc, PartialDiagnostic PD) {
+  // This bypasses a lof of the filters in the diag engine, as it's
+  // to be used to attach notes to diagnostics which have already
+  // been filtered through.
+  DiagnosticBuilder Builder(Diags.Report(Loc, PD.getDiagID()));
+  PD.Emit(Builder);
+};
+  }
+
+  void PrintPragmaAttributeInstantiationPoint(
+  InstantiationContextDiagFuncRef DiagFunc);
+  void PrintPragmaAttributeInstantiationPoint() {
+PrintPragmaAttributeInstantiationPoint(getDefaultDiagFunc());
+  }
 
   void DiagnoseUnterminatedPragmaAttribute();
 
@@ -13263,18 +13279,22 @@ class Sema final : public SemaBase {
   void pushCodeSynthesisContext(CodeSynthesisContext Ctx);
   void popCodeSynthesisContext();
 
-  void PrintContextStack() {
+  void PrintContextStack(InstantiationContextDiagFuncRef DiagFunc) {
 if (!CodeSynthesisContexts.empty() &&
 CodeSynthesisContexts.size() != LastEmittedCodeSynthesisContextDepth) {
-  PrintInstantiationStack();
+  PrintInstantiationStack(DiagFunc);
   LastEmittedCodeSynthesisContextDepth = CodeSynthesisContexts.size();
 }
 if (PragmaAttributeCurrentTargetDecl)
-  PrintPragmaAttributeInstantiationPoint();
+  PrintPragmaAttributeInstantiationPoint(DiagFunc);
   }
+  void PrintContextStack() { PrintContextStack(getDefaultDiagFunc()); }
   /// Prints the current instantiation stack through a series of
   /// notes.
-  void PrintInstantiationStack();
+  void PrintInstantiationStack(InstantiationContextDiagFuncRef DiagFunc);
+  void PrintInstantiationStack() {
+PrintInstantiationStack(getDefaultDiagFunc());
+  }
 
   /// Determines whether we are currently in a context where
   /// template argument substitution failures are not considered
diff --git a/clang/li

[clang] [clang] print correct context for diagnostics suppressed by deduction (PR #125453)

2025-02-07 Thread Younan Zhang via cfe-commits 


@@ -1909,7 +1909,22 @@ class Sema final : public SemaBase {
   /// '\#pragma clang attribute push' directives to the given declaration.
   void AddPragmaAttributes(Scope *S, Decl *D);
 
-  void PrintPragmaAttributeInstantiationPoint();
+  using DiagFuncRef =
+  llvm::function_ref;
+  auto getDefaultDiagFunc() {
+return [this](SourceLocation Loc, PartialDiagnostic PD) {
+  // This bypasses a lof of the filters in the diag engine, as it's

zyn0217 wrote:

```suggestion
  // This bypasses a lot of the filters in the diag engine, as it's
```

https://github.com/llvm/llvm-project/pull/125453
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] print correct context for diagnostics suppressed by deduction (PR #125453)

2025-02-07 Thread Younan Zhang via cfe-commits 


@@ -1909,7 +1909,19 @@ class Sema final : public SemaBase {
   /// '\#pragma clang attribute push' directives to the given declaration.
   void AddPragmaAttributes(Scope *S, Decl *D);
 
-  void PrintPragmaAttributeInstantiationPoint();
+  using DiagFuncRef =

zyn0217 wrote:

I think I agree with @cor3ntin here, we need a more specific name than 
`DiagFuncRef` (which looks like something supposed to be localized in a 
function other than the god Sema) - how about `InstantiationContextDiagFuncRef`?

https://github.com/llvm/llvm-project/pull/125453
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] print correct context for diagnostics suppressed by deduction (PR #125453)

2025-02-07 Thread Younan Zhang via cfe-commits 


@@ -1654,11 +1654,20 @@ void Sema::EmitDiagnostic(unsigned DiagID, const 
DiagnosticBuilder &DB) {
 }
 
 case DiagnosticIDs::SFINAE_Suppress:
+  if (DiagnosticsEngine::Level Level = getDiagnostics().getDiagnosticLevel(
+  DiagInfo.getID(), DiagInfo.getLocation());
+  Level == DiagnosticsEngine::Ignored)
+return;
   // Make a copy of this suppressed diagnostic and store it with the
   // template-deduction information;
   if (*Info) {
-(*Info)->addSuppressedDiagnostic(DiagInfo.getLocation(),
-   PartialDiagnostic(DiagInfo, 
Context.getDiagAllocator()));
+(*Info)->addSuppressedDiagnostic(
+DiagInfo.getLocation(),
+PartialDiagnostic(DiagInfo, Context.getDiagAllocator()));
+if (!Diags.getDiagnosticIDs()->isNote(DiagID))
+  PrintContextStack([Info](SourceLocation Loc, PartialDiagnostic PD) {
+(*Info)->addSuppressedDiagnostic(Loc, std::move(PD));
+  });

zyn0217 wrote:

If I read it right, the issue arises because we weren't recording 
`instantiation required at...` in TemplateDeductionInfo, right? Is there any 
risk of emitting duplicate diagnostics by doing so? (I might be overconcerned 
if I'm misremembering some implementation details.)

https://github.com/llvm/llvm-project/pull/125453
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] print correct context for diagnostics suppressed by deduction (PR #125453)

2025-02-06 Thread Matheus Izvekov via cfe-commits 

https://github.com/mizvekov updated 
https://github.com/llvm/llvm-project/pull/125453

>From a36dcb035a30b560eb9dd14fd3ce2f8e8952b8ee Mon Sep 17 00:00:00 2001
From: Matheus Izvekov 
Date: Sun, 2 Feb 2025 23:47:15 -0300
Subject: [PATCH] [clang] print correct context for diagnostics suppressed by
 deduction

This patch makes it so the correct instantiation context is printed
for diagnostics suppessed by template argument deduction.

The context is saved along with the suppressed diagnostic, and
when the declaration they were attached to becomes used, we print
the correct context, instead of whatever context was at this point.
---
 clang/docs/ReleaseNotes.rst   |   2 +
 clang/include/clang/Sema/Sema.h   |  29 +-
 clang/lib/Sema/Sema.cpp   |  13 +-
 clang/lib/Sema/SemaAttr.cpp   |   6 +-
 clang/lib/Sema/SemaExpr.cpp   |   7 +-
 clang/lib/Sema/SemaTemplateInstantiate.cpp| 271 +-
 clang/test/CXX/drs/cwg0xx.cpp |   5 +
 clang/test/CXX/drs/cwg4xx.cpp |   3 +-
 .../CXX/temp/temp.arg/temp.arg.type/p2.cpp|   9 +-
 clang/test/SemaCXX/anonymous-struct.cpp   |   5 +-
 clang/test/SemaCXX/bool-increment-SFINAE.cpp  |   4 +-
 clang/test/SemaCXX/cxx98-compat-flags.cpp |   2 +
 clang/test/SemaCXX/cxx98-compat.cpp   |   2 +
 clang/test/SemaCXX/deprecated.cpp |   4 +-
 clang/test/SemaCXX/lambda-expressions.cpp |   2 +
 clang/test/SemaCXX/undefined-internal.cpp |   6 +-
 clang/test/SemaTemplate/recovery-crash.cpp|   1 +
 clang/test/SemaTemplate/temp_arg_nontype.cpp  |   1 +
 18 files changed, 211 insertions(+), 161 deletions(-)

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 21c1ff25d2862ba..dfce173d414ebb2 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -146,6 +146,8 @@ Bug Fixes to C++ Support
 
 
 - Clang is now better at keeping track of friend function template instance 
contexts. (#GH55509)
+- Clang now prints the correct instantiation context for diagnostics suppressed
+  by template argument deduction.
 
 Bug Fixes to AST Handling
 ^
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index 1870d1271c556cb..e39930c1a450343 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -1909,7 +1909,22 @@ class Sema final : public SemaBase {
   /// '\#pragma clang attribute push' directives to the given declaration.
   void AddPragmaAttributes(Scope *S, Decl *D);
 
-  void PrintPragmaAttributeInstantiationPoint();
+  using DiagFuncRef =
+  llvm::function_ref;
+  auto getDefaultDiagFunc() {
+return [this](SourceLocation Loc, PartialDiagnostic PD) {
+  // This bypasses a lof of the filters in the diag engine, as it's
+  // to be used to attach notes to diagnostics which have already
+  // been filtered through.
+  DiagnosticBuilder Builder(Diags.Report(Loc, PD.getDiagID()));
+  PD.Emit(Builder);
+};
+  }
+
+  void PrintPragmaAttributeInstantiationPoint(DiagFuncRef DiagFunc);
+  void PrintPragmaAttributeInstantiationPoint() {
+PrintPragmaAttributeInstantiationPoint(getDefaultDiagFunc());
+  }
 
   void DiagnoseUnterminatedPragmaAttribute();
 
@@ -13263,18 +13278,22 @@ class Sema final : public SemaBase {
   void pushCodeSynthesisContext(CodeSynthesisContext Ctx);
   void popCodeSynthesisContext();
 
-  void PrintContextStack() {
+  void PrintContextStack(DiagFuncRef DiagFunc) {
 if (!CodeSynthesisContexts.empty() &&
 CodeSynthesisContexts.size() != LastEmittedCodeSynthesisContextDepth) {
-  PrintInstantiationStack();
+  PrintInstantiationStack(DiagFunc);
   LastEmittedCodeSynthesisContextDepth = CodeSynthesisContexts.size();
 }
 if (PragmaAttributeCurrentTargetDecl)
-  PrintPragmaAttributeInstantiationPoint();
+  PrintPragmaAttributeInstantiationPoint(DiagFunc);
   }
+  void PrintContextStack() { PrintContextStack(getDefaultDiagFunc()); }
   /// Prints the current instantiation stack through a series of
   /// notes.
-  void PrintInstantiationStack();
+  void PrintInstantiationStack(DiagFuncRef DiagFunc);
+  void PrintInstantiationStack() {
+PrintInstantiationStack(getDefaultDiagFunc());
+  }
 
   /// Determines whether we are currently in a context where
   /// template argument substitution failures are not considered
diff --git a/clang/lib/Sema/Sema.cpp b/clang/lib/Sema/Sema.cpp
index 15c18f9a4525b22..f3f7111a3a200f3 100644
--- a/clang/lib/Sema/Sema.cpp
+++ b/clang/lib/Sema/Sema.cpp
@@ -1654,11 +1654,20 @@ void Sema::EmitDiagnostic(unsigned DiagID, const 
DiagnosticBuilder &DB) {
 }
 
 case DiagnosticIDs::SFINAE_Suppress:
+  if (DiagnosticsEngine::Level Level = getDiagnostics().getDiagnosticLevel(
+  DiagInfo.getID(), DiagInfo.getLocation());
+  Level == DiagnosticsEngine::Ignored)

[clang] [clang] print correct context for diagnostics suppressed by deduction (PR #125453)

2025-02-05 Thread Matheus Izvekov via cfe-commits 

mizvekov wrote:

https://llvm-compile-time-tracker.com/compare.php?from=4eab2194872d54e2d4496135a277b1610ff33ead&to=adde9f1f8eabe4d98ba09fd978f8d152a9865347&stat=instructions:u

These performance results don't indicate an impact either.

https://github.com/llvm/llvm-project/pull/125453
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] print correct context for diagnostics suppressed by deduction (PR #125453)

2025-02-05 Thread Erich Keane via cfe-commits 


@@ -1909,7 +1909,19 @@ class Sema final : public SemaBase {
   /// '\#pragma clang attribute push' directives to the given declaration.
   void AddPragmaAttributes(Scope *S, Decl *D);
 
-  void PrintPragmaAttributeInstantiationPoint();
+  using DiagFuncRef =
+  llvm::function_ref;
+  auto getDefaultDiagFunc() {
+return [this](SourceLocation Loc, PartialDiagnostic PD) {

erichkeane wrote:

Ah, sorry I didn't clarify.  I'm fine not using `PartialDiagnosticAt`, I've had 
a similar concern with a different patch I was working on.  I was mostly 
answering your question/exposition about the source locations.  

https://github.com/llvm/llvm-project/pull/125453
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] print correct context for diagnostics suppressed by deduction (PR #125453)

2025-02-05 Thread Matheus Izvekov via cfe-commits 


@@ -1909,7 +1909,19 @@ class Sema final : public SemaBase {
   /// '\#pragma clang attribute push' directives to the given declaration.
   void AddPragmaAttributes(Scope *S, Decl *D);
 
-  void PrintPragmaAttributeInstantiationPoint();
+  using DiagFuncRef =
+  llvm::function_ref;
+  auto getDefaultDiagFunc() {
+return [this](SourceLocation Loc, PartialDiagnostic PD) {

mizvekov wrote:

I tried that, but I didn't like the result.

most of the uses are lines like:
```
  DiagFunc(Active->PointOfInstantiation,
   PDiag(diag::note_template_default_arg_checking)
   << getTemplateArgumentBindingsText(TemplateParams,
  Active->TemplateArgs,
  Active->NumTemplateArgs)
   << Active->InstantiationRange);
```

Which become:
```
  DiagFunc(PartialDiagnosticAt(Active->PointOfInstantiation,
   PDiag(diag::note_template_default_arg_checking)
   << getTemplateArgumentBindingsText(TemplateParams,
  Active->TemplateArgs,
  Active->NumTemplateArgs)
   << Active->InstantiationRange));
```

You can swap the explicit construction with braces, but that still looks a bit 
off.

On the receiving side, none of the users take a PartialDiagnosticAt anyway, so 
you have to decompose it.

So right now this is just adding friction to the API.

If we really wanted to do this, I think we would have to provide equivalents to 
all existing APIs which take a partial diagnostic, including the '<<' operator, 
then this would look nice, but it's a bigger cleanup which is a bit unrelated 
to this patch.

Ie if we had:
```C++
  DiagFunc(PDiagAt(Active->PointOfInstantiation,
   diag::note_template_default_arg_checking)
   << getTemplateArgumentBindingsText(TemplateParams,
  Active->TemplateArgs,
  Active->NumTemplateArgs)
   << Active->InstantiationRange));
```
And:
```C++
return [this](PartialDiagnosticAt PDA) {
  DiagnosticBuilder Builder(Diags.Report(PDA));
  PDA.Emit(Builder);
}
```

https://github.com/llvm/llvm-project/pull/125453
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] print correct context for diagnostics suppressed by deduction (PR #125453)

2025-02-05 Thread Matheus Izvekov via cfe-commits 

https://github.com/mizvekov updated 
https://github.com/llvm/llvm-project/pull/125453

>From adde9f1f8eabe4d98ba09fd978f8d152a9865347 Mon Sep 17 00:00:00 2001
From: Matheus Izvekov 
Date: Sun, 2 Feb 2025 23:47:15 -0300
Subject: [PATCH] [clang] print correct context for diagnostics suppressed by
 deduction

This patch makes it so the correct instantiation context is printed
for diagnostics suppessed by template argument deduction.

The context is saved along with the suppressed diagnostic, and
when the declaration they were attached to becomes used, we print
the correct context, instead of whatever context was at this point.
---
 clang/docs/ReleaseNotes.rst   |   2 +
 clang/include/clang/Sema/Sema.h   |  26 +-
 clang/lib/Sema/Sema.cpp   |  13 +-
 clang/lib/Sema/SemaAttr.cpp   |   6 +-
 clang/lib/Sema/SemaExpr.cpp   |   7 +-
 clang/lib/Sema/SemaTemplateInstantiate.cpp| 271 +-
 clang/test/CXX/drs/cwg0xx.cpp |   5 +
 clang/test/CXX/drs/cwg4xx.cpp |   3 +-
 .../CXX/temp/temp.arg/temp.arg.type/p2.cpp|   9 +-
 clang/test/SemaCXX/anonymous-struct.cpp   |   5 +-
 clang/test/SemaCXX/bool-increment-SFINAE.cpp  |   4 +-
 clang/test/SemaCXX/cxx98-compat-flags.cpp |   2 +
 clang/test/SemaCXX/cxx98-compat.cpp   |   2 +
 clang/test/SemaCXX/deprecated.cpp |   4 +-
 clang/test/SemaCXX/lambda-expressions.cpp |   2 +
 clang/test/SemaCXX/undefined-internal.cpp |   6 +-
 clang/test/SemaTemplate/recovery-crash.cpp|   1 +
 clang/test/SemaTemplate/temp_arg_nontype.cpp  |   1 +
 18 files changed, 208 insertions(+), 161 deletions(-)

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 21c1ff25d2862ba..dfce173d414ebb2 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -146,6 +146,8 @@ Bug Fixes to C++ Support
 
 
 - Clang is now better at keeping track of friend function template instance 
contexts. (#GH55509)
+- Clang now prints the correct instantiation context for diagnostics suppressed
+  by template argument deduction.
 
 Bug Fixes to AST Handling
 ^
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index 1870d1271c556cb..c0e01c817cf100b 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -1909,7 +1909,19 @@ class Sema final : public SemaBase {
   /// '\#pragma clang attribute push' directives to the given declaration.
   void AddPragmaAttributes(Scope *S, Decl *D);
 
-  void PrintPragmaAttributeInstantiationPoint();
+  using DiagFuncRef =
+  llvm::function_ref;
+  auto getDefaultDiagFunc() {
+return [this](SourceLocation Loc, PartialDiagnostic PD) {
+  DiagnosticBuilder Builder(Diags.Report(Loc, PD.getDiagID()));
+  PD.Emit(Builder);
+};
+  }
+
+  void PrintPragmaAttributeInstantiationPoint(DiagFuncRef DiagFunc);
+  void PrintPragmaAttributeInstantiationPoint() {
+PrintPragmaAttributeInstantiationPoint(getDefaultDiagFunc());
+  }
 
   void DiagnoseUnterminatedPragmaAttribute();
 
@@ -13263,18 +13275,22 @@ class Sema final : public SemaBase {
   void pushCodeSynthesisContext(CodeSynthesisContext Ctx);
   void popCodeSynthesisContext();
 
-  void PrintContextStack() {
+  void PrintContextStack(DiagFuncRef DiagFunc) {
 if (!CodeSynthesisContexts.empty() &&
 CodeSynthesisContexts.size() != LastEmittedCodeSynthesisContextDepth) {
-  PrintInstantiationStack();
+  PrintInstantiationStack(DiagFunc);
   LastEmittedCodeSynthesisContextDepth = CodeSynthesisContexts.size();
 }
 if (PragmaAttributeCurrentTargetDecl)
-  PrintPragmaAttributeInstantiationPoint();
+  PrintPragmaAttributeInstantiationPoint(DiagFunc);
   }
+  void PrintContextStack() { PrintContextStack(getDefaultDiagFunc()); }
   /// Prints the current instantiation stack through a series of
   /// notes.
-  void PrintInstantiationStack();
+  void PrintInstantiationStack(DiagFuncRef DiagFunc);
+  void PrintInstantiationStack() {
+PrintInstantiationStack(getDefaultDiagFunc());
+  }
 
   /// Determines whether we are currently in a context where
   /// template argument substitution failures are not considered
diff --git a/clang/lib/Sema/Sema.cpp b/clang/lib/Sema/Sema.cpp
index 15c18f9a4525b22..f3f7111a3a200f3 100644
--- a/clang/lib/Sema/Sema.cpp
+++ b/clang/lib/Sema/Sema.cpp
@@ -1654,11 +1654,20 @@ void Sema::EmitDiagnostic(unsigned DiagID, const 
DiagnosticBuilder &DB) {
 }
 
 case DiagnosticIDs::SFINAE_Suppress:
+  if (DiagnosticsEngine::Level Level = getDiagnostics().getDiagnosticLevel(
+  DiagInfo.getID(), DiagInfo.getLocation());
+  Level == DiagnosticsEngine::Ignored)
+return;
   // Make a copy of this suppressed diagnostic and store it with the
   // template-deduction information;
   if (*Info) {
-(*Info)->addSup

[clang] [clang] print correct context for diagnostics suppressed by deduction (PR #125453)

2025-02-04 Thread Matheus Izvekov via cfe-commits 

mizvekov wrote:

> it's a permitted point of instantiation; we could try it and see if it works 
> well enough in practice. We could also delay implicit definitions of special 
> members like we do for template instantiations to reduce the impact.

Another possibility would be only saving the context up to the point of use of 
the decl, and then finishing the ancestors from there.

https://github.com/llvm/llvm-project/pull/125453
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] print correct context for diagnostics suppressed by deduction (PR #125453)

2025-02-04 Thread Erich Keane via cfe-commits 


@@ -1909,7 +1909,19 @@ class Sema final : public SemaBase {
   /// '\#pragma clang attribute push' directives to the given declaration.
   void AddPragmaAttributes(Scope *S, Decl *D);
 
-  void PrintPragmaAttributeInstantiationPoint();
+  using DiagFuncRef =
+  llvm::function_ref;
+  auto getDefaultDiagFunc() {
+return [this](SourceLocation Loc, PartialDiagnostic PD) {

erichkeane wrote:

`PartialDiagnostic`, or `PDiag` don't actually have a location.  
`PartialDiagnostic` is only the diag-ID, plus up to the full-list (meaning, 
doesn't have to be the full list) of `<<` arguments.  `PartialDiagnosticAt` is 
a `std::pair` for cases where you already know the location, exactly the use 
here.

https://github.com/llvm/llvm-project/pull/125453
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] print correct context for diagnostics suppressed by deduction (PR #125453)

2025-02-04 Thread Erich Keane via cfe-commits 


@@ -1909,7 +1909,19 @@ class Sema final : public SemaBase {
   /// '\#pragma clang attribute push' directives to the given declaration.
   void AddPragmaAttributes(Scope *S, Decl *D);
 
-  void PrintPragmaAttributeInstantiationPoint();
+  using DiagFuncRef =
+  llvm::function_ref;
+  auto getDefaultDiagFunc() {
+return [this](SourceLocation Loc, PartialDiagnostic PD) {
+  DiagnosticBuilder Builder(Diags.Report(Loc, PD.getDiagID()));

erichkeane wrote:

ah!  Thanks for the clarifiation.  Can we get a comment to that effect?

https://github.com/llvm/llvm-project/pull/125453
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] print correct context for diagnostics suppressed by deduction (PR #125453)

2025-02-03 Thread Matheus Izvekov via cfe-commits 

mizvekov wrote:

> Thank you for tackling this longstanding issue! How much does saving this 
> extra state add to the runtime and memory usage on a template-heavy 
> compilation?

I tried building stdexec, the difference is within the noise.
Do you have any other examples of template-heavy code in mind?

> If the cost is concerning, one other option we could consider here would be 
> performing pending local instantiations eagerly when we reach the end of a 
> region in which we have a context note -- that'd mean we don't need to save 
> state for later. That's subtly behavior changing because it will pick an 
> earlier point of instantiation for those local instantiations, but it's a 
> permitted point of instantiation; we could try it and see if it works well 
> enough in practice. We could also delay implicit definitions of special 
> members like we do for template instantiations to reduce the impact.

Sounds worth a try, if we can find any concerns.

https://github.com/llvm/llvm-project/pull/125453
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] print correct context for diagnostics suppressed by deduction (PR #125453)

2025-02-03 Thread Matheus Izvekov via cfe-commits 

https://github.com/mizvekov updated 
https://github.com/llvm/llvm-project/pull/125453

>From ff34c10553ed645fc8277cb72247f6cb67a024e6 Mon Sep 17 00:00:00 2001
From: Matheus Izvekov 
Date: Sun, 2 Feb 2025 23:47:15 -0300
Subject: [PATCH] [clang] print correct context for diagnostics suppressed by
 deduction

This patch makes it so the correct instantiation context is printed
for diagnostics suppessed by template argument deduction.

The context is saved along with the suppressed diagnostic, and
when the declaration they were attached to becomes used, we print
the correct context, instead of whatever context was at this point.
---
 clang/docs/ReleaseNotes.rst   |   3 +
 clang/include/clang/Sema/Sema.h   |  26 +-
 clang/lib/Sema/Sema.cpp   |  13 +-
 clang/lib/Sema/SemaAttr.cpp   |   6 +-
 clang/lib/Sema/SemaExpr.cpp   |   7 +-
 clang/lib/Sema/SemaTemplateInstantiate.cpp| 271 +-
 clang/test/CXX/drs/cwg0xx.cpp |   5 +
 clang/test/CXX/drs/cwg4xx.cpp |   3 +-
 .../CXX/temp/temp.arg/temp.arg.type/p2.cpp|   9 +-
 clang/test/SemaCXX/anonymous-struct.cpp   |   5 +-
 clang/test/SemaCXX/bool-increment-SFINAE.cpp  |   4 +-
 clang/test/SemaCXX/cxx98-compat-flags.cpp |   2 +
 clang/test/SemaCXX/cxx98-compat.cpp   |   2 +
 clang/test/SemaCXX/deprecated.cpp |   4 +-
 clang/test/SemaCXX/lambda-expressions.cpp |   2 +
 clang/test/SemaCXX/undefined-internal.cpp |   6 +-
 clang/test/SemaTemplate/recovery-crash.cpp|   1 +
 clang/test/SemaTemplate/temp_arg_nontype.cpp  |   1 +
 18 files changed, 209 insertions(+), 161 deletions(-)

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 8be1ea2fb01455f..70c3b062d977177 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -128,6 +128,9 @@ Bug Fixes to Attribute Support
 Bug Fixes to C++ Support
 
 
+- Clang now prints the correct instantiation context for diagnostics suppressed
+  by template argument deduction.
+
 Bug Fixes to AST Handling
 ^
 
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index 472a0e25adc9752..7d01dc1aa4c00bd 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -1909,7 +1909,19 @@ class Sema final : public SemaBase {
   /// '\#pragma clang attribute push' directives to the given declaration.
   void AddPragmaAttributes(Scope *S, Decl *D);
 
-  void PrintPragmaAttributeInstantiationPoint();
+  using DiagFuncRef =
+  llvm::function_ref;
+  auto getDefaultDiagFunc() {
+return [this](SourceLocation Loc, PartialDiagnostic PD) {
+  DiagnosticBuilder Builder(Diags.Report(Loc, PD.getDiagID()));
+  PD.Emit(Builder);
+};
+  }
+
+  void PrintPragmaAttributeInstantiationPoint(DiagFuncRef DiagFunc);
+  void PrintPragmaAttributeInstantiationPoint() {
+PrintPragmaAttributeInstantiationPoint(getDefaultDiagFunc());
+  }
 
   void DiagnoseUnterminatedPragmaAttribute();
 
@@ -13260,18 +13272,22 @@ class Sema final : public SemaBase {
   void pushCodeSynthesisContext(CodeSynthesisContext Ctx);
   void popCodeSynthesisContext();
 
-  void PrintContextStack() {
+  void PrintContextStack(DiagFuncRef DiagFunc) {
 if (!CodeSynthesisContexts.empty() &&
 CodeSynthesisContexts.size() != LastEmittedCodeSynthesisContextDepth) {
-  PrintInstantiationStack();
+  PrintInstantiationStack(DiagFunc);
   LastEmittedCodeSynthesisContextDepth = CodeSynthesisContexts.size();
 }
 if (PragmaAttributeCurrentTargetDecl)
-  PrintPragmaAttributeInstantiationPoint();
+  PrintPragmaAttributeInstantiationPoint(DiagFunc);
   }
+  void PrintContextStack() { PrintContextStack(getDefaultDiagFunc()); }
   /// Prints the current instantiation stack through a series of
   /// notes.
-  void PrintInstantiationStack();
+  void PrintInstantiationStack(DiagFuncRef DiagFunc);
+  void PrintInstantiationStack() {
+PrintInstantiationStack(getDefaultDiagFunc());
+  }
 
   /// Determines whether we are currently in a context where
   /// template argument substitution failures are not considered
diff --git a/clang/lib/Sema/Sema.cpp b/clang/lib/Sema/Sema.cpp
index 9507d7602aa401a..33e2bd1e030513f 100644
--- a/clang/lib/Sema/Sema.cpp
+++ b/clang/lib/Sema/Sema.cpp
@@ -1654,11 +1654,20 @@ void Sema::EmitDiagnostic(unsigned DiagID, const 
DiagnosticBuilder &DB) {
 }
 
 case DiagnosticIDs::SFINAE_Suppress:
+  if (DiagnosticsEngine::Level Level = getDiagnostics().getDiagnosticLevel(
+  DiagInfo.getID(), DiagInfo.getLocation());
+  Level == DiagnosticsEngine::Ignored)
+return;
   // Make a copy of this suppressed diagnostic and store it with the
   // template-deduction information;
   if (*Info) {
-(*Info)->addSuppressedDiagnostic(DiagInfo.getLocation(),
-

[clang] [clang] print correct context for diagnostics suppressed by deduction (PR #125453)

2025-02-03 Thread Matheus Izvekov via cfe-commits 


@@ -1909,7 +1909,19 @@ class Sema final : public SemaBase {
   /// '\#pragma clang attribute push' directives to the given declaration.
   void AddPragmaAttributes(Scope *S, Decl *D);
 
-  void PrintPragmaAttributeInstantiationPoint();
+  using DiagFuncRef =
+  llvm::function_ref;
+  auto getDefaultDiagFunc() {
+return [this](SourceLocation Loc, PartialDiagnostic PD) {

mizvekov wrote:

Yeah I know, but nothing we are using takes it, so it doesn't buy much.

I am passing both things for consistency with existing practice, but I don't 
understand why we have PartialDiagnosticsAt if the PartialDiagnostic already 
has a SourceLocation. I may look into that at some point.

https://github.com/llvm/llvm-project/pull/125453
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] print correct context for diagnostics suppressed by deduction (PR #125453)

2025-02-03 Thread Matheus Izvekov via cfe-commits 


@@ -1909,7 +1909,19 @@ class Sema final : public SemaBase {
   /// '\#pragma clang attribute push' directives to the given declaration.
   void AddPragmaAttributes(Scope *S, Decl *D);
 
-  void PrintPragmaAttributeInstantiationPoint();
+  using DiagFuncRef =
+  llvm::function_ref;
+  auto getDefaultDiagFunc() {
+return [this](SourceLocation Loc, PartialDiagnostic PD) {
+  DiagnosticBuilder Builder(Diags.Report(Loc, PD.getDiagID()));

mizvekov wrote:

This bypasses a lot of the things `this->Diag(Loc, PD)` does, like SFINAE 
handling, ignoring notes attached to ignored diagnostics, adding instantiation 
context notes, and more, because we have already done them before storing the 
suppressed diagnostics, and we are just attaching notes to them.

https://github.com/llvm/llvm-project/pull/125453
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] print correct context for diagnostics suppressed by deduction (PR #125453)

2025-02-03 Thread Matheus Izvekov via cfe-commits 


@@ -1909,7 +1909,19 @@ class Sema final : public SemaBase {
   /// '\#pragma clang attribute push' directives to the given declaration.
   void AddPragmaAttributes(Scope *S, Decl *D);
 
-  void PrintPragmaAttributeInstantiationPoint();
+  using DiagFuncRef =
+  llvm::function_ref;
+  auto getDefaultDiagFunc() {
+return [this](SourceLocation Loc, PartialDiagnostic PD) {
+  DiagnosticBuilder Builder(Diags.Report(Loc, PD.getDiagID()));
+  PD.Emit(Builder);
+};
+  }

mizvekov wrote:

I thought about / tried that first, but assuming multiple users, I think there 
is less duplication this way, and the duplication is more obvious.
There are some other functions that can start taking this parameter in future 
patches.

https://github.com/llvm/llvm-project/pull/125453
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] print correct context for diagnostics suppressed by deduction (PR #125453)

2025-02-03 Thread Matheus Izvekov via cfe-commits 


@@ -1909,7 +1909,19 @@ class Sema final : public SemaBase {
   /// '\#pragma clang attribute push' directives to the given declaration.
   void AddPragmaAttributes(Scope *S, Decl *D);
 
-  void PrintPragmaAttributeInstantiationPoint();
+  using DiagFuncRef =

mizvekov wrote:

This is currently used for more things than the InstantiationContext alone, so 
the name doesn't need to be that specific to it.

`InstantiationContextDiagCallback` doesn't ring like a type to me, FuncRef is 
more clear in that respect.

https://github.com/llvm/llvm-project/pull/125453
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] print correct context for diagnostics suppressed by deduction (PR #125453)

2025-02-03 Thread Matheus Izvekov via cfe-commits 


@@ -1654,11 +1654,20 @@ void Sema::EmitDiagnostic(unsigned DiagID, const 
DiagnosticBuilder &DB) {
 }
 
 case DiagnosticIDs::SFINAE_Suppress:
+  if (DiagnosticsEngine::Level Level = getDiagnostics().getDiagnosticLevel(
+  DiagInfo.getID(), DiagInfo.getLocation());
+  Level == DiagnosticsEngine::Ignored)
+return;
   // Make a copy of this suppressed diagnostic and store it with the
   // template-deduction information;
   if (*Info) {
-(*Info)->addSuppressedDiagnostic(DiagInfo.getLocation(),
-   PartialDiagnostic(DiagInfo, 
Context.getDiagAllocator()));
+(*Info)->addSuppressedDiagnostic(
+DiagInfo.getLocation(),
+PartialDiagnostic(DiagInfo, Context.getDiagAllocator()));
+if (!Diags.getDiagnosticIDs()->isNote(DiagID))
+  PrintContextStack([Info](SourceLocation Loc, PartialDiagnostic PD) {
+(*Info)->addSuppressedDiagnostic(Loc, std::move(PD));
+  });

mizvekov wrote:

Yeah, it's one of the first things I tried, but it's harder to debug, and 
harder to be sure of the correctness of the whole thing, as there is a lot of 
interactions there. This callback scheme is way more straightforward and I like 
that.

https://github.com/llvm/llvm-project/pull/125453
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] print correct context for diagnostics suppressed by deduction (PR #125453)

2025-02-03 Thread Erich Keane via cfe-commits 


@@ -1909,7 +1909,19 @@ class Sema final : public SemaBase {
   /// '\#pragma clang attribute push' directives to the given declaration.
   void AddPragmaAttributes(Scope *S, Decl *D);
 
-  void PrintPragmaAttributeInstantiationPoint();
+  using DiagFuncRef =
+  llvm::function_ref;
+  auto getDefaultDiagFunc() {
+return [this](SourceLocation Loc, PartialDiagnostic PD) {
+  DiagnosticBuilder Builder(Diags.Report(Loc, PD.getDiagID()));

erichkeane wrote:

Could this just be `this->Diag(Loc, PD);` ?? Or is there a reason for a new 
Diag-builder/emit call?  It looks like you're capturing `Sema` anyway.

https://github.com/llvm/llvm-project/pull/125453
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] print correct context for diagnostics suppressed by deduction (PR #125453)

2025-02-03 Thread Erich Keane via cfe-commits 

https://github.com/erichkeane commented:

Share some of Corentin's concerns, else just a few comments.

https://github.com/llvm/llvm-project/pull/125453
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] print correct context for diagnostics suppressed by deduction (PR #125453)

2025-02-03 Thread Erich Keane via cfe-commits 


@@ -1909,7 +1909,19 @@ class Sema final : public SemaBase {
   /// '\#pragma clang attribute push' directives to the given declaration.
   void AddPragmaAttributes(Scope *S, Decl *D);
 
-  void PrintPragmaAttributeInstantiationPoint();
+  using DiagFuncRef =
+  llvm::function_ref;
+  auto getDefaultDiagFunc() {
+return [this](SourceLocation Loc, PartialDiagnostic PD) {

erichkeane wrote:

Also, we have `PartialDiagnosticAt` for a `SourceLocation` + 
`PartialDiagnostic`.

https://github.com/llvm/llvm-project/pull/125453
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] print correct context for diagnostics suppressed by deduction (PR #125453)

2025-02-03 Thread Erich Keane via cfe-commits 

https://github.com/erichkeane edited 
https://github.com/llvm/llvm-project/pull/125453
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] print correct context for diagnostics suppressed by deduction (PR #125453)

2025-02-03 Thread Vlad Serebrennikov via cfe-commits 

https://github.com/Endilll edited 
https://github.com/llvm/llvm-project/pull/125453
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] print correct context for diagnostics suppressed by deduction (PR #125453)

2025-02-03 Thread Vlad Serebrennikov via cfe-commits 

https://github.com/Endilll commented:

Changes to C++ DR tests look good otherwise.

https://github.com/llvm/llvm-project/pull/125453
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] print correct context for diagnostics suppressed by deduction (PR #125453)

2025-02-03 Thread Vlad Serebrennikov via cfe-commits 


@@ -1386,6 +1386,7 @@ namespace cwg488 { // cwg488: 2.9 c++11
 enum E { e };
 f(e);
 // cxx98-error@-1 {{template argument uses local type 'E'}}
+// cxx98-note@-2 {{while substituting deduced template arguments}}

Endilll wrote:

```suggestion
//   cxx98-note@-2 {{while substituting deduced template arguments}}
```

https://github.com/llvm/llvm-project/pull/125453
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] print correct context for diagnostics suppressed by deduction (PR #125453)

2025-02-03 Thread Vlad Serebrennikov via cfe-commits 


@@ -1058,8 +1061,10 @@ namespace cwg62 { // cwg62: 2.9
 // cxx98-error@-1 {{template argument uses local type }}
 get();
 // cxx98-error@-1 {{template argument uses local type }}
+// cxx98-note@-2 {{while substituting explicitly-specified template 
arguments}}
 get();
 // cxx98-error@-1 {{template argument uses local type }}
+// cxx98-note@-2 {{while substituting explicitly-specified template 
arguments}}

Endilll wrote:

```suggestion
//   cxx98-note@-2 {{while substituting explicitly-specified template 
arguments}}
```

https://github.com/llvm/llvm-project/pull/125453
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] print correct context for diagnostics suppressed by deduction (PR #125453)

2025-02-03 Thread Vlad Serebrennikov via cfe-commits 


@@ -1058,8 +1061,10 @@ namespace cwg62 { // cwg62: 2.9
 // cxx98-error@-1 {{template argument uses local type }}
 get();
 // cxx98-error@-1 {{template argument uses local type }}
+// cxx98-note@-2 {{while substituting explicitly-specified template 
arguments}}

Endilll wrote:

```suggestion
//   cxx98-note@-2 {{while substituting explicitly-specified template 
arguments}}
```

https://github.com/llvm/llvm-project/pull/125453
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] print correct context for diagnostics suppressed by deduction (PR #125453)

2025-02-02 Thread Richard Smith via cfe-commits 

zygoloid wrote:

Thank you for tackling this longstanding issue! How much does saving this extra 
state add to the runtime and memory usage on a template-heavy compilation?

If the cost is concerning, one other option we could consider here would be 
performing pending local instantiations eagerly when we reach the end of a 
region in which we have a context note -- that'd mean we don't need to save 
state for later. That's subtly behavior changing because it will pick an 
earlier point of instantiation for those local instantiations, but it's a 
permitted point of instantiation; we could try it and see if it works well 
enough in practice. We could also delay implicit definitions of special members 
like we do for template instantiations to reduce the impact.

https://github.com/llvm/llvm-project/pull/125453
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] print correct context for diagnostics suppressed by deduction (PR #125453)

2025-02-02 Thread via cfe-commits 


@@ -1909,7 +1909,19 @@ class Sema final : public SemaBase {
   /// '\#pragma clang attribute push' directives to the given declaration.
   void AddPragmaAttributes(Scope *S, Decl *D);
 
-  void PrintPragmaAttributeInstantiationPoint();
+  using DiagFuncRef =

cor3ntin wrote:

I would like a more specific name, like `InstantiationContextDiagCallback` or 
something along these lines

https://github.com/llvm/llvm-project/pull/125453
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] print correct context for diagnostics suppressed by deduction (PR #125453)

2025-02-02 Thread via cfe-commits 


@@ -1654,11 +1654,20 @@ void Sema::EmitDiagnostic(unsigned DiagID, const 
DiagnosticBuilder &DB) {
 }
 
 case DiagnosticIDs::SFINAE_Suppress:
+  if (DiagnosticsEngine::Level Level = getDiagnostics().getDiagnosticLevel(
+  DiagInfo.getID(), DiagInfo.getLocation());
+  Level == DiagnosticsEngine::Ignored)
+return;
   // Make a copy of this suppressed diagnostic and store it with the
   // template-deduction information;
   if (*Info) {
-(*Info)->addSuppressedDiagnostic(DiagInfo.getLocation(),
-   PartialDiagnostic(DiagInfo, 
Context.getDiagAllocator()));
+(*Info)->addSuppressedDiagnostic(
+DiagInfo.getLocation(),
+PartialDiagnostic(DiagInfo, Context.getDiagAllocator()));
+if (!Diags.getDiagnosticIDs()->isNote(DiagID))
+  PrintContextStack([Info](SourceLocation Loc, PartialDiagnostic PD) {
+(*Info)->addSuppressedDiagnostic(Loc, std::move(PD));
+  });

cor3ntin wrote:

Did you consider doing that in PrintInstantiationStack directly?
ie just modifying `PrintInstantiationStack`'s body such that if we are in a 
sfinae context we call addSuppressedDiagnostic on that context there, and not 
have to deal with callbacks outside of this function?

https://github.com/llvm/llvm-project/pull/125453
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] print correct context for diagnostics suppressed by deduction (PR #125453)

2025-02-02 Thread via cfe-commits 


@@ -1909,7 +1909,19 @@ class Sema final : public SemaBase {
   /// '\#pragma clang attribute push' directives to the given declaration.
   void AddPragmaAttributes(Scope *S, Decl *D);
 
-  void PrintPragmaAttributeInstantiationPoint();
+  using DiagFuncRef =
+  llvm::function_ref;
+  auto getDefaultDiagFunc() {
+return [this](SourceLocation Loc, PartialDiagnostic PD) {
+  DiagnosticBuilder Builder(Diags.Report(Loc, PD.getDiagID()));
+  PD.Emit(Builder);
+};
+  }

cor3ntin wrote:

I think we could / should put that into SemaTemplateInstantiate - and have the 
function taking a  DiagFuncRef have that default parameter defaulted to `{}`

https://github.com/llvm/llvm-project/pull/125453
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] print correct context for diagnostics suppressed by deduction (PR #125453)

2025-02-02 Thread Matheus Izvekov via cfe-commits 

https://github.com/mizvekov created 
https://github.com/llvm/llvm-project/pull/125453

This patch makes it so the correct instantiation context is printed for 
diagnostics suppessed by template argument deduction.

The context is saved along with the suppressed diagnostic, and when the 
declaration they were attached to becomes used, we print the correct context, 
instead of whatever context was at this point.

>From f6ea38d9ab756eaf85b5943a12056f4f534d4ca0 Mon Sep 17 00:00:00 2001
From: Matheus Izvekov 
Date: Sun, 2 Feb 2025 23:47:15 -0300
Subject: [PATCH] [clang] print correct context for diagnostics suppressed by
 deduction

This patch makes it so the correct instantiation context is printed
for diagnostics suppessed by template argument deduction.

The context is saved along with the suppressed diagnostic, and
when the declaration they were attached to becomes used, we print
the correct context, instead of whatever context was at this point.
---
 clang/docs/ReleaseNotes.rst   |   3 +
 clang/include/clang/Sema/Sema.h   |  26 +-
 clang/lib/Sema/Sema.cpp   |  13 +-
 clang/lib/Sema/SemaAttr.cpp   |   6 +-
 clang/lib/Sema/SemaExpr.cpp   |   7 +-
 clang/lib/Sema/SemaTemplateInstantiate.cpp| 271 +-
 clang/test/CXX/drs/cwg0xx.cpp |   5 +
 clang/test/CXX/drs/cwg4xx.cpp |   3 +-
 .../CXX/temp/temp.arg/temp.arg.type/p2.cpp|   9 +-
 clang/test/SemaCXX/anonymous-struct.cpp   |   5 +-
 clang/test/SemaCXX/bool-increment-SFINAE.cpp  |   4 +-
 clang/test/SemaCXX/cxx98-compat-flags.cpp |   2 +
 clang/test/SemaCXX/cxx98-compat.cpp   |   2 +
 clang/test/SemaCXX/deprecated.cpp |   4 +-
 clang/test/SemaCXX/lambda-expressions.cpp |   2 +
 clang/test/SemaCXX/undefined-internal.cpp |   6 +-
 clang/test/SemaTemplate/recovery-crash.cpp|   1 +
 clang/test/SemaTemplate/temp_arg_nontype.cpp  |   1 +
 18 files changed, 209 insertions(+), 161 deletions(-)

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 8be1ea2fb01455..70c3b062d97717 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -128,6 +128,9 @@ Bug Fixes to Attribute Support
 Bug Fixes to C++ Support
 
 
+- Clang now prints the correct instantiation context for diagnostics suppressed
+  by template argument deduction.
+
 Bug Fixes to AST Handling
 ^
 
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index 472a0e25adc975..7d01dc1aa4c00b 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -1909,7 +1909,19 @@ class Sema final : public SemaBase {
   /// '\#pragma clang attribute push' directives to the given declaration.
   void AddPragmaAttributes(Scope *S, Decl *D);
 
-  void PrintPragmaAttributeInstantiationPoint();
+  using DiagFuncRef =
+  llvm::function_ref;
+  auto getDefaultDiagFunc() {
+return [this](SourceLocation Loc, PartialDiagnostic PD) {
+  DiagnosticBuilder Builder(Diags.Report(Loc, PD.getDiagID()));
+  PD.Emit(Builder);
+};
+  }
+
+  void PrintPragmaAttributeInstantiationPoint(DiagFuncRef DiagFunc);
+  void PrintPragmaAttributeInstantiationPoint() {
+PrintPragmaAttributeInstantiationPoint(getDefaultDiagFunc());
+  }
 
   void DiagnoseUnterminatedPragmaAttribute();
 
@@ -13260,18 +13272,22 @@ class Sema final : public SemaBase {
   void pushCodeSynthesisContext(CodeSynthesisContext Ctx);
   void popCodeSynthesisContext();
 
-  void PrintContextStack() {
+  void PrintContextStack(DiagFuncRef DiagFunc) {
 if (!CodeSynthesisContexts.empty() &&
 CodeSynthesisContexts.size() != LastEmittedCodeSynthesisContextDepth) {
-  PrintInstantiationStack();
+  PrintInstantiationStack(DiagFunc);
   LastEmittedCodeSynthesisContextDepth = CodeSynthesisContexts.size();
 }
 if (PragmaAttributeCurrentTargetDecl)
-  PrintPragmaAttributeInstantiationPoint();
+  PrintPragmaAttributeInstantiationPoint(DiagFunc);
   }
+  void PrintContextStack() { PrintContextStack(getDefaultDiagFunc()); }
   /// Prints the current instantiation stack through a series of
   /// notes.
-  void PrintInstantiationStack();
+  void PrintInstantiationStack(DiagFuncRef DiagFunc);
+  void PrintInstantiationStack() {
+PrintInstantiationStack(getDefaultDiagFunc());
+  }
 
   /// Determines whether we are currently in a context where
   /// template argument substitution failures are not considered
diff --git a/clang/lib/Sema/Sema.cpp b/clang/lib/Sema/Sema.cpp
index 9507d7602aa401..33e2bd1e030513 100644
--- a/clang/lib/Sema/Sema.cpp
+++ b/clang/lib/Sema/Sema.cpp
@@ -1654,11 +1654,20 @@ void Sema::EmitDiagnostic(unsigned DiagID, const 
DiagnosticBuilder &DB) {
 }
 
 case DiagnosticIDs::SFINAE_Suppress:
+  if (DiagnosticsEngine::Level Level = getDiagnostics().getDiagnosticLevel(
+  DiagInfo.ge

[clang] [clang] print correct context for diagnostics suppressed by deduction (PR #125453)

2025-02-02 Thread via cfe-commits 

llvmbot wrote:




@llvm/pr-subscribers-clang

Author: Matheus Izvekov (mizvekov)


Changes

This patch makes it so the correct instantiation context is printed for 
diagnostics suppessed by template argument deduction.

The context is saved along with the suppressed diagnostic, and when the 
declaration they were attached to becomes used, we print the correct context, 
instead of whatever context was at this point.

---

Patch is 38.76 KiB, truncated to 20.00 KiB below, full version: 
https://github.com/llvm/llvm-project/pull/125453.diff


18 Files Affected:

- (modified) clang/docs/ReleaseNotes.rst (+3) 
- (modified) clang/include/clang/Sema/Sema.h (+21-5) 
- (modified) clang/lib/Sema/Sema.cpp (+11-2) 
- (modified) clang/lib/Sema/SemaAttr.cpp (+3-3) 
- (modified) clang/lib/Sema/SemaExpr.cpp (+4-3) 
- (modified) clang/lib/Sema/SemaTemplateInstantiate.cpp (+136-135) 
- (modified) clang/test/CXX/drs/cwg0xx.cpp (+5) 
- (modified) clang/test/CXX/drs/cwg4xx.cpp (+2-1) 
- (modified) clang/test/CXX/temp/temp.arg/temp.arg.type/p2.cpp (+6-3) 
- (modified) clang/test/SemaCXX/anonymous-struct.cpp (+2-3) 
- (modified) clang/test/SemaCXX/bool-increment-SFINAE.cpp (+2-2) 
- (modified) clang/test/SemaCXX/cxx98-compat-flags.cpp (+2) 
- (modified) clang/test/SemaCXX/cxx98-compat.cpp (+2) 
- (modified) clang/test/SemaCXX/deprecated.cpp (+2-2) 
- (modified) clang/test/SemaCXX/lambda-expressions.cpp (+2) 
- (modified) clang/test/SemaCXX/undefined-internal.cpp (+4-2) 
- (modified) clang/test/SemaTemplate/recovery-crash.cpp (+1) 
- (modified) clang/test/SemaTemplate/temp_arg_nontype.cpp (+1) 


``diff
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 8be1ea2fb01455..70c3b062d97717 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -128,6 +128,9 @@ Bug Fixes to Attribute Support
 Bug Fixes to C++ Support
 
 
+- Clang now prints the correct instantiation context for diagnostics suppressed
+  by template argument deduction.
+
 Bug Fixes to AST Handling
 ^
 
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index 472a0e25adc975..7d01dc1aa4c00b 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -1909,7 +1909,19 @@ class Sema final : public SemaBase {
   /// '\#pragma clang attribute push' directives to the given declaration.
   void AddPragmaAttributes(Scope *S, Decl *D);
 
-  void PrintPragmaAttributeInstantiationPoint();
+  using DiagFuncRef =
+  llvm::function_ref;
+  auto getDefaultDiagFunc() {
+return [this](SourceLocation Loc, PartialDiagnostic PD) {
+  DiagnosticBuilder Builder(Diags.Report(Loc, PD.getDiagID()));
+  PD.Emit(Builder);
+};
+  }
+
+  void PrintPragmaAttributeInstantiationPoint(DiagFuncRef DiagFunc);
+  void PrintPragmaAttributeInstantiationPoint() {
+PrintPragmaAttributeInstantiationPoint(getDefaultDiagFunc());
+  }
 
   void DiagnoseUnterminatedPragmaAttribute();
 
@@ -13260,18 +13272,22 @@ class Sema final : public SemaBase {
   void pushCodeSynthesisContext(CodeSynthesisContext Ctx);
   void popCodeSynthesisContext();
 
-  void PrintContextStack() {
+  void PrintContextStack(DiagFuncRef DiagFunc) {
 if (!CodeSynthesisContexts.empty() &&
 CodeSynthesisContexts.size() != LastEmittedCodeSynthesisContextDepth) {
-  PrintInstantiationStack();
+  PrintInstantiationStack(DiagFunc);
   LastEmittedCodeSynthesisContextDepth = CodeSynthesisContexts.size();
 }
 if (PragmaAttributeCurrentTargetDecl)
-  PrintPragmaAttributeInstantiationPoint();
+  PrintPragmaAttributeInstantiationPoint(DiagFunc);
   }
+  void PrintContextStack() { PrintContextStack(getDefaultDiagFunc()); }
   /// Prints the current instantiation stack through a series of
   /// notes.
-  void PrintInstantiationStack();
+  void PrintInstantiationStack(DiagFuncRef DiagFunc);
+  void PrintInstantiationStack() {
+PrintInstantiationStack(getDefaultDiagFunc());
+  }
 
   /// Determines whether we are currently in a context where
   /// template argument substitution failures are not considered
diff --git a/clang/lib/Sema/Sema.cpp b/clang/lib/Sema/Sema.cpp
index 9507d7602aa401..33e2bd1e030513 100644
--- a/clang/lib/Sema/Sema.cpp
+++ b/clang/lib/Sema/Sema.cpp
@@ -1654,11 +1654,20 @@ void Sema::EmitDiagnostic(unsigned DiagID, const 
DiagnosticBuilder &DB) {
 }
 
 case DiagnosticIDs::SFINAE_Suppress:
+  if (DiagnosticsEngine::Level Level = getDiagnostics().getDiagnosticLevel(
+  DiagInfo.getID(), DiagInfo.getLocation());
+  Level == DiagnosticsEngine::Ignored)
+return;
   // Make a copy of this suppressed diagnostic and store it with the
   // template-deduction information;
   if (*Info) {
-(*Info)->addSuppressedDiagnostic(DiagInfo.getLocation(),
-   PartialDiagnostic(DiagInfo, 
Context.getDiagAllocator()));
+(*Info)->addSuppressedDiagnost