[PATCH] D121646: [Concepts] Fix an assertion failure while diagnosing constrained function candidates

[Erich Keane via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGbc7034c1ecfb: [Concepts] Fix an assertion failure while 
diagnosing constrained (authored by royjacobson, committed by erichkeane).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121646

Files:
  clang/lib/Sema/SemaOverload.cpp
  clang/test/SemaTemplate/concepts.cpp


Index: clang/test/SemaTemplate/concepts.cpp
===
--- clang/test/SemaTemplate/concepts.cpp
+++ clang/test/SemaTemplate/concepts.cpp
@@ -208,3 +208,17 @@
 return (int*)nullptr; // FIXME: should error
   }
 }
+
+namespace PR54379 {
+template 
+struct A {
+  static void f() requires (N == 0) { return; } // expected-note {{candidate 
template ignored: constraints not satisfied}} expected-note {{evaluated to 
false}}
+  static void f() requires (N == 1) { return; } // expected-note {{candidate 
template ignored: constraints not satisfied}} expected-note {{evaluated to 
false}}
+};
+void (*f1)() = A<2>::f; // expected-error {{address of overloaded function 'f' 
does not match required type}}
+
+struct B {
+  template  static void f() requires (N2 == 0) { return; }  // 
expected-note {{candidate template ignored: constraints not satisfied [with N2 
= 1]}} expected-note {{evaluated to false}}
+};
+void (*f2)() = B::f; // expected-error {{address of overloaded function 'f' 
does not match required type}}
+}
Index: clang/lib/Sema/SemaOverload.cpp
===
--- clang/lib/Sema/SemaOverload.cpp
+++ clang/lib/Sema/SemaOverload.cpp
@@ -10256,10 +10256,19 @@
   return false;
 if (!Satisfaction.IsSatisfied) {
   if (Complain) {
-if (InOverloadResolution)
+if (InOverloadResolution) {
+  SmallString<128> TemplateArgString;
+  if (FunctionTemplateDecl *FunTmpl = FD->getPrimaryTemplate()) {
+TemplateArgString += " ";
+TemplateArgString += S.getTemplateArgumentBindingsText(
+FunTmpl->getTemplateParameters(),
+*FD->getTemplateSpecializationArgs());
+  }
+
   S.Diag(FD->getBeginLoc(),
- diag::note_ovl_candidate_unsatisfied_constraints);
-else
+ diag::note_ovl_candidate_unsatisfied_constraints)
+  << TemplateArgString;
+} else
   S.Diag(Loc, diag::err_addrof_function_constraints_not_satisfied)
   << FD;
 S.DiagnoseUnsatisfiedConstraint(Satisfaction);


Index: clang/test/SemaTemplate/concepts.cpp
===
--- clang/test/SemaTemplate/concepts.cpp
+++ clang/test/SemaTemplate/concepts.cpp
@@ -208,3 +208,17 @@
 return (int*)nullptr; // FIXME: should error
   }
 }
