[PATCH] c++: odr-use argument to a function NTTP [PR53164]

2021-10-04 Thread Patrick Palka via Gcc-patches
When passing a function template as the argument to a function NTTP
inside a template, we resolve it to the right specialization ahead of
time via resolve_address_of_overloaded_function, though the call to
mark_used within defers odr-using it until instantiation time (as usual).
But at instantiation time we end up never calling mark_used on the
specialization.

This patch fixes this by adding a call to mark_used in
convert_nontype_argument_function.

PR c++/53164

gcc/cp/ChangeLog:

* pt.c (convert_nontype_argument_function): Call mark_used.

gcc/testsuite/ChangeLog:

* g++.dg/template/non-dependent16.C: New test.
---
 gcc/cp/pt.c |  3 +++
 gcc/testsuite/g++.dg/template/non-dependent16.C | 16 
 2 files changed, 19 insertions(+)
 create mode 100644 gcc/testsuite/g++.dg/template/non-dependent16.C

diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
index f950f4a21b7..5e819c9598c 100644
--- a/gcc/cp/pt.c
+++ b/gcc/cp/pt.c
@@ -6668,6 +6668,9 @@ convert_nontype_argument_function (tree type, tree expr,
   return NULL_TREE;
 }
 
+  if (!mark_used (fn_no_ptr, complain) && !(complain & tf_error))
+return NULL_TREE;
+
   linkage = decl_linkage (fn_no_ptr);
   if (cxx_dialect >= cxx11 ? linkage == lk_none : linkage != lk_external)
 {
diff --git a/gcc/testsuite/g++.dg/template/non-dependent16.C 
b/gcc/testsuite/g++.dg/template/non-dependent16.C
new file mode 100644
index 000..b7dca8f6752
--- /dev/null
+++ b/gcc/testsuite/g++.dg/template/non-dependent16.C
@@ -0,0 +1,16 @@
+// PR c++/53164
+
+template
+void f(T) {
+  T::fail; // { dg-error "not a member" }
+}
+
+template
+struct A { };
+
+template
+void g() {
+  A a;
+}
+
+template void g<0>();
-- 
2.33.0.610.gcefe983a32



Re: [PATCH] c++: odr-use argument to a function NTTP [PR53164]

2021-10-05 Thread Patrick Palka via Gcc-patches
On Mon, 4 Oct 2021, Patrick Palka wrote:

> When passing a function template as the argument to a function NTTP
> inside a template, we resolve it to the right specialization ahead of
> time via resolve_address_of_overloaded_function, though the call to
> mark_used within defers odr-using it until instantiation time (as usual).
> But at instantiation time we end up never calling mark_used on the
> specialization.
> 
> This patch fixes this by adding a call to mark_used in
> convert_nontype_argument_function.
> 
>   PR c++/53164
> 
> gcc/cp/ChangeLog:
> 
>   * pt.c (convert_nontype_argument_function): Call mark_used.
> 
> gcc/testsuite/ChangeLog:
> 
>   * g++.dg/template/non-dependent16.C: New test.
> ---
>  gcc/cp/pt.c |  3 +++
>  gcc/testsuite/g++.dg/template/non-dependent16.C | 16 
>  2 files changed, 19 insertions(+)
>  create mode 100644 gcc/testsuite/g++.dg/template/non-dependent16.C
> 
> diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
> index f950f4a21b7..5e819c9598c 100644
> --- a/gcc/cp/pt.c
> +++ b/gcc/cp/pt.c
> @@ -6668,6 +6668,9 @@ convert_nontype_argument_function (tree type, tree expr,
>return NULL_TREE;
>  }
>  
> +  if (!mark_used (fn_no_ptr, complain) && !(complain & tf_error))
> +return NULL_TREE;
> +
>linkage = decl_linkage (fn_no_ptr);
>if (cxx_dialect >= cxx11 ? linkage == lk_none : linkage != lk_external)
>  {
> diff --git a/gcc/testsuite/g++.dg/template/non-dependent16.C 
> b/gcc/testsuite/g++.dg/template/non-dependent16.C
> new file mode 100644
> index 000..b7dca8f6752
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/template/non-dependent16.C
> @@ -0,0 +1,16 @@
> +// PR c++/53164
> +
> +template
> +void f(T) {
> +  T::fail; // { dg-error "not a member" }
> +}
> +
> +template
> +struct A { };
> +
> +template
> +void g() {
> +  A a;
> +}

I should mention that the original testcase in the PR was slightly
different than this one in that it also performed a call to the NTTP,
e.g.

  template
  struct A {
static void h() {
  p(0);
}
  };

  template
  void g() {
A::h();
  }

  templated void g<0>();

and not even the call was enough to odr-use f, apparently because the
CALL_EXPR case of tsubst_expr calls mark_used on the callee only when
it's a FUNCTION_DECL, but in this case after substitution it's an
ADDR_EXPR of a FUNCTION_DECL.  Fixing this by looking through the ADDR_EXPR
worked, but IIUC the call isn't necessary for f to be odr-used, simply
using f as a template argument should be sufficient, so it seems the
above is better fix.

> +
> +template void g<0>();
> -- 
> 2.33.0.610.gcefe983a32
> 
> 



Re: [PATCH] c++: odr-use argument to a function NTTP [PR53164]

2021-10-05 Thread Jason Merrill via Gcc-patches

On 10/5/21 15:17, Patrick Palka wrote:

On Mon, 4 Oct 2021, Patrick Palka wrote:


When passing a function template as the argument to a function NTTP
inside a template, we resolve it to the right specialization ahead of
time via resolve_address_of_overloaded_function, though the call to
mark_used within defers odr-using it until instantiation time (as usual).
But at instantiation time we end up never calling mark_used on the
specialization.

This patch fixes this by adding a call to mark_used in
convert_nontype_argument_function.

PR c++/53164

gcc/cp/ChangeLog:

* pt.c (convert_nontype_argument_function): Call mark_used.

gcc/testsuite/ChangeLog:

* g++.dg/template/non-dependent16.C: New test.
---
  gcc/cp/pt.c |  3 +++
  gcc/testsuite/g++.dg/template/non-dependent16.C | 16 
  2 files changed, 19 insertions(+)
  create mode 100644 gcc/testsuite/g++.dg/template/non-dependent16.C

diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
index f950f4a21b7..5e819c9598c 100644
--- a/gcc/cp/pt.c
+++ b/gcc/cp/pt.c
@@ -6668,6 +6668,9 @@ convert_nontype_argument_function (tree type, tree expr,
return NULL_TREE;
  }
  
+  if (!mark_used (fn_no_ptr, complain) && !(complain & tf_error))

+return NULL_TREE;
+
linkage = decl_linkage (fn_no_ptr);
if (cxx_dialect >= cxx11 ? linkage == lk_none : linkage != lk_external)
  {
diff --git a/gcc/testsuite/g++.dg/template/non-dependent16.C 
b/gcc/testsuite/g++.dg/template/non-dependent16.C
new file mode 100644
index 000..b7dca8f6752
--- /dev/null
+++ b/gcc/testsuite/g++.dg/template/non-dependent16.C
@@ -0,0 +1,16 @@
+// PR c++/53164
+
+template
+void f(T) {
+  T::fail; // { dg-error "not a member" }
+}
+
+template
+struct A { };
+
+template
+void g() {
+  A a;
+}


I should mention that the original testcase in the PR was slightly
different than this one in that it also performed a call to the NTTP,
e.g.

   template
   struct A {
 static void h() {
   p(0);
 }
   };

   template
   void g() {
 A::h();
   }

   templated void g<0>();

and not even the call was enough to odr-use f, apparently because the
CALL_EXPR case of tsubst_expr calls mark_used on the callee only when
it's a FUNCTION_DECL, but in this case after substitution it's an
ADDR_EXPR of a FUNCTION_DECL.  Fixing this by looking through the ADDR_EXPR
worked, but IIUC the call isn't necessary for f to be odr-used, simply
using f as a template argument should be sufficient, so it seems the
above is better fix.


I agree that pedantically the use happens when substituting into the use 
of A, but convert_nontype_argument_function seems like a weird place 
to implement that; it's only called again during instantiation of A, 
when we instantiate the injected-class-name.  If A isn't 
instantiated, e.g. if 'a' is a pointer to A, we again don't 
instantiate f.


I see that clang doesn't reject your testcase, either, but MSVC and icc 
do (even with 'a' a pointer): https://godbolt.org/z/MGE6TcMch


Jason



Re: [PATCH] c++: odr-use argument to a function NTTP [PR53164]

2021-10-06 Thread Patrick Palka via Gcc-patches
On Tue, 5 Oct 2021, Jason Merrill wrote:

> On 10/5/21 15:17, Patrick Palka wrote:
> > On Mon, 4 Oct 2021, Patrick Palka wrote:
> > 
> > > When passing a function template as the argument to a function NTTP
> > > inside a template, we resolve it to the right specialization ahead of
> > > time via resolve_address_of_overloaded_function, though the call to
> > > mark_used within defers odr-using it until instantiation time (as usual).
> > > But at instantiation time we end up never calling mark_used on the
> > > specialization.
> > > 
> > > This patch fixes this by adding a call to mark_used in
> > > convert_nontype_argument_function.
> > > 
> > >   PR c++/53164
> > > 
> > > gcc/cp/ChangeLog:
> > > 
> > >   * pt.c (convert_nontype_argument_function): Call mark_used.
> > > 
> > > gcc/testsuite/ChangeLog:
> > > 
> > >   * g++.dg/template/non-dependent16.C: New test.
> > > ---
> > >   gcc/cp/pt.c |  3 +++
> > >   gcc/testsuite/g++.dg/template/non-dependent16.C | 16 
> > >   2 files changed, 19 insertions(+)
> > >   create mode 100644 gcc/testsuite/g++.dg/template/non-dependent16.C
> > > 
> > > diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
> > > index f950f4a21b7..5e819c9598c 100644
> > > --- a/gcc/cp/pt.c
> > > +++ b/gcc/cp/pt.c
> > > @@ -6668,6 +6668,9 @@ convert_nontype_argument_function (tree type, tree
> > > expr,
> > > return NULL_TREE;
> > >   }
> > >   +  if (!mark_used (fn_no_ptr, complain) && !(complain & tf_error))
> > > +return NULL_TREE;
> > > +
> > > linkage = decl_linkage (fn_no_ptr);
> > > if (cxx_dialect >= cxx11 ? linkage == lk_none : linkage !=
> > > lk_external)
> > >   {
> > > diff --git a/gcc/testsuite/g++.dg/template/non-dependent16.C
> > > b/gcc/testsuite/g++.dg/template/non-dependent16.C
> > > new file mode 100644
> > > index 000..b7dca8f6752
> > > --- /dev/null
> > > +++ b/gcc/testsuite/g++.dg/template/non-dependent16.C
> > > @@ -0,0 +1,16 @@
> > > +// PR c++/53164
> > > +
> > > +template
> > > +void f(T) {
> > > +  T::fail; // { dg-error "not a member" }
> > > +}
> > > +
> > > +template
> > > +struct A { };
> > > +
> > > +template
> > > +void g() {
> > > +  A a;
> > > +}
> > 
> > I should mention that the original testcase in the PR was slightly
> > different than this one in that it also performed a call to the NTTP,
> > e.g.
> > 
> >template
> >struct A {
> >  static void h() {
> >p(0);
> >  }
> >};
> > 
> >template
> >void g() {
> >  A::h();
> >}
> > 
> >templated void g<0>();
> > 
> > and not even the call was enough to odr-use f, apparently because the
> > CALL_EXPR case of tsubst_expr calls mark_used on the callee only when
> > it's a FUNCTION_DECL, but in this case after substitution it's an
> > ADDR_EXPR of a FUNCTION_DECL.  Fixing this by looking through the ADDR_EXPR
> > worked, but IIUC the call isn't necessary for f to be odr-used, simply
> > using f as a template argument should be sufficient, so it seems the
> > above is better fix.
> 
> I agree that pedantically the use happens when substituting into the use of
> A, but convert_nontype_argument_function seems like a weird place to
> implement that; it's only called again during instantiation of A, when we
> instantiate the injected-class-name.  If A isn't instantiated, e.g. if 'a'
> is a pointer to A, we again don't instantiate f.

I see, makes sense..  I'm not sure where else we can mark the use, then.
Since we resolve the OVERLOAD f to the FUNCTION_DECL f ahead of
time (during which mark_used doesn't actually instantiate f because
we're inside a template), at instantiation time the type A is already
non-dependent so tsubst_aggr_type avoids doing the work that would end
up calling convert_nontype_argument_function.

> 
> I see that clang doesn't reject your testcase, either, but MSVC and icc do
> (even with 'a' a pointer): https://godbolt.org/z/MGE6TcMch

FWIW although Clang doesn't reject 'A a;', it does reject
'using type = A;' weirdly enough: https://godbolt.org/z/T9qEn6bWW


Shall we just go with the other more specific approach, that makes sure
the CALL_EXPR case of tsubst_expr calls mark_used when the callee is an
ADDR_EXPR?  Something like (bootstrapped and regtested):

-- >8 --

PR c++/53164

gcc/cp/ChangeLog:

* pt.c (tsubst_copy_and_build) : Look through
ADDR_EXPR after substituting into the callee.

gcc/testsuite/ChangeLog:

* g++.dg/template/fn-ptr3.C: New test.
---
 gcc/cp/pt.c |  4 
 gcc/testsuite/g++.dg/template/fn-ptr3.C | 20 
 2 files changed, 24 insertions(+)
 create mode 100644 gcc/testsuite/g++.dg/template/fn-ptr3.C

diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
index 19e03369ffa..5af3a6472f8 100644
--- a/gcc/cp/pt.c
+++ b/gcc/cp/pt.c
@@ -20310,6 +20310,10 @@ tsubst_copy_and_build (tree t,
 
if (BASELINK_P (function))
  qualified_p = true;
+
+   if (TREE_CODE (functi

Re: [PATCH] c++: odr-use argument to a function NTTP [PR53164]

2021-10-06 Thread Patrick Palka via Gcc-patches
On Wed, 6 Oct 2021, Patrick Palka wrote:

> On Tue, 5 Oct 2021, Jason Merrill wrote:
> 
> > On 10/5/21 15:17, Patrick Palka wrote:
> > > On Mon, 4 Oct 2021, Patrick Palka wrote:
> > > 
> > > > When passing a function template as the argument to a function NTTP
> > > > inside a template, we resolve it to the right specialization ahead of
> > > > time via resolve_address_of_overloaded_function, though the call to
> > > > mark_used within defers odr-using it until instantiation time (as 
> > > > usual).
> > > > But at instantiation time we end up never calling mark_used on the
> > > > specialization.
> > > > 
> > > > This patch fixes this by adding a call to mark_used in
> > > > convert_nontype_argument_function.
> > > > 
> > > > PR c++/53164
> > > > 
> > > > gcc/cp/ChangeLog:
> > > > 
> > > > * pt.c (convert_nontype_argument_function): Call mark_used.
> > > > 
> > > > gcc/testsuite/ChangeLog:
> > > > 
> > > > * g++.dg/template/non-dependent16.C: New test.
> > > > ---
> > > >   gcc/cp/pt.c |  3 +++
> > > >   gcc/testsuite/g++.dg/template/non-dependent16.C | 16 
> > > >   2 files changed, 19 insertions(+)
> > > >   create mode 100644 gcc/testsuite/g++.dg/template/non-dependent16.C
> > > > 
> > > > diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
> > > > index f950f4a21b7..5e819c9598c 100644
> > > > --- a/gcc/cp/pt.c
> > > > +++ b/gcc/cp/pt.c
> > > > @@ -6668,6 +6668,9 @@ convert_nontype_argument_function (tree type, tree
> > > > expr,
> > > > return NULL_TREE;
> > > >   }
> > > >   +  if (!mark_used (fn_no_ptr, complain) && !(complain & tf_error))
> > > > +return NULL_TREE;
> > > > +
> > > > linkage = decl_linkage (fn_no_ptr);
> > > > if (cxx_dialect >= cxx11 ? linkage == lk_none : linkage !=
> > > > lk_external)
> > > >   {
> > > > diff --git a/gcc/testsuite/g++.dg/template/non-dependent16.C
> > > > b/gcc/testsuite/g++.dg/template/non-dependent16.C
> > > > new file mode 100644
> > > > index 000..b7dca8f6752
> > > > --- /dev/null
> > > > +++ b/gcc/testsuite/g++.dg/template/non-dependent16.C
> > > > @@ -0,0 +1,16 @@
> > > > +// PR c++/53164
> > > > +
> > > > +template
> > > > +void f(T) {
> > > > +  T::fail; // { dg-error "not a member" }
> > > > +}
> > > > +
> > > > +template
> > > > +struct A { };
> > > > +
> > > > +template
> > > > +void g() {
> > > > +  A a;
> > > > +}
> > > 
> > > I should mention that the original testcase in the PR was slightly
> > > different than this one in that it also performed a call to the NTTP,
> > > e.g.
> > > 
> > >template
> > >struct A {
> > >  static void h() {
> > >p(0);
> > >  }
> > >};
> > > 
> > >template
> > >void g() {
> > >  A::h();
> > >}
> > > 
> > >templated void g<0>();
> > > 
> > > and not even the call was enough to odr-use f, apparently because the
> > > CALL_EXPR case of tsubst_expr calls mark_used on the callee only when
> > > it's a FUNCTION_DECL, but in this case after substitution it's an
> > > ADDR_EXPR of a FUNCTION_DECL.  Fixing this by looking through the 
> > > ADDR_EXPR
> > > worked, but IIUC the call isn't necessary for f to be odr-used, simply
> > > using f as a template argument should be sufficient, so it seems the
> > > above is better fix.
> > 
> > I agree that pedantically the use happens when substituting into the use of
> > A, but convert_nontype_argument_function seems like a weird place to
> > implement that; it's only called again during instantiation of A, when we
> > instantiate the injected-class-name.  If A isn't instantiated, e.g. if 
> > 'a'
> > is a pointer to A, we again don't instantiate f.
> 
> I see, makes sense..  I'm not sure where else we can mark the use, then.
> Since we resolve the OVERLOAD f to the FUNCTION_DECL f ahead of
> time (during which mark_used doesn't actually instantiate f because
> we're inside a template), at instantiation time the type A is already
> non-dependent so tsubst_aggr_type avoids doing the work that would end
> up calling convert_nontype_argument_function.
> 
> > 
> > I see that clang doesn't reject your testcase, either, but MSVC and icc do
> > (even with 'a' a pointer): https://godbolt.org/z/MGE6TcMch
> 
> FWIW although Clang doesn't reject 'A a;', it does reject
> 'using type = A;' weirdly enough: https://godbolt.org/z/T9qEn6bWW
> 
> 
> Shall we just go with the other more specific approach, that makes sure
> the CALL_EXPR case of tsubst_expr calls mark_used when the callee is an
> ADDR_EXPR?  Something like (bootstrapped and regtested):

Err, this approach is wrong because by stripping the ADDR_EXPR here we
end up checking access of the unwrapped FUNCTION_DECL again after
substituting into the call.  So we incorrectly reject e.g.

  template
  void g() {
P(); // error: ‘static void A::h()’ is private within this context
  }

  struct A {
void f() {
  g();
}
  private:
static void h();
  };

since A::h isn't accessible from g.

Re: [PATCH] c++: odr-use argument to a function NTTP [PR53164]

2021-10-06 Thread Jason Merrill via Gcc-patches

On 10/6/21 15:52, Patrick Palka wrote:

On Wed, 6 Oct 2021, Patrick Palka wrote:


On Tue, 5 Oct 2021, Jason Merrill wrote:


On 10/5/21 15:17, Patrick Palka wrote:

On Mon, 4 Oct 2021, Patrick Palka wrote:


When passing a function template as the argument to a function NTTP
inside a template, we resolve it to the right specialization ahead of
time via resolve_address_of_overloaded_function, though the call to
mark_used within defers odr-using it until instantiation time (as usual).
But at instantiation time we end up never calling mark_used on the
specialization.

This patch fixes this by adding a call to mark_used in
convert_nontype_argument_function.

PR c++/53164

gcc/cp/ChangeLog:

* pt.c (convert_nontype_argument_function): Call mark_used.

gcc/testsuite/ChangeLog:

* g++.dg/template/non-dependent16.C: New test.
---
   gcc/cp/pt.c |  3 +++
   gcc/testsuite/g++.dg/template/non-dependent16.C | 16 
   2 files changed, 19 insertions(+)
   create mode 100644 gcc/testsuite/g++.dg/template/non-dependent16.C

diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
index f950f4a21b7..5e819c9598c 100644
--- a/gcc/cp/pt.c
+++ b/gcc/cp/pt.c
@@ -6668,6 +6668,9 @@ convert_nontype_argument_function (tree type, tree
expr,
 return NULL_TREE;
   }
   +  if (!mark_used (fn_no_ptr, complain) && !(complain & tf_error))
+return NULL_TREE;
+
 linkage = decl_linkage (fn_no_ptr);
 if (cxx_dialect >= cxx11 ? linkage == lk_none : linkage !=
lk_external)
   {
diff --git a/gcc/testsuite/g++.dg/template/non-dependent16.C
b/gcc/testsuite/g++.dg/template/non-dependent16.C
new file mode 100644
index 000..b7dca8f6752
--- /dev/null
+++ b/gcc/testsuite/g++.dg/template/non-dependent16.C
@@ -0,0 +1,16 @@
+// PR c++/53164
+
+template
+void f(T) {
+  T::fail; // { dg-error "not a member" }
+}
+
+template
+struct A { };
+
+template
+void g() {
+  A a;
+}


I should mention that the original testcase in the PR was slightly
different than this one in that it also performed a call to the NTTP,
e.g.

template
struct A {
  static void h() {
p(0);
  }
};

template
void g() {
  A::h();
}

templated void g<0>();

and not even the call was enough to odr-use f, apparently because the
CALL_EXPR case of tsubst_expr calls mark_used on the callee only when
it's a FUNCTION_DECL, but in this case after substitution it's an
ADDR_EXPR of a FUNCTION_DECL.  Fixing this by looking through the ADDR_EXPR
worked, but IIUC the call isn't necessary for f to be odr-used, simply
using f as a template argument should be sufficient, so it seems the
above is better fix.


I agree that pedantically the use happens when substituting into the use of
A, but convert_nontype_argument_function seems like a weird place to
implement that; it's only called again during instantiation of A, when we
instantiate the injected-class-name.  If A isn't instantiated, e.g. if 'a'
is a pointer to A, we again don't instantiate f.


I see, makes sense..  I'm not sure where else we can mark the use, then.
Since we resolve the OVERLOAD f to the FUNCTION_DECL f ahead of
time (during which mark_used doesn't actually instantiate f because
we're inside a template), at instantiation time the type A is already
non-dependent so tsubst_aggr_type avoids doing the work that would end
up calling convert_nontype_argument_function.



I see that clang doesn't reject your testcase, either, but MSVC and icc do
(even with 'a' a pointer): https://godbolt.org/z/MGE6TcMch


FWIW although Clang doesn't reject 'A a;', it does reject
'using type = A;' weirdly enough: https://godbolt.org/z/T9qEn6bWW


Shall we just go with the other more specific approach, that makes sure
the CALL_EXPR case of tsubst_expr calls mark_used when the callee is an
ADDR_EXPR?  Something like (bootstrapped and regtested):


Err, this approach is wrong because by stripping the ADDR_EXPR here we
end up checking access of the unwrapped FUNCTION_DECL again after
substituting into the call.  So we incorrectly reject e.g.

   template
   void g() {
 P(); // error: ‘static void A::h()’ is private within this context
   }

   struct A {
 void f() {
   g();
 }
   private:
 static void h();
   };

since A::h isn't accessible from g.


I guess you could call mark_used directly instead of stripping the 
ADDR_EXPR.


Or for the general problem, perhaps we could mark the TEMPLATE_INFO or 
TI_ARGS to indicate that we still need to mark_used the arguments when 
we encounter A again during instantiation?




-- >8 --

PR c++/53164

gcc/cp/ChangeLog:

* pt.c (tsubst_copy_and_build) : Look through
ADDR_EXPR after substituting into the callee.

gcc/testsuite/ChangeLog:

* g++.dg/template/fn-ptr3.C: New test.
---
  gcc/cp/pt.c |  4 
  gcc/testsuite/g++.dg/template/fn-ptr3.C | 20 
  2 files changed, 24 insertions(+)
  

Re: [PATCH] c++: odr-use argument to a function NTTP [PR53164]

2021-10-07 Thread Patrick Palka via Gcc-patches
On Wed, 6 Oct 2021, Jason Merrill wrote:

> On 10/6/21 15:52, Patrick Palka wrote:
> > On Wed, 6 Oct 2021, Patrick Palka wrote:
> > 
> > > On Tue, 5 Oct 2021, Jason Merrill wrote:
> > > 
> > > > On 10/5/21 15:17, Patrick Palka wrote:
> > > > > On Mon, 4 Oct 2021, Patrick Palka wrote:
> > > > > 
> > > > > > When passing a function template as the argument to a function NTTP
> > > > > > inside a template, we resolve it to the right specialization ahead
> > > > > > of
> > > > > > time via resolve_address_of_overloaded_function, though the call to
> > > > > > mark_used within defers odr-using it until instantiation time (as
> > > > > > usual).
> > > > > > But at instantiation time we end up never calling mark_used on the
> > > > > > specialization.
> > > > > > 
> > > > > > This patch fixes this by adding a call to mark_used in
> > > > > > convert_nontype_argument_function.
> > > > > > 
> > > > > > PR c++/53164
> > > > > > 
> > > > > > gcc/cp/ChangeLog:
> > > > > > 
> > > > > > * pt.c (convert_nontype_argument_function): Call mark_used.
> > > > > > 
> > > > > > gcc/testsuite/ChangeLog:
> > > > > > 
> > > > > > * g++.dg/template/non-dependent16.C: New test.
> > > > > > ---
> > > > > >gcc/cp/pt.c |  3 +++
> > > > > >gcc/testsuite/g++.dg/template/non-dependent16.C | 16
> > > > > > 
> > > > > >2 files changed, 19 insertions(+)
> > > > > >create mode 100644
> > > > > > gcc/testsuite/g++.dg/template/non-dependent16.C
> > > > > > 
> > > > > > diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
> > > > > > index f950f4a21b7..5e819c9598c 100644
> > > > > > --- a/gcc/cp/pt.c
> > > > > > +++ b/gcc/cp/pt.c
> > > > > > @@ -6668,6 +6668,9 @@ convert_nontype_argument_function (tree type,
> > > > > > tree
> > > > > > expr,
> > > > > >  return NULL_TREE;
> > > > > >}
> > > > > >+  if (!mark_used (fn_no_ptr, complain) && !(complain &
> > > > > > tf_error))
> > > > > > +return NULL_TREE;
> > > > > > +
> > > > > >  linkage = decl_linkage (fn_no_ptr);
> > > > > >  if (cxx_dialect >= cxx11 ? linkage == lk_none : linkage !=
> > > > > > lk_external)
> > > > > >{
> > > > > > diff --git a/gcc/testsuite/g++.dg/template/non-dependent16.C
> > > > > > b/gcc/testsuite/g++.dg/template/non-dependent16.C
> > > > > > new file mode 100644
> > > > > > index 000..b7dca8f6752
> > > > > > --- /dev/null
> > > > > > +++ b/gcc/testsuite/g++.dg/template/non-dependent16.C
> > > > > > @@ -0,0 +1,16 @@
> > > > > > +// PR c++/53164
> > > > > > +
> > > > > > +template
> > > > > > +void f(T) {
> > > > > > +  T::fail; // { dg-error "not a member" }
> > > > > > +}
> > > > > > +
> > > > > > +template
> > > > > > +struct A { };
> > > > > > +
> > > > > > +template
> > > > > > +void g() {
> > > > > > +  A a;
> > > > > > +}
> > > > > 
> > > > > I should mention that the original testcase in the PR was slightly
> > > > > different than this one in that it also performed a call to the NTTP,
> > > > > e.g.
> > > > > 
> > > > > template
> > > > > struct A {
> > > > >   static void h() {
> > > > > p(0);
> > > > >   }
> > > > > };
> > > > > 
> > > > > template
> > > > > void g() {
> > > > >   A::h();
> > > > > }
> > > > > 
> > > > > templated void g<0>();
> > > > > 
> > > > > and not even the call was enough to odr-use f, apparently because the
> > > > > CALL_EXPR case of tsubst_expr calls mark_used on the callee only when
> > > > > it's a FUNCTION_DECL, but in this case after substitution it's an
> > > > > ADDR_EXPR of a FUNCTION_DECL.  Fixing this by looking through the
> > > > > ADDR_EXPR
> > > > > worked, but IIUC the call isn't necessary for f to be odr-used, simply
> > > > > using f as a template argument should be sufficient, so it seems the
> > > > > above is better fix.
> > > > 
> > > > I agree that pedantically the use happens when substituting into the use
> > > > of
> > > > A, but convert_nontype_argument_function seems like a weird place to
> > > > implement that; it's only called again during instantiation of A,
> > > > when we
> > > > instantiate the injected-class-name.  If A isn't instantiated, e.g.
> > > > if 'a'
> > > > is a pointer to A, we again don't instantiate f.
> > > 
> > > I see, makes sense..  I'm not sure where else we can mark the use, then.
> > > Since we resolve the OVERLOAD f to the FUNCTION_DECL f ahead of
> > > time (during which mark_used doesn't actually instantiate f because
> > > we're inside a template), at instantiation time the type A is already
> > > non-dependent so tsubst_aggr_type avoids doing the work that would end
> > > up calling convert_nontype_argument_function.
> > > 
> > > > 
> > > > I see that clang doesn't reject your testcase, either, but MSVC and icc
> > > > do
> > > > (even with 'a' a pointer): https://godbolt.org/z/MGE6TcMch
> > > 
> > > FWIW although Clang doesn't reject 'A a;', it does reject
> > > 'using type = A;' weirdly enough: https://godbolt.org/

Re: [PATCH] c++: odr-use argument to a function NTTP [PR53164]

2021-10-07 Thread Jason Merrill via Gcc-patches

On 10/7/21 11:17, Patrick Palka wrote:

On Wed, 6 Oct 2021, Jason Merrill wrote:


On 10/6/21 15:52, Patrick Palka wrote:

On Wed, 6 Oct 2021, Patrick Palka wrote:


On Tue, 5 Oct 2021, Jason Merrill wrote:


On 10/5/21 15:17, Patrick Palka wrote:

On Mon, 4 Oct 2021, Patrick Palka wrote:


When passing a function template as the argument to a function NTTP
inside a template, we resolve it to the right specialization ahead
of
time via resolve_address_of_overloaded_function, though the call to
mark_used within defers odr-using it until instantiation time (as
usual).
But at instantiation time we end up never calling mark_used on the
specialization.

This patch fixes this by adding a call to mark_used in
convert_nontype_argument_function.

PR c++/53164

gcc/cp/ChangeLog:

* pt.c (convert_nontype_argument_function): Call mark_used.

gcc/testsuite/ChangeLog:

* g++.dg/template/non-dependent16.C: New test.
---
gcc/cp/pt.c |  3 +++
gcc/testsuite/g++.dg/template/non-dependent16.C | 16

2 files changed, 19 insertions(+)
create mode 100644
gcc/testsuite/g++.dg/template/non-dependent16.C

diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
index f950f4a21b7..5e819c9598c 100644
--- a/gcc/cp/pt.c
+++ b/gcc/cp/pt.c
@@ -6668,6 +6668,9 @@ convert_nontype_argument_function (tree type,
tree
expr,
  return NULL_TREE;
}
+  if (!mark_used (fn_no_ptr, complain) && !(complain &
tf_error))
+return NULL_TREE;
+
  linkage = decl_linkage (fn_no_ptr);
  if (cxx_dialect >= cxx11 ? linkage == lk_none : linkage !=
lk_external)
{
diff --git a/gcc/testsuite/g++.dg/template/non-dependent16.C
b/gcc/testsuite/g++.dg/template/non-dependent16.C
new file mode 100644
index 000..b7dca8f6752
--- /dev/null
+++ b/gcc/testsuite/g++.dg/template/non-dependent16.C
@@ -0,0 +1,16 @@
+// PR c++/53164
+
+template
+void f(T) {
+  T::fail; // { dg-error "not a member" }
+}
+
+template
+struct A { };
+
+template
+void g() {
+  A a;
+}


I should mention that the original testcase in the PR was slightly
different than this one in that it also performed a call to the NTTP,
e.g.

 template
 struct A {
   static void h() {
 p(0);
   }
 };

 template
 void g() {
   A::h();
 }

 templated void g<0>();

and not even the call was enough to odr-use f, apparently because the
CALL_EXPR case of tsubst_expr calls mark_used on the callee only when
it's a FUNCTION_DECL, but in this case after substitution it's an
ADDR_EXPR of a FUNCTION_DECL.  Fixing this by looking through the
ADDR_EXPR
worked, but IIUC the call isn't necessary for f to be odr-used, simply
using f as a template argument should be sufficient, so it seems the
above is better fix.


I agree that pedantically the use happens when substituting into the use
of
A, but convert_nontype_argument_function seems like a weird place to
implement that; it's only called again during instantiation of A,
when we
instantiate the injected-class-name.  If A isn't instantiated, e.g.
if 'a'
is a pointer to A, we again don't instantiate f.


I see, makes sense..  I'm not sure where else we can mark the use, then.
Since we resolve the OVERLOAD f to the FUNCTION_DECL f ahead of
time (during which mark_used doesn't actually instantiate f because
we're inside a template), at instantiation time the type A is already
non-dependent so tsubst_aggr_type avoids doing the work that would end
up calling convert_nontype_argument_function.



I see that clang doesn't reject your testcase, either, but MSVC and icc
do
(even with 'a' a pointer): https://godbolt.org/z/MGE6TcMch


FWIW although Clang doesn't reject 'A a;', it does reject
'using type = A;' weirdly enough: https://godbolt.org/z/T9qEn6bWW


Shall we just go with the other more specific approach, that makes sure
the CALL_EXPR case of tsubst_expr calls mark_used when the callee is an
ADDR_EXPR?  Something like (bootstrapped and regtested):


Err, this approach is wrong because by stripping the ADDR_EXPR here we
end up checking access of the unwrapped FUNCTION_DECL again after
substituting into the call.  So we incorrectly reject e.g.

template
void g() {
  P(); // error: ‘static void A::h()’ is private within this context
}

struct A {
  void f() {
g();
  }
private:
  static void h();
};

since A::h isn't accessible from g.


I guess you could call mark_used directly instead of stripping the ADDR_EXPR.


That seems to work nicely, how does the below look?  Bootstrapped and
regtested on x86_64-pc-linux-gnu.



Or for the general problem, perhaps we could mark the TEMPLATE_INFO or TI_ARGS
to indicate that we still need to mark_used the arguments when we encounter
A again during instantiation?


That sounds plausible, though I suppose it might not be worth it only to
handle such a corner case..


Indeed.  A lower-overhead possibility would be to remember, for a 
template