Re: [PATCH] c++: unqual lookup performed twice w/ template-id ADL [PR102670]

[Jason Merrill <jason@redhat.com>]

On 11/3/21 12:04, Patrick Palka wrote:
> Here we're incorrectly performing unqualified lookup of 'adl' again at
> substitution time for the call adl<I>(t) (for which name lookup at parse
> time found nothing) which causes us to reject the testcase because the
> second unqualified lookup finds the later-declared variable template
> 'adl', leading to confusion.  Fixed thusly.
> 
> The testcase concepts-recursive-sat1.C needed to be adjusted use ADL
> proper instead of relying on this incorrect behavior.
> 
> Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK for
> trunk and perhaps 11 given it's a C++20 bugfix?

OK for trunk.  Not for 11, I think, as it also affects non-C++20 code.

> 	PR c++/102670
> 
> gcc/cp/ChangeLog:
> 
> 	* pt.c (tsubst_copy_and_build) <case CALL_EXPR>: When looking
> 	for an identifier callee in the koenig_p case, also look through
> 	TEMPLATE_ID_EXPR.  Use tsubst_copy to substitute through the
> 	template arguments of the template-id.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* g++.dg/cpp2a/concepts-recursive-sat1.C:
> 	* g++.dg/cpp2a/fn-template23.C: New test.
> ---
>   gcc/cp/pt.c                                   | 11 +++++-
>   .../g++.dg/cpp2a/concepts-recursive-sat1.C    | 15 +++++---
>   gcc/testsuite/g++.dg/cpp2a/fn-template23.C    | 36 +++++++++++++++++++
>   3 files changed, 56 insertions(+), 6 deletions(-)
>   create mode 100644 gcc/testsuite/g++.dg/cpp2a/fn-template23.C
> 
> diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
> index 66040035b2f..40f84648ed2 100644
> --- a/gcc/cp/pt.c
> +++ b/gcc/cp/pt.c
> @@ -20256,7 +20256,10 @@ tsubst_copy_and_build (tree t,
>   					    /*done=*/false,
>   					    /*address_p=*/false);
>   	  }
> -	else if (koenig_p && identifier_p (function))
> +	else if (koenig_p
> +		 && (identifier_p (function)
> +		     || (TREE_CODE (function) == TEMPLATE_ID_EXPR
> +			 && identifier_p (TREE_OPERAND (function, 0)))))
>   	  {
>   	    /* Do nothing; calling tsubst_copy_and_build on an identifier
>   	       would incorrectly perform unqualified lookup again.
> @@ -20269,6 +20272,12 @@ tsubst_copy_and_build (tree t,
>   	       FIXME but doing that causes c++/15272, so we need to stop
>   	       using IDENTIFIER_NODE in that situation.  */
>   	    qualified_p = false;
> +
> +	    if (TREE_CODE (function) == TEMPLATE_ID_EXPR)
> +	      /* Use tsubst_copy to substitute through the template arguments
> +		 of the template-id without performing unqualified lookup on
> +		 the template name.  */
> +	      function = tsubst_copy (function, args, complain, in_decl);
>   	  }
>   	else
>   	  {
> diff --git a/gcc/testsuite/g++.dg/cpp2a/concepts-recursive-sat1.C b/gcc/testsuite/g++.dg/cpp2a/concepts-recursive-sat1.C
> index 22696c30d81..4c178b77946 100644
> --- a/gcc/testsuite/g++.dg/cpp2a/concepts-recursive-sat1.C
> +++ b/gcc/testsuite/g++.dg/cpp2a/concepts-recursive-sat1.C
> @@ -3,16 +3,21 @@
>   template<int N, typename T>
>   concept Foo = requires(T t) { foo<N + 1>(t); }; // { dg-error "template instantiation depth" }
>   
> -template<int N = 1, typename T = int>
> -  requires Foo<N, T>
> -int foo(T t)
> +namespace ns
>   {
> -  return foo<N + 1>(t);
> +  struct S { };
> +
> +  template<int N, typename T>
> +    requires Foo<N, T>
> +  int foo(T t)
> +  {
> +    return foo<N + 1>(t);
> +  }
>   }
>   
>   int main(int, char**)
>   {
> -  return foo<1>(1);
> +  return ns::foo<1>(ns::S{});
>   }
>   
>   // { dg-prune-output "compilation terminated" }
> diff --git a/gcc/testsuite/g++.dg/cpp2a/fn-template23.C b/gcc/testsuite/g++.dg/cpp2a/fn-template23.C
> new file mode 100644
> index 00000000000..b85d4c96dab
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/cpp2a/fn-template23.C
> @@ -0,0 +1,36 @@
> +// PR c++/102670
> +// { dg-do compile { target c++20 } }
> +
> +namespace ns {
> +  struct S { };
> +
> +  template<int I>
> +  constexpr int adl(const S &) {
> +    return I;
> +  }
> +}
> +
> +namespace redirect {
> +  template<class T, int I>
> +  concept can_call_adl = requires(T t) {
> +    adl<I>(t);
> +  };
> +
> +  template<int I>
> +  struct adl_fn {
> +    template<can_call_adl<I> T>
> +    constexpr decltype(auto) operator()(T t) const {
> +      return adl<I>(t);
> +    }
> +  };
> +
> +  namespace {
> +    template<int I>
> +    constexpr inline adl_fn<I> adl{};
> +  }
> +}
> +
> +int main() {
> +  static_assert(redirect::can_call_adl<ns::S, 3>);
> +  redirect::adl<3>(ns::S{});
> +}
>