+
+namespace PR54379 {
+template 
+struct A {
+  static void f() requires (N == 0) { return; } // expected-note {{candidate template ignored: constraints not satisfied}} expected-note {{evaluated to false}}
+  static void f() requires (N == 1) { return; } // expected-note {{candidate template ignored: constraints not satisfied}} expected-note {{evaluated to false}}
+};
+void (*f1)() = A<2>::f; // expected-error {{address of overloaded function 'f' does not match required type}}
+
+struct B {
+  template  static void f() requires (N2 == 0) { return; }  // expected-note {{candidate template ignored: constraints not satisfied [with N2 = 1]}} expected-note {{evaluated to false}}
+};
+void (*f2)() = B::f; // expected-error {{address of overloaded function 'f' does not match required type}}
+}
Index: clang/lib/Sema/SemaOverload.cpp
===
--- clang/lib/Sema/SemaOverload.cpp
+++ clang/lib/Sema/SemaOverload.cpp
@@ -10256,10 +10256,19 @@
   return false;
 if (!Satisfaction.IsSatisfied) {
   if (Complain) {
-if (InOverloadResolution)
+if (InOverloadResolution) {
+  SmallString<128> TemplateArgString;
+  if (FunctionTemplateDecl *FunTmpl = FD->getPrimaryTemplate()) {
+TemplateArgString += " ";
+TemplateArgString += S.getTemplateArgumentBindingsText(
+FunTmpl->getTemplateParameters(),
+*FD->getTemplateSpecializationArgs());
+  }
+
   S.Diag(FD->getBeginLoc(),
- diag::note_ovl_candidate_unsatisfied_constraints);
-else
+ diag::note_ovl_candidate_unsatisfied_constraints)
+  << TemplateArgString;
+} else
   S.Diag(Loc, diag::err_addrof_function_constraints_not_satisfied)
   << FD;
 S.DiagnoseUnsatisfiedConstraint(Satisfaction);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D121646: [Concepts] Fix an assertion failure while diagnosing constrained function candidates

[Roy Jacobson via Phabricator via cfe-commits]

royjacobson added a comment.

In D121646#3382243 , @erichkeane 
wrote:

> In D121646#3380902 , @royjacobson 
> wrote:
>
>> In D121646#3380893 , @erichkeane 
>> wrote:
>>
>>> This seems acceptable to me. Though, I wonder if 
>>> `getTemplateArgumentBindingsText` should produce the leading space, that 
>>> way we could just pass the result of it directly, rather than have to 
>>> create a SmallString everywhere.  WDYT?
>>
>> There are other places (outside SemaOverload) that use this function and 
>> don't need a space, though.
>> I thought about just putting the space in the diagnostic itself, but then 
>> sometimes you don't get template arguments and it fails.
>
> I see.  I was hoping that wasn't the case!  OK, LGTM.  Let me know if you 
> need me to commit this for you.

Yes, thank you :)
(with author as "Roy Jacobson ")


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121646

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D121646: [Concepts] Fix an assertion failure while diagnosing constrained function candidates

[Erich Keane via Phabricator via cfe-commits]

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

In D121646#3380902 , @royjacobson 
wrote:

> In D121646#3380893 , @erichkeane 
> wrote:
>
>> This seems acceptable to me. Though, I wonder if 
>> `getTemplateArgumentBindingsText` should produce the leading space, that way 
>> we could just pass the result of it directly, rather than have to create a 
>> SmallString everywhere.  WDYT?
>
> There are other places (outside SemaOverload) that use this function and 
> don't need a space, though.
> I thought about just putting the space in the diagnostic itself, but then 
> sometimes you don't get template arguments and it fails.

I see.  I was hoping that wasn't the case!  OK, LGTM.  Let me know if you need 
me to commit this for you.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121646

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D121646: [Concepts] Fix an assertion failure while diagnosing constrained function candidates

[Roy Jacobson via Phabricator via cfe-commits]

royjacobson updated this revision to Diff 415251.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121646

Files:
  clang/lib/Sema/SemaOverload.cpp
  clang/test/SemaTemplate/concepts.cpp


Index: clang/test/SemaTemplate/concepts.cpp
===
--- clang/test/SemaTemplate/concepts.cpp
+++ clang/test/SemaTemplate/concepts.cpp
@@ -208,3 +208,17 @@
 return (int*)nullptr; // FIXME: should error
   }
 }
+
+namespace PR54379 {
+template 
+struct A {
+  static void f() requires (N == 0) { return; } // expected-note {{candidate 
template ignored: constraints not satisfied}} expected-note {{evaluated to 
false}}
+  static void f() requires (N == 1) { return; } // expected-note {{candidate 
template ignored: constraints not satisfied}} expected-note {{evaluated to 
false}}
+};
+void (*f1)() = A<2>::f; // expected-error {{address of overloaded function 'f' 
does not match required type}}
+
+struct B {
+  template  static void f() requires (N2 == 0) { return; }  // 
expected-note {{candidate template ignored: constraints not satisfied [with N2 
= 1]}} expected-note {{evaluated to false}}
+};
+void (*f2)() = B::f; // expected-error {{address of overloaded function 'f' 
does not match required type}}
+}
Index: clang/lib/Sema/SemaOverload.cpp
===
--- clang/lib/Sema/SemaOverload.cpp
+++ clang/lib/Sema/SemaOverload.cpp
@@ -10256,10 +10256,19 @@
   return false;
 if (!Satisfaction.IsSatisfied) {
   if (Complain) {
-if (InOverloadResolution)
+if (InOverloadResolution) {
+  SmallString<128> TemplateArgString;
+  if (FunctionTemplateDecl *FunTmpl = FD->getPrimaryTemplate()) {
+TemplateArgString += " ";
+TemplateArgString += S.getTemplateArgumentBindingsText(
+FunTmpl->getTemplateParameters(),
+*FD->getTemplateSpecializationArgs());
+  }
+
   S.Diag(FD->getBeginLoc(),
- diag::note_ovl_candidate_unsatisfied_constraints);
-else
+ diag::note_ovl_candidate_unsatisfied_constraints)
+  << TemplateArgString;
+} else
   S.Diag(Loc, diag::err_addrof_function_constraints_not_satisfied)
   << FD;
 S.DiagnoseUnsatisfiedConstraint(Satisfaction);


Index: clang/test/SemaTemplate/concepts.cpp
===
--- clang/test/SemaTemplate/concepts.cpp
+++ clang/test/SemaTemplate/concepts.cpp
@@ -208,3 +208,17 @@
 return (int*)nullptr; // FIXME: should error
   }
 }
