Skip to content

Commit

Permalink
[Clang][Sema] Explicit template arguments are not substituted into th…
Browse files Browse the repository at this point in the history
…e exception specification of a function (llvm#90760)

[temp.deduct.general] p6 states:
> At certain points in the template argument deduction process it is
necessary to take a function type that makes use of template parameters
and replace those template parameters with the corresponding template
arguments.
This is done at the beginning of template argument deduction when any
explicitly specified template arguments are substituted into the
function type, and again at the end of template argument deduction when
any template arguments that were deduced or obtained from default
arguments are substituted.

[temp.deduct.general] p7 goes on to say:
> The _deduction substitution loci_ are
> - the function type outside of the _noexcept-specifier_,
> - the explicit-specifier,
> - the template parameter declarations, and
> - the template argument list of a partial specialization
>
> The substitution occurs in all types and expressions that are used in
the deduction substitution loci. [...]

Consider the following:
```cpp
struct A
{
    static constexpr bool x = true;
};

template<typename T, typename U>
void f(T, U) noexcept(T::x); // #1

template<typename T, typename U>
void f(T, U*) noexcept(T::y); // llvm#2

template<>
void f<A>(A, int*) noexcept; // clang currently accepts, GCC and EDG reject
```

Currently, `Sema::SubstituteExplicitTemplateArguments` will substitute
into the _noexcept-specifier_ when deducing template arguments from a
function declaration or when deducing template arguments for taking the
address of a function template (and the substitution is treated as a
SFINAE context). In the above example, `#1` is selected as the primary
template because substitution of the explicit template arguments into
the _noexcept-specifier_ of `llvm#2` failed, which resulted in the candidate
being ignored.

This behavior is incorrect ([temp.deduct.general] note 4 says as much), and
this patch corrects it by deferring all substitution into the
_noexcept-specifier_ until it is instantiated.

As part of the necessary changes to make this patch work, the
instantiation of the exception specification of a function template
specialization when taking the address of a function template is changed
to only occur for the function selected by overload resolution per
[except.spec] p13.1 (as opposed to being instantiated for every candidate).
  • Loading branch information
sdkrystian authored May 8, 2024
1 parent 04d0a69 commit 77c5cea
Show file tree
Hide file tree
Showing 8 changed files with 75 additions and 51 deletions.
2 changes: 2 additions & 0 deletions clang/docs/ReleaseNotes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -691,6 +691,8 @@ Bug Fixes to C++ Support
- Fix an assertion failure when parsing an invalid members of an anonymous class. (#GH85447)
- Fixed a misuse of ``UnresolvedLookupExpr`` for ill-formed templated expressions. Fixes (#GH48673), (#GH63243)
and (#GH88832).
- Clang now defers all substitution into the exception specification of a function template specialization
until the noexcept-specifier is instantiated.

Bug Fixes to AST Handling
^^^^^^^^^^^^^^^^^^^^^^^^^
Expand Down
24 changes: 18 additions & 6 deletions clang/lib/Sema/SemaInit.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6576,12 +6576,12 @@ void InitializationSequence::InitializeFrom(Sema &S,

AddPassByIndirectCopyRestoreStep(DestType, ShouldCopy);
} else if (ICS.isBad()) {
DeclAccessPair dap;
if (isLibstdcxxPointerReturnFalseHack(S, Entity, Initializer)) {
if (isLibstdcxxPointerReturnFalseHack(S, Entity, Initializer))
AddZeroInitializationStep(Entity.getType());
} else if (Initializer->getType() == Context.OverloadTy &&
!S.ResolveAddressOfOverloadedFunction(Initializer, DestType,
false, dap))
else if (DeclAccessPair Found;
Initializer->getType() == Context.OverloadTy &&
!S.ResolveAddressOfOverloadedFunction(Initializer, DestType,
/*Complain=*/false, Found))
SetFailed(InitializationSequence::FK_AddressOfOverloadFailed);
else if (Initializer->getType()->isFunctionType() &&
isExprAnUnaddressableFunction(S, Initializer))
Expand Down Expand Up @@ -9641,6 +9641,8 @@ bool InitializationSequence::Diagnose(Sema &S,
if (!Failed())
return false;

QualType DestType = Entity.getType();

// When we want to diagnose only one element of a braced-init-list,
// we need to factor it out.
Expr *OnlyArg;
Expand All @@ -9650,11 +9652,21 @@ bool InitializationSequence::Diagnose(Sema &S,
OnlyArg = List->getInit(0);
else
OnlyArg = Args[0];

if (OnlyArg->getType() == S.Context.OverloadTy) {
DeclAccessPair Found;
if (FunctionDecl *FD = S.ResolveAddressOfOverloadedFunction(
OnlyArg, DestType.getNonReferenceType(), /*Complain=*/false,
Found)) {
if (Expr *Resolved =
S.FixOverloadedFunctionReference(OnlyArg, Found, FD).get())
OnlyArg = Resolved;
}
}
}
else
OnlyArg = nullptr;

QualType DestType = Entity.getType();
switch (Failure) {
case FK_TooManyInitsForReference:
// FIXME: Customize for the initialized entity?
Expand Down
32 changes: 3 additions & 29 deletions clang/lib/Sema/SemaTemplateDeduction.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1323,13 +1323,11 @@ bool Sema::isSameOrCompatibleFunctionType(QualType P, QualType A) {
return Context.hasSameType(P, A);

// Noreturn and noexcept adjustment.
QualType AdjustedParam;
if (IsFunctionConversion(P, A, AdjustedParam))
return Context.hasSameType(AdjustedParam, A);
if (QualType AdjustedParam; IsFunctionConversion(P, A, AdjustedParam))
P = AdjustedParam;

// FIXME: Compatible calling conventions.

return Context.hasSameType(P, A);
return Context.hasSameFunctionTypeIgnoringExceptionSpec(P, A);
}

/// Get the index of the first template parameter that was originally from the
Expand Down Expand Up @@ -3509,23 +3507,6 @@ TemplateDeductionResult Sema::SubstituteExplicitTemplateArguments(
if (FunctionType) {
auto EPI = Proto->getExtProtoInfo();
EPI.ExtParameterInfos = ExtParamInfos.getPointerOrNull(ParamTypes.size());

// In C++1z onwards, exception specifications are part of the function type,
// so substitution into the type must also substitute into the exception
// specification.
SmallVector<QualType, 4> ExceptionStorage;
if (getLangOpts().CPlusPlus17 &&
SubstExceptionSpec(
Function->getLocation(), EPI.ExceptionSpec, ExceptionStorage,
getTemplateInstantiationArgs(
FunctionTemplate, nullptr, /*Final=*/true,
/*Innermost=*/SugaredExplicitArgumentList->asArray(),
/*RelativeToPrimary=*/false,
/*Pattern=*/nullptr,
/*ForConstraintInstantiation=*/false,
/*SkipForSpecialization=*/true)))
return TemplateDeductionResult::SubstitutionFailure;

*FunctionType = BuildFunctionType(ResultType, ParamTypes,
Function->getLocation(),
Function->getDeclName(),
Expand Down Expand Up @@ -4705,13 +4686,6 @@ TemplateDeductionResult Sema::DeduceTemplateArguments(
Info.getLocation()))
return TemplateDeductionResult::MiscellaneousDeductionFailure;

auto *SpecializationFPT =
Specialization->getType()->castAs<FunctionProtoType>();
if (IsAddressOfFunction && getLangOpts().CPlusPlus17 &&
isUnresolvedExceptionSpec(SpecializationFPT->getExceptionSpecType()) &&
!ResolveExceptionSpec(Info.getLocation(), SpecializationFPT))
return TemplateDeductionResult::MiscellaneousDeductionFailure;

// Adjust the exception specification of the argument to match the
// substituted and resolved type we just formed. (Calling convention and
// noreturn can't be dependent, so we don't actually need this for them
Expand Down
13 changes: 5 additions & 8 deletions clang/test/CXX/drs/dr13xx.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -281,13 +281,10 @@ namespace cwg1330 { // cwg1330: 4 c++11
decltype(f<char>()) f2; // #cwg1330-f-char
bool f3 = noexcept(f<float>()); /// #cwg1330-f-float
#endif
// In C++17 onwards, substituting explicit template arguments into the
// function type substitutes into the exception specification (because it's
// part of the type). In earlier languages, we don't notice there's a problem
// until we've already started to instantiate.
template int f<short>(); // #cwg1330-f-short
// since-cxx17-error@-1 {{explicit instantiation of 'f' does not refer to a function template, variable template, member function, member class, or static data member}}
// since-cxx17-note@#cwg1330-f {{candidate template ignored: substitution failure [with T = short]: type 'short' cannot be used prior to '::' because it has no members}}
// since-cxx17-error@#cwg1330-f {{type 'short' cannot be used prior to '::' because it has no members}}
// since-cxx17-note@#cwg1330-f {{in instantiation of exception specification for 'f<short>' requested here}}
// since-cxx17-note@#cwg1330-f-short {{in instantiation of function template specialization 'cwg1330::f<short>' requested here}}

template<typename T> struct C {
C() throw(typename T::type); // #cwg1330-C
Expand Down Expand Up @@ -500,7 +497,7 @@ namespace cwg1359 { // cwg1359: 3.5
union B { constexpr B() = default; int a; }; // #cwg1359-B
// cxx11-17-error@-1 {{defaulted definition of default constructor cannot be marked constexpr before C++23}}
union C { constexpr C() = default; int a, b; }; // #cwg1359-C
// cxx11-17-error@-1 {{defaulted definition of default constructor cannot be marked constexpr}}
// cxx11-17-error@-1 {{defaulted definition of default constructor cannot be marked constexpr}}
struct X { constexpr X() = default; union {}; };
// since-cxx11-error@-1 {{declaration does not declare anything}}
struct Y { constexpr Y() = default; union { int a; }; }; // #cwg1359-Y
Expand Down Expand Up @@ -720,7 +717,7 @@ struct A {
} // namespace cwg1397

namespace cwg1399 { // cwg1399: dup 1388
template<typename ...T> void f(T..., int, T...) {} // #cwg1399-f
template<typename ...T> void f(T..., int, T...) {} // #cwg1399-f
// cxx98-error@-1 {{variadic templates are a C++11 extension}}
void g() {
f(0);
Expand Down
27 changes: 27 additions & 0 deletions clang/test/CXX/except/except.spec/p13.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -72,3 +72,30 @@ template<>
void f(A, int***); // expected-error {{'f<A, int>' is missing exception specification 'noexcept'}}

}

namespace N3 {

template<typename T, typename U>
void f(T, U) noexcept(T::y); // #1

template<typename T, typename U> // #2
void f(T, U*) noexcept(T::x);

// Deduction should succeed for both candidates, and #2 should be selected by overload resolution.
// Only the exception specification of #2 should be instantiated.
void (*x)(A, int*) = f;
}

namespace N4 {

template<typename T, typename U>
void f(T, U) noexcept(T::x); // #1

template<typename T, typename U>
void f(T, U*) noexcept(T::y); // #2
// expected-error@-1 {{no member named 'y' in 'A'}}

// Deduction should succeed for both candidates, and #2 should be selected by overload resolution.
// Only the exception specification of #2 should be instantiated.
void (*x)(A, int*) = f; // expected-note {{in instantiation of exception specification for 'f<A, int>' requested here}}
}
14 changes: 14 additions & 0 deletions clang/test/CXX/temp/temp.deduct/p7.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
// RUN: %clang_cc1 -verify %s

struct A {
static constexpr bool x = true;
};

template<typename T, typename U>
void f(T, U) noexcept(T::x);

template<typename T, typename U>
void f(T, U*) noexcept(T::y); // expected-error {{no member named 'y' in 'A'}}

template<>
void f<A>(A, int*); // expected-note {{in instantiation of exception specification}}
4 changes: 2 additions & 2 deletions clang/test/SemaCXX/cxx1z-noexcept-function-type.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ template<typename A, typename B> void redecl3() throw(B); // expected-error {{do

typedef int I;
template<bool B> void redecl4(I) noexcept(B);
template<bool B> void redecl4(I) noexcept(B); // expected-note {{could not match 'void (I) noexcept(false)' (aka 'void (int) noexcept(false)') against 'void (int) noexcept'}}
template<bool B> void redecl4(I) noexcept(B);

void (*init_with_exact_type_a)(int) noexcept = redecl4<true>;
void (*init_with_mismatched_type_a)(int) = redecl4<true>;
Expand All @@ -27,7 +27,7 @@ using DeducedType_a = decltype(deduce_auto_from_noexcept_function_ptr_a);
using DeducedType_a = void (*)(int) noexcept;

void (*init_with_exact_type_b)(int) = redecl4<false>;
void (*init_with_mismatched_type_b)(int) noexcept = redecl4<false>; // expected-error {{does not match required type}}
void (*init_with_mismatched_type_b)(int) noexcept = redecl4<false>; // expected-error {{cannot initialize a variable of type}}
auto deduce_auto_from_noexcept_function_ptr_b = redecl4<false>;
using DeducedType_b = decltype(deduce_auto_from_noexcept_function_ptr_b);
using DeducedType_b = void (*)(int);
Expand Down
10 changes: 4 additions & 6 deletions clang/test/SemaTemplate/temp_arg_type.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ A<0> *a1; // expected-error{{template argument for template type parameter must
A<A> *a2; // expected-error{{use of class template 'A' requires template arguments}}

A<int> *a3;
A<int()> *a4;
A<int()> *a4;
A<int(float)> *a5;
A<A<int> > *a6;

Expand Down Expand Up @@ -95,15 +95,13 @@ namespace deduce_noexcept {
template void dep() noexcept(true); // expected-error {{does not refer to a function template}}
template void dep() noexcept(false); // expected-error {{does not refer to a function template}}

// FIXME: It's also not clear whether this should be valid: do we substitute
// into the function type (including the exception specification) or not?
template<typename T> typename T::type1 f() noexcept(T::a);
template<typename T> typename T::type2 f() noexcept(T::b) {}
template<typename T> typename T::type1 f() noexcept(T::a); // expected-note {{candidate}}
template<typename T> typename T::type2 f() noexcept(T::b) {} // expected-note {{candidate}}
struct X {
static constexpr bool b = true;
using type1 = void;
using type2 = void;
};
template void f<X>();
template void f<X>(); // expected-error {{partial ordering for explicit instantiation of 'f' is ambiguous}}
}
#endif

0 comments on commit 77c5cea

Please sign in to comment.