[PATCH] c++: template-id ADL and partial instantiation [PR99911]
Jason Merrill
jason@redhat.com
Thu Nov 18 16:02:22 GMT 2021
On 11/18/21 10:31, Patrick Palka wrote:
> On Thu, 18 Nov 2021, Patrick Palka wrote:
>
>> On Thu, 18 Nov 2021, Jason Merrill wrote:
>>
>>> On 11/10/21 11:53, Patrick Palka wrote:
>>>> Here when partially instantiating the call get<U>(T{}) with T=N::A
>>>> (for which earlier unqualified name lookup for 'get' found nothing)
>>>> the arguments after substitution are no longer dependent but the callee
>>>> still is, so perform_koenig_lookup postpones ADL. But then we go on to
>>>> diagnose the unresolved template name anyway, as if ADL was already
>>>> performed and failed.
>>>>
>>>> This patch fixes this by avoiding the error path in question when the
>>>> template arguments of an unresolved template-id are dependent, which
>>>> mirrors the dependence check in perform_koenig_lookup.
>>>
>>> This change is OK.
>>>
>>>> In passing, this
>>>> patch also disables the -fpermissive fallback that performs a second
>>>> unqualified lookup in the template-id ADL case; this fallback seems to be
>>>> intended for legacy code and shouldn't be used for C++20 template-id ADL.
>>>
>>> Why wouldn't we want the more helpful diagnostic?
>>
>> The "no declarations were found by ADL" diagnostic is helpful, but the
>> backwards compatibility logic doesn't correctly handle the template-id
>> case. E.g. for
>>
>> template<class T>
>> void f() {
>> g<int>(T{});
>> }
>>
>> template<class T>
>> void g(int); // #1
>>
>> int main() {
>> f<int>();
>> }
>>
>> we get the helpful diagnostic followed by a confusing one because we
>> didn't incorporate the template-id's template arguments when replacing
>> the callee with the later-declared template #1:
>>
>> <stdin>: In instantiation of ‘void f() [with T = int]’:
>> <stdin>:10:9: required from here
>> <stdin>:3:6: error: ‘g’ was not declared in this scope, and no declarations were found by argument-dependent lookup at the point of instantiation [-fpermissive]
>> <stdin>:7:6: note: ‘template<class T> void g(int)’ declared here, later in the translation unit
>> <stdin>:3:6: error: no matching function for call to ‘g(int)’
>> <stdin>:7:6: note: candidate: ‘template<class T> void g(int)’
>> <stdin>:7:6: note: template argument deduction/substitution failed:
>> <stdin>:3:6: note: couldn’t deduce template parameter ‘T’
>>
>>
>> We also ignores template-ness of the name being looked up, so e.g. for:
>>
>> template<class T>
>> void f() {
>> g<>(T{});
>> }
>>
>> void g(int); // #1
>>
>> int main() {
>> f<int>();
>> }
>>
>> the secondary unqualified lookup finds the later-declared non-template #1:
>>
>> <stdin>: In instantiation of ‘void f() [with T = int]’:
>> <stdin>:9:9: required from here
>> <stdin>:3:6: error: ‘g’ was not declared in this scope, and no declarations were found by argument-dependent lookup at the point of instantiation [-fpermissive]
>> <stdin>:6:6: note: ‘void g(int)’ declared here, later in the translation unit
>>
>> which doesn't seem right.
>>
>> To fix the first issue, rather than disabling the diagnostic perhaps we
>> should just disable the backwards compatibility logic in the template-id
>> case, as in the below?
>
> Now in patch form:
>
> -- >8 --
>
> Subject: [PATCH] c++: error recovery during C++20 template-id ADL failure
>
> When diagnosing ADL failure we perform a second unqualified lookup for
> backwards compatibility with legacy code, and for better diagnostics.
>
> For C++20 template-id ADL however, the backwards compatibility logic
> causes confusing subsequent diagnostics, such as in the testcase below
> where we report deduction failure following the useful "no declarations
> were found by ADL" diagnostic because we've discarded the arguments of
> the template-id when replacing it with the later-declared template.
>
> So for C++20 template-id ADL, this patch just disables the backwards
> compatibility code while keeping the useful diagnostic.
>
> gcc/cp/ChangeLog:
>
> * pt.c (tsubst_copy_and_build) <case CALL_EXPR>: Disable the
> -fpermissive fallback for template-id ADL, but keep the
> diagnostic.
>
> gcc/testsuite/ChangeLog:
>
> * g++.dg/cpp2a/fn-template25.C: New test.
> ---
> gcc/cp/pt.c | 23 +++++++++++++---------
> gcc/testsuite/g++.dg/cpp2a/fn-template25.C | 14 +++++++++++++
> 2 files changed, 28 insertions(+), 9 deletions(-)
> create mode 100644 gcc/testsuite/g++.dg/cpp2a/fn-template25.C
>
> diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
> index ad51c07347b..3f1550a17ad 100644
> --- a/gcc/cp/pt.c
> +++ b/gcc/cp/pt.c
> @@ -20439,8 +20439,12 @@ tsubst_copy_and_build (tree t,
> (function, 1))))
> && !any_type_dependent_arguments_p (call_args))
> {
> + bool template_id_p = false;
> if (TREE_CODE (function) == TEMPLATE_ID_EXPR)
> - function = TREE_OPERAND (function, 0);
> + {
> + function = TREE_OPERAND (function, 0);
> + template_id_p = true;
> + }
I think
bool template_id_p = TREE_CODE (function) == TEMPLATE_ID_EXPR;
would simplify this. OK either way.
> if (koenig_p && (complain & tf_warning_or_error))
> {
> /* For backwards compatibility and good diagnostics, try
> @@ -20454,20 +20458,21 @@ tsubst_copy_and_build (tree t,
>
> if (unq != function)
> {
> - /* In a lambda fn, we have to be careful to not
> - introduce new this captures. Legacy code can't
> - be using lambdas anyway, so it's ok to be
> - stricter. */
> - bool in_lambda = (current_class_type
> - && LAMBDA_TYPE_P (current_class_type));
> char const *const msg
> = G_("%qD was not declared in this scope, "
> "and no declarations were found by "
> "argument-dependent lookup at the point "
> "of instantiation");
>
> + bool in_lambda = (current_class_type
> + && LAMBDA_TYPE_P (current_class_type));
> + /* In a lambda fn, we have to be careful to not
> + introduce new this captures. Legacy code can't
> + be using lambdas anyway, so it's ok to be
> + stricter. Be strict with C++20 template-id ADL too. */
> + bool strict = in_lambda || template_id_p;
> bool diag = true;
> - if (in_lambda)
> + if (strict)
> error_at (cp_expr_loc_or_input_loc (t),
> msg, function);
> else
> @@ -20503,7 +20508,7 @@ tsubst_copy_and_build (tree t,
> inform (DECL_SOURCE_LOCATION (fn),
> "%qD declared here, later in the "
> "translation unit", fn);
> - if (in_lambda)
> + if (strict)
> RETURN (error_mark_node);
> }
>
> diff --git a/gcc/testsuite/g++.dg/cpp2a/fn-template25.C b/gcc/testsuite/g++.dg/cpp2a/fn-template25.C
> new file mode 100644
> index 00000000000..a8888af2023
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/cpp2a/fn-template25.C
> @@ -0,0 +1,14 @@
> +// { dg-do compile { target c++20 } }
> +
> +template<class T>
> +void f() {
> + g<int>(T{}); // { dg-error "argument-dependent lookup" }
> + // { dg-bogus "no match" "" { target *-*-* } .-1 }
> +}
> +
> +template<class T>
> +void g(int); // { dg-message "declared here, later" }
> +
> +int main() {
> + f<int>();
> +}
>
More information about the Gcc-patches
mailing list