+
+namespace PR54379 {
+template 
+struct A {
+  static void f() requires (N == 0) { return; } // expected-note {{candidate template ignored: constraints not satisfied}} expected-note {{evaluated to false}}
+  static void f() requires (N == 1) { return; } // expected-note {{candidate template ignored: constraints not satisfied}} expected-note {{evaluated to false}}
+};
+void (*f1)() = A<2>::f; // expected-error {{address of overloaded function 'f' does not match required type}}
+
+struct B {
+  template  static void f() requires (N2 == 0) { return; }  // expected-note {{candidate template ignored: constraints not satisfied [with N2 = 1]}} expected-note {{evaluated to false}}
+};
+void (*f2)() = B::f; // expected-error {{address of overloaded function 'f' does not match required type}}
+}
Index: clang/lib/Sema/SemaOverload.cpp
===
--- clang/lib/Sema/SemaOverload.cpp
+++ clang/lib/Sema/SemaOverload.cpp
@@ -10256,10 +10256,19 @@
   return false;
 if (!Satisfaction.IsSatisfied) {
   if (Complain) {
-if (InOverloadResolution)
+if (InOverloadResolution) {
+  SmallString<128> TemplateArgString;
+  if (FunctionTemplateDecl *FunTmpl = FD->getPrimaryTemplate()) {
+TemplateArgString += " ";
+TemplateArgString += S.getTemplateArgumentBindingsText(
+FunTmpl->getTemplateParameters(),
+*FD->getTemplateSpecializationArgs());
+  }
+
   S.Diag(FD->getBeginLoc(),
- diag::note_ovl_candidate_unsatisfied_constraints);
-else
+ diag::note_ovl_candidate_unsatisfied_constraints)
+  << TemplateArgString;
+} else
   S.Diag(Loc, diag::err_addrof_function_constraints_not_satisfied)
   << FD;
 S.DiagnoseUnsatisfiedConstraint(Satisfaction);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D121646: [Concepts] Fix an assertion failure while diagnosing constrained function candidates

[Roy Jacobson via Phabricator via cfe-commits]

royjacobson added a comment.

In D121646#3380893 , @erichkeane 
wrote:

> This seems acceptable to me. Though, I wonder if 
> `getTemplateArgumentBindingsText` should produce the leading space, that way 
> we could just pass the result of it directly, rather than have to create a 
> SmallString everywhere.  WDYT?

There are other places (outside SemaOverload) that use this function and don't 
need a space, though.
I thought about just putting the space in the diagnostic itself, but then 
sometimes you don't get template arguments and it fails.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121646

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D121646: [Concepts] Fix an assertion failure while diagnosing constrained function candidates

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

This seems acceptable to me. Though, I wonder if 
`getTemplateArgumentBindingsText` should produce the leading space, that way we 
could just pass the result of it directly, rather than have to create a 
SmallString everywhere.  WDYT?




Comment at: clang/lib/Sema/SemaOverload.cpp:10261
+  SmallString<128> TemplateArgString;
+  TemplateArgString.clear();
+  if (FunctionTemplateDecl *FunTmpl = FD->getPrimaryTemplate()) {

.clear shouldn't be required as you just constructed it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121646

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D121646: [Concepts] Fix an assertion failure while diagnosing constrained function candidates

[Roy Jacobson via Phabricator via cfe-commits]

royjacobson created this revision.
royjacobson added a reviewer: erichkeane.
Herald added a project: All.
royjacobson requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

See: https://github.com/llvm/llvm-project/issues/54379

I tried to see if I can reuse `ResolveAddressOfOverloadedFunction` for explicit 
function instantiation and so I managed to hit this ICE.

Bug was the diagnostic required an argument (%0) and specific code path didn't 
pass an argument.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D121646

Files:
  clang/lib/Sema/SemaOverload.cpp
  clang/test/SemaTemplate/concepts.cpp


Index: clang/test/SemaTemplate/concepts.cpp
===
--- clang/test/SemaTemplate/concepts.cpp
+++ clang/test/SemaTemplate/concepts.cpp
@@ -208,3 +208,17 @@
 return (int*)nullptr; // FIXME: should error
   }
 }
