Fix assertion failure during conversion function overload resolution.

[katzdm]

When clang is built with assertions, an otherwise silent (and seemingly innocuous) assertion failure from SemaConcept.cpp is triggered by the following program:

struct S {
  operator int();
  template <typename T> operator T();
};

constexpr auto r = &S::operator int;

The function in question compares the "constrained-ness" of S::operator int and S::operator T<int>; the template kind of the former is TK_NonTemplate, whereas the template kind of the later is TK_FunctionTemplateSpecialization. The later kind is not "expected" by the function, thus the assertion-failure.

[llvmbot]

@llvm/pr-subscribers-clang

Author: Daniel M. Katz (katzdm)

Changes

When clang is built with assertions, an otherwise silent (and seemingly innocuous) assertion failure from SemaConcept.cpp is triggered by the following program:

struct S {
  operator int();
  template <typename T> operator T();
};

constexpr auto r = &S::operator int;

The function in question compares the "constrained-ness" of S::operator int and S::operator T<int>; the template kind of the former is TK_NonTemplate, whereas the template kind of the later is TK_FunctionTemplateSpecialization. The later kind is not "expected" by the function, thus the assertion-failure.

---

Full diff: https://github.com/llvm/llvm-project/pull/98671.diff

2 Files Affected:

• (modified) clang/lib/Sema/SemaConcept.cpp (+2-1)
• (added) clang/test/SemaCXX/PR98671.cpp (+14)

diff --git a/clang/lib/Sema/SemaConcept.cpp b/clang/lib/Sema/SemaConcept.cpp
index 202dd86c67f62..f94fb8be20e07 100644
--- a/clang/lib/Sema/SemaConcept.cpp
+++ b/clang/lib/Sema/SemaConcept.cpp
@@ -1519,7 +1519,8 @@ bool Sema::IsAtLeastAsConstrained(NamedDecl *D1,
     auto IsExpectedEntity = [](const FunctionDecl *FD) {
       FunctionDecl::TemplatedKind Kind = FD->getTemplatedKind();
       return Kind == FunctionDecl::TK_NonTemplate ||
-             Kind == FunctionDecl::TK_FunctionTemplate;
+             Kind == FunctionDecl::TK_FunctionTemplate ||
+             Kind == FunctionDecl::TK_FunctionTemplateSpecialization;
     };
     const auto *FD2 = dyn_cast<FunctionDecl>(D2);
     (void)IsExpectedEntity;
diff --git a/clang/test/SemaCXX/PR98671.cpp b/clang/test/SemaCXX/PR98671.cpp
new file mode 100644
index 0000000000000..696b750759854
--- /dev/null
+++ b/clang/test/SemaCXX/PR98671.cpp
@@ -0,0 +1,14 @@
+// RUN: %clang_cc1 -std=c++20 -fsyntax-only %s -verify
+
+struct S {
+  operator int();
+
+  template <typename T>
+  operator T();
+};
+
+
+// Ensure that no assertion is raised when overload resolution fails while
+// choosing between an operator function template and an operator function.
+constexpr auto r = &S::operator int;
+// expected-error@-1 {{initializer of type '<overloaded function type>'}}

[cor3ntin]

I think this looks good, thanks

Adding @AaronBallman @mizvekov for additional pairs of eyes

[AaronBallman]

LGTM!

[mizvekov]

LGTM as well

[katzdm]

Looking further into this, I think this is a narrow problem that arises when selecting a function from an overload set which includes a specialization of a conversion function template.

Consider a class:

struct S {
  template <typename> void fn();
  template <typename T> operator T();
};

When looking up &S::fn<int>, the associated LookupResult will be a singleton containing the FunctionTemplateDecl for fn, not the function template specialization fn<int>. The AddressOfFunctionResolver in SemaOverload.cpp will then call Sema::ResolveSingleFunctionTemplateSpecialization, which will use template argument deduction machinery to instantiate fn<int>.

Looking up &S::operator int, however, is something stranger: Because there are no apparent template-arguments, the specialization is resolved during lookup instead of afterwards. This is necessary because the information needed to select the specialization is encoded in the unqualified-id itself, rather than in names and expressions that are parsed thereafter. From what I've been able to tell, this situation seems unique to conversion-function-templates, and I haven't found other cases where the LookupResult directly contains a function template specialization.

From here, deducing the placeholder type for auto r = &S::operator int; uses Sema::resolveAddressOfSingleOverloadCandidate to try to narrow to a single candidate. In the presence of constraints, this calls Sema::getMoreConstrainedFunction to select a "best viable function."

The implementation of Sema::getMoreConstrainedFunction already attempts to map instantiated member-functions back to their "pattern declarations" (which will have type TK_FunctionTemplate) before calling Sema::IsAtLeastAsConstrained. This handles cases like:

template <typename>
struct S {
  void fn();
};

but not our conversion-function-template. Since I believe that specializations of conversion-function-templates are the only specializations that can appear in the LookupResult, I suggest a narrow carve-out to handle this case, and an assertion to alert us if there are any other such cases which we aren't aware of. All tests seem to pass.

[AaronBallman]

CC @zygoloid for a potential additional set of eyes

[zygoloid]

Hmm. See https://lists.isocpp.org/core/2024/06/15952.php -- I think we should never be comparing constraints between functions whose signatures aren't otherwise the same, and in particular we should never compare the constraints of a conversion function template against the constraints of a conversion function.

Conversion functions do add another wrinkle here, because we can find them in different scopes too. For example:

template<typename T> concept C = true;
template<typename T> concept D = C<T> && true;

template<typename T>
struct A {
  operator T() requires C<T>;
};

template<typename U>
struct B {
  operator U() requires D<U>;
};

struct X : A<int>, B<int> {};

int n = X();
auto p = &X::operator int;

It doesn't make sense to compare the constraints of the conversion functions of the two templates here, because T simply has nothing to do with U. (Today, Clang and MSVC report ambiguity for both n and p; GCC and EDG accept n but reject p.)

That said... an example like the following sidesteps all these issues and still crashes clang, so I think the above comments are somewhat orthogonal to this patch (except that I think we should have something like this as a test case instead of the current test case):

template<typename T> concept C = true;
template<typename T> concept D = C<T> && true;

struct X {
  template<C T> operator T();
  template<D T> operator T();
};

auto p = &X::operator int;

[mizvekov]

That said... an example like the following sidesteps all these issues and still crashes clang, so I think the above comments are somewhat orthogonal to this patch (except that I think we should have something like this as a test case instead of the current test case):

template<typename T> concept C = true;
template<typename T> concept D = C<T> && true;

struct X {
  template<C T> operator T();
  template<D T> operator T();
};

auto p = &X::operator int;

FYI that code should be affected by #18291
I am currently working on a fix for that issue, since it affects CWG2398 stuffs as well.

[katzdm]

@zygoloid Friendly ping here, if you have a chance to take another look.

[katzdm]

@AaronBallman @cor3ntin Would you mind taking another look?

[AaronBallman]

LGTM, but please add a release note so users know about the fix.

[katzdm]

LGTM, but please add a release note so users know about the fix.

@AaronBallman Appreciate the reminder! Done.

[AaronBallman]

LGTM, but please add a release note so users know about the fix.

@AaronBallman Appreciate the reminder! Done.

Thanks! Do you need someone to land the changes?

[katzdm]

LGTM, but please add a release note so users know about the fix.

@AaronBallman Appreciate the reminder! Done.

Thanks! Do you need someone to land the changes?

Yep, I don't have commit privilege - would appreciate the help :)