+
+namespace PR54379 {
+template 
+struct A {
+  static void f() requires (N == 0) { return; } // expected-note {{candidate 
template ignored: constraints not satisfied}} expected-note {{evaluated to 
false}}
+  static void f() requires (N == 1) { return; } // expected-note {{candidate 
template ignored: constraints not satisfied}} expected-note {{evaluated to 
false}}
+};
+void (*f1)() = A<2>::f; // expected-error {{address of overloaded function 'f' 
does not match required type}}
+
+struct B {
+  template  static void f() requires (N2 == 0) { return; }  // 
expected-note {{candidate template ignored: constraints not satisfied [with N2 
= 1]}} expected-note {{evaluated to false}}
+};
+void (*f2)() = B::f; // expected-error {{address of overloaded function 'f' 
does not match required type}}
+}
Index: clang/lib/Sema/SemaOverload.cpp
===
--- clang/lib/Sema/SemaOverload.cpp
+++ clang/lib/Sema/SemaOverload.cpp
@@ -10256,10 +10256,19 @@
   return false;
 if (!Satisfaction.IsSatisfied) {
   if (Complain) {
-if (InOverloadResolution)
+if (InOverloadResolution) {
+  SmallString<128> TemplateArgString;
+  TemplateArgString.clear();
+  if (FunctionTemplateDecl *FunTmpl = FD->getPrimaryTemplate()) {
+TemplateArgString += " ";
+TemplateArgString += S.getTemplateArgumentBindingsText(
+  FunTmpl->getTemplateParameters(), 
*FD->getTemplateSpecializationArgs());
+  }
+
   S.Diag(FD->getBeginLoc(),
- diag::note_ovl_candidate_unsatisfied_constraints);
-else
+ diag::note_ovl_candidate_unsatisfied_constraints)
+  << TemplateArgString;
+} else
   S.Diag(Loc, diag::err_addrof_function_constraints_not_satisfied)
   << FD;
 S.DiagnoseUnsatisfiedConstraint(Satisfaction);


Index: clang/test/SemaTemplate/concepts.cpp
===
--- clang/test/SemaTemplate/concepts.cpp
+++ clang/test/SemaTemplate/concepts.cpp
@@ -208,3 +208,17 @@
 return (int*)nullptr; // FIXME: should error
   }
 }
+
+namespace PR54379 {
+template 
+struct A {
+  static void f() requires (N == 0) { return; } // expected-note {{candidate template ignored: constraints not satisfied}} expected-note {{evaluated to false}}
+  static void f() requires (N == 1) { return; } // expected-note {{candidate template ignored: constraints not satisfied}} expected-note {{evaluated to false}}
+};
+void (*f1)() = A<2>::f; // expected-error {{address of overloaded function 'f' does not match required type}}
+
+struct B {
+  template  static void f() requires (N2 == 0) { return; }  // expected-note {{candidate template ignored: constraints not satisfied [with N2 = 1]}} expected-note {{evaluated to false}}
+};
+void (*f2)() = B::f; // expected-error {{address of overloaded function 'f' does not match required type}}
+}
Index: clang/lib/Sema/SemaOverload.cpp
===
--- clang/lib/Sema/SemaOverload.cpp
+++ clang/lib/Sema/SemaOverload.cpp
@@ -10256,10 +10256,19 @@
   return false;
 if (!Satisfaction.IsSatisfied) {
   if (Complain) {
-if (InOverloadResolution)
+if (InOverloadResolution) {
+  SmallString<128> TemplateArgString;
+  TemplateArgString.clear();
+  if (FunctionTemplateDecl *FunTmpl = FD->getPrimaryTemplate()) {
+TemplateArgString += " ";
+TemplateArgString += S.getTemplateArgumentBindingsText(
+  FunTmpl->getTemplateParameters(), *FD->getTemplateSpecializationArgs());
+  }
+
   S.Diag(FD->getBeginLoc(),
- diag::note_ovl_candidate_unsatisfied_constraints);
-else
+ diag::note_ovl_candidate_unsatisfied_constraints)
+  << TemplateArgString;
+} else
   S.Diag(Loc,