C++ PATCH to fix static init with () in a template (PR c++/84582)

Message ID 20180227191313.GN2995@redhat.com
State New
Headers show
Series
  • C++ PATCH to fix static init with () in a template (PR c++/84582)
Related show

Commit Message

Marek Polacek Feb. 27, 2018, 7:13 p.m.
My recent change introducing cxx_constant_init caused this code

template <class> class A {
  static const long b = 0;
  static const unsigned c = (b);
};

to be rejected.  The reason is that force_paren_expr turns "b" into "*(const
long int &) &b", where the former is not value-dependent but the latter is
value-dependent.  So when we get to maybe_constant_init_1:
5147   if (!is_nondependent_static_init_expression (t))
5148     /* Don't try to evaluate it.  */;
it's not evaluated and we get the non-constant initialization error.
(Before we'd always evaluated the expression.)

Bootstrapped/regtested on x86_64-linux, ok for trunk?

2018-02-27  Marek Polacek  <polacek@redhat.com>

	PR c++/84582
	* semantics.c (force_paren_expr): Avoid creating a static cast
	when processing a template.

	* g++.dg/cpp1z/static1.C: New test.
	* g++.dg/template/static37.C: New test.


	Marek

Comments

Jason Merrill Feb. 27, 2018, 9:16 p.m. | #1
On 02/27/2018 02:13 PM, Marek Polacek wrote:
> My recent change introducing cxx_constant_init caused this code

> 

> template <class> class A {

>    static const long b = 0;

>    static const unsigned c = (b);

> };

> 

> to be rejected.  The reason is that force_paren_expr turns "b" into "*(const

> long int &) &b", where the former is not value-dependent but the latter is

> value-dependent.  So when we get to maybe_constant_init_1:

> 5147   if (!is_nondependent_static_init_expression (t))

> 5148     /* Don't try to evaluate it.  */;

> it's not evaluated and we get the non-constant initialization error.

> (Before we'd always evaluated the expression.)

> 

> Bootstrapped/regtested on x86_64-linux, ok for trunk?

> 

> 2018-02-27  Marek Polacek  <polacek@redhat.com>

> 

> 	PR c++/84582

> 	* semantics.c (force_paren_expr): Avoid creating a static cast

> 	when processing a template.

> 

> 	* g++.dg/cpp1z/static1.C: New test.

> 	* g++.dg/template/static37.C: New test.

> 

> diff --git gcc/cp/semantics.c gcc/cp/semantics.c

> index 35569d0cb0d..b48de2df4e2 100644

> --- gcc/cp/semantics.c

> +++ gcc/cp/semantics.c

> @@ -1697,7 +1697,7 @@ force_paren_expr (tree expr)

>       expr = build1 (PAREN_EXPR, TREE_TYPE (expr), expr);

>     else if (VAR_P (expr) && DECL_HARD_REGISTER (expr))

>       /* We can't bind a hard register variable to a reference.  */;

> -  else

> +  else if (!processing_template_decl)


Hmm, this means that we forget about the parentheses in a template.  I'm 
surprised that this didn't break anything in the testsuite.  In 
particular, auto-fn15.C.  I've attached an addition to auto-fn15.C to 
catch this issue.

Can we use PAREN_EXPR instead of the static_cast in a template?

Jason
diff --git a/gcc/testsuite/g++.dg/cpp1y/auto-fn15.C b/gcc/testsuite/g++.dg/cpp1y/auto-fn15.C
index ba9f3579f62..0db428f7270 100644
--- a/gcc/testsuite/g++.dg/cpp1y/auto-fn15.C
+++ b/gcc/testsuite/g++.dg/cpp1y/auto-fn15.C
@@ -22,6 +22,8 @@ template <class T>
 decltype(auto) h5(T t) { return t.i; }
 template <class T>
 decltype(auto) h6(T t) { return (t.i); }
+template <class T>
+decltype(auto) h7(T t) { return (i); }
 
 int main()
 {
@@ -48,4 +50,5 @@ int main()
   same_type<decltype(h4()),int&>();
   same_type<decltype(h5(a)),int>();
   same_type<decltype(h6(a)),int&>();
+  same_type<decltype(h7(a)),int&>();
 }
Marek Polacek Feb. 28, 2018, 2:32 p.m. | #2
On Tue, Feb 27, 2018 at 04:16:31PM -0500, Jason Merrill wrote:
> On 02/27/2018 02:13 PM, Marek Polacek wrote:

> > My recent change introducing cxx_constant_init caused this code

> > 

> > template <class> class A {

> >    static const long b = 0;

> >    static const unsigned c = (b);

> > };

> > 

> > to be rejected.  The reason is that force_paren_expr turns "b" into "*(const

> > long int &) &b", where the former is not value-dependent but the latter is

> > value-dependent.  So when we get to maybe_constant_init_1:

> > 5147   if (!is_nondependent_static_init_expression (t))

> > 5148     /* Don't try to evaluate it.  */;

> > it's not evaluated and we get the non-constant initialization error.

> > (Before we'd always evaluated the expression.)

> > 

> > Bootstrapped/regtested on x86_64-linux, ok for trunk?

> > 

> > 2018-02-27  Marek Polacek  <polacek@redhat.com>

> > 

> > 	PR c++/84582

> > 	* semantics.c (force_paren_expr): Avoid creating a static cast

> > 	when processing a template.

> > 

> > 	* g++.dg/cpp1z/static1.C: New test.

> > 	* g++.dg/template/static37.C: New test.

> > 

> > diff --git gcc/cp/semantics.c gcc/cp/semantics.c

> > index 35569d0cb0d..b48de2df4e2 100644

> > --- gcc/cp/semantics.c

> > +++ gcc/cp/semantics.c

> > @@ -1697,7 +1697,7 @@ force_paren_expr (tree expr)

> >       expr = build1 (PAREN_EXPR, TREE_TYPE (expr), expr);

> >     else if (VAR_P (expr) && DECL_HARD_REGISTER (expr))

> >       /* We can't bind a hard register variable to a reference.  */;

> > -  else

> > +  else if (!processing_template_decl)

> 

> Hmm, this means that we forget about the parentheses in a template.  I'm

> surprised that this didn't break anything in the testsuite.  In particular,

> auto-fn15.C.  I've attached an addition to auto-fn15.C to catch this issue.


Thanks, you're right.  I'll use it.

> Can we use PAREN_EXPR instead of the static_cast in a template?


I don't think so, it would fix the issue you pointed out in auto-fn15.C but
it wouldn't fix the original test.  The problem with using PAREN_EXPR in a
template is that instantiate_non_dependent_expr will turn in into the
static cast anyway; tsubst_copy_and_build has
    case PAREN_EXPR:
      RETURN (finish_parenthesized_expr (RECUR (TREE_OPERAND (t, 0))));
so it calls force_paren_expr and this time we're not in a template.  And
then when calling cxx_constant_init we have the same issue.

Should we play some ugly games with maybe_undo_parenthesized_ref?

	Marek
Jason Merrill Feb. 28, 2018, 3:51 p.m. | #3
On Wed, Feb 28, 2018 at 9:32 AM, Marek Polacek <polacek@redhat.com> wrote:
> On Tue, Feb 27, 2018 at 04:16:31PM -0500, Jason Merrill wrote:

>> On 02/27/2018 02:13 PM, Marek Polacek wrote:

>> > My recent change introducing cxx_constant_init caused this code

>> >

>> > template <class> class A {

>> >    static const long b = 0;

>> >    static const unsigned c = (b);

>> > };

>> >

>> > to be rejected.  The reason is that force_paren_expr turns "b" into "*(const

>> > long int &) &b", where the former is not value-dependent but the latter is

>> > value-dependent.  So when we get to maybe_constant_init_1:

>> > 5147   if (!is_nondependent_static_init_expression (t))

>> > 5148     /* Don't try to evaluate it.  */;

>> > it's not evaluated and we get the non-constant initialization error.

>> > (Before we'd always evaluated the expression.)

>> >

>> > Bootstrapped/regtested on x86_64-linux, ok for trunk?

>> >

>> > 2018-02-27  Marek Polacek  <polacek@redhat.com>

>> >

>> >     PR c++/84582

>> >     * semantics.c (force_paren_expr): Avoid creating a static cast

>> >     when processing a template.

>> >

>> >     * g++.dg/cpp1z/static1.C: New test.

>> >     * g++.dg/template/static37.C: New test.

>> >

>> > diff --git gcc/cp/semantics.c gcc/cp/semantics.c

>> > index 35569d0cb0d..b48de2df4e2 100644

>> > --- gcc/cp/semantics.c

>> > +++ gcc/cp/semantics.c

>> > @@ -1697,7 +1697,7 @@ force_paren_expr (tree expr)

>> >       expr = build1 (PAREN_EXPR, TREE_TYPE (expr), expr);

>> >     else if (VAR_P (expr) && DECL_HARD_REGISTER (expr))

>> >       /* We can't bind a hard register variable to a reference.  */;

>> > -  else

>> > +  else if (!processing_template_decl)

>>

>> Hmm, this means that we forget about the parentheses in a template.  I'm

>> surprised that this didn't break anything in the testsuite.  In particular,

>> auto-fn15.C.  I've attached an addition to auto-fn15.C to catch this issue.

>

> Thanks, you're right.  I'll use it.

>

>> Can we use PAREN_EXPR instead of the static_cast in a template?

>

> I don't think so, it would fix the issue you pointed out in auto-fn15.C but

> it wouldn't fix the original test.  The problem with using PAREN_EXPR in a

> template is that instantiate_non_dependent_expr will turn in into the

> static cast anyway; tsubst_copy_and_build has

>     case PAREN_EXPR:

>       RETURN (finish_parenthesized_expr (RECUR (TREE_OPERAND (t, 0))));

> so it calls force_paren_expr and this time we're not in a template.  And

> then when calling cxx_constant_init we have the same issue.


Then maybe we need something like fold_non_dependent_expr, which
checks for dependency before substitution and then immediately
evaluates the result.

Jason
Marek Polacek Feb. 28, 2018, 9:19 p.m. | #4
On Wed, Feb 28, 2018 at 10:51:17AM -0500, Jason Merrill wrote:
> On Wed, Feb 28, 2018 at 9:32 AM, Marek Polacek <polacek@redhat.com> wrote:

> > On Tue, Feb 27, 2018 at 04:16:31PM -0500, Jason Merrill wrote:

> >> On 02/27/2018 02:13 PM, Marek Polacek wrote:

> >> > My recent change introducing cxx_constant_init caused this code

> >> >

> >> > template <class> class A {

> >> >    static const long b = 0;

> >> >    static const unsigned c = (b);

> >> > };

> >> >

> >> > to be rejected.  The reason is that force_paren_expr turns "b" into "*(const

> >> > long int &) &b", where the former is not value-dependent but the latter is

> >> > value-dependent.  So when we get to maybe_constant_init_1:

> >> > 5147   if (!is_nondependent_static_init_expression (t))

> >> > 5148     /* Don't try to evaluate it.  */;

> >> > it's not evaluated and we get the non-constant initialization error.

> >> > (Before we'd always evaluated the expression.)

> >> >

> >> > Bootstrapped/regtested on x86_64-linux, ok for trunk?

> >> >

> >> > 2018-02-27  Marek Polacek  <polacek@redhat.com>

> >> >

> >> >     PR c++/84582

> >> >     * semantics.c (force_paren_expr): Avoid creating a static cast

> >> >     when processing a template.

> >> >

> >> >     * g++.dg/cpp1z/static1.C: New test.

> >> >     * g++.dg/template/static37.C: New test.

> >> >

> >> > diff --git gcc/cp/semantics.c gcc/cp/semantics.c

> >> > index 35569d0cb0d..b48de2df4e2 100644

> >> > --- gcc/cp/semantics.c

> >> > +++ gcc/cp/semantics.c

> >> > @@ -1697,7 +1697,7 @@ force_paren_expr (tree expr)

> >> >       expr = build1 (PAREN_EXPR, TREE_TYPE (expr), expr);

> >> >     else if (VAR_P (expr) && DECL_HARD_REGISTER (expr))

> >> >       /* We can't bind a hard register variable to a reference.  */;

> >> > -  else

> >> > +  else if (!processing_template_decl)

> >>

> >> Hmm, this means that we forget about the parentheses in a template.  I'm

> >> surprised that this didn't break anything in the testsuite.  In particular,

> >> auto-fn15.C.  I've attached an addition to auto-fn15.C to catch this issue.

> >

> > Thanks, you're right.  I'll use it.

> >

> >> Can we use PAREN_EXPR instead of the static_cast in a template?

> >

> > I don't think so, it would fix the issue you pointed out in auto-fn15.C but

> > it wouldn't fix the original test.  The problem with using PAREN_EXPR in a

> > template is that instantiate_non_dependent_expr will turn in into the

> > static cast anyway; tsubst_copy_and_build has

> >     case PAREN_EXPR:

> >       RETURN (finish_parenthesized_expr (RECUR (TREE_OPERAND (t, 0))));

> > so it calls force_paren_expr and this time we're not in a template.  And

> > then when calling cxx_constant_init we have the same issue.

> 

> Then maybe we need something like fold_non_dependent_expr, which

> checks for dependency before substitution and then immediately

> evaluates the result.


I hope you meant something like this.  Further testing also revealed that
maybe_undo_parenthesized_ref should be able to unwrap PAREN_EXPR (so that
(fn1)(); in paren2.C is handled correctly), and that lvalue_kind should look
into PAREN_EXPR so as to give the correct answer regarding lvalueness: we
should accept

template<typename T>
void foo (int i)
{
  ++(i);
}

Apologies if I'm on the wrong track.

Bootstrapped/regtested on x86_64-linux, ok for trunk?

2018-02-28  Marek Polacek  <polacek@redhat.com>
	    Jason Merrill  <jason@redhat.com>

	PR c++/84582
	* semantics.c (force_paren_expr): Avoid creating the static cast
	when in a template.  Create a PAREN_EXPR when in a template.
	(maybe_undo_parenthesized_ref): Unwrap PAREN_EXPR.
	* typeck2.c (store_init_value): Call fold_non_dependent_expr instead
	of instantiate_non_dependent_expr.
	* tree.c (lvalue_kind): Handle PAREN_EXPR like NON_DEPENDENT_EXPR.

	* g++.dg/cpp1y/auto-fn15.C: Extend testing.
	* g++.dg/cpp1z/static1.C: New test.
	* g++.dg/template/static37.C: New test.

diff --git gcc/cp/semantics.c gcc/cp/semantics.c
index 35569d0cb0d..722e3718a14 100644
--- gcc/cp/semantics.c
+++ gcc/cp/semantics.c
@@ -1697,7 +1697,7 @@ force_paren_expr (tree expr)
     expr = build1 (PAREN_EXPR, TREE_TYPE (expr), expr);
   else if (VAR_P (expr) && DECL_HARD_REGISTER (expr))
     /* We can't bind a hard register variable to a reference.  */;
-  else
+  else if (!processing_template_decl)
     {
       cp_lvalue_kind kind = lvalue_kind (expr);
       if ((kind & ~clk_class) != clk_none)
@@ -1713,6 +1713,8 @@ force_paren_expr (tree expr)
 	    REF_PARENTHESIZED_P (expr) = true;
 	}
     }
+  else
+    expr = build1 (PAREN_EXPR, TREE_TYPE (expr), expr);
 
   return expr;
 }
@@ -1724,9 +1726,10 @@ force_paren_expr (tree expr)
 tree
 maybe_undo_parenthesized_ref (tree t)
 {
-  if (cxx_dialect >= cxx14
-      && INDIRECT_REF_P (t)
-      && REF_PARENTHESIZED_P (t))
+  if (cxx_dialect < cxx14)
+    return t;
+
+  if (INDIRECT_REF_P (t) && REF_PARENTHESIZED_P (t))
     {
       t = TREE_OPERAND (t, 0);
       while (TREE_CODE (t) == NON_LVALUE_EXPR
@@ -1737,6 +1740,8 @@ maybe_undo_parenthesized_ref (tree t)
 		  || TREE_CODE (t) == STATIC_CAST_EXPR);
       t = TREE_OPERAND (t, 0);
     }
+  else if (TREE_CODE (t) == PAREN_EXPR)
+    t = TREE_OPERAND (t, 0);
 
   return t;
 }
diff --git gcc/cp/tree.c gcc/cp/tree.c
index 9b9e36a1173..19f1c0629c9 100644
--- gcc/cp/tree.c
+++ gcc/cp/tree.c
@@ -239,6 +239,7 @@ lvalue_kind (const_tree ref)
       return lvalue_kind (BASELINK_FUNCTIONS (CONST_CAST_TREE (ref)));
 
     case NON_DEPENDENT_EXPR:
+    case PAREN_EXPR:
       return lvalue_kind (TREE_OPERAND (ref, 0));
 
     case VIEW_CONVERT_EXPR:
diff --git gcc/cp/typeck2.c gcc/cp/typeck2.c
index 153b46cca77..583c65d4d0a 100644
--- gcc/cp/typeck2.c
+++ gcc/cp/typeck2.c
@@ -822,7 +822,7 @@ store_init_value (tree decl, tree init, vec<tree, va_gc>** cleanups, int flags)
   if (decl_maybe_constant_var_p (decl) || TREE_STATIC (decl))
     {
       bool const_init;
-      value = instantiate_non_dependent_expr (value);
+      value = fold_non_dependent_expr (value);
       if (DECL_DECLARED_CONSTEXPR_P (decl)
 	  || (DECL_IN_AGGR_P (decl) && !DECL_VAR_DECLARED_INLINE_P (decl)))
 	{
diff --git gcc/testsuite/g++.dg/cpp1y/auto-fn15.C gcc/testsuite/g++.dg/cpp1y/auto-fn15.C
index ba9f3579f62..0db428f7270 100644
--- gcc/testsuite/g++.dg/cpp1y/auto-fn15.C
+++ gcc/testsuite/g++.dg/cpp1y/auto-fn15.C
@@ -22,6 +22,8 @@ template <class T>
 decltype(auto) h5(T t) { return t.i; }
 template <class T>
 decltype(auto) h6(T t) { return (t.i); }
+template <class T>
+decltype(auto) h7(T t) { return (i); }
 
 int main()
 {
@@ -48,4 +50,5 @@ int main()
   same_type<decltype(h4()),int&>();
   same_type<decltype(h5(a)),int>();
   same_type<decltype(h6(a)),int&>();
+  same_type<decltype(h7(a)),int&>();
 }
diff --git gcc/testsuite/g++.dg/cpp1z/static1.C gcc/testsuite/g++.dg/cpp1z/static1.C
index e69de29bb2d..cb872997c5a 100644
--- gcc/testsuite/g++.dg/cpp1z/static1.C
+++ gcc/testsuite/g++.dg/cpp1z/static1.C
@@ -0,0 +1,19 @@
+// PR c++/84582
+// { dg-options -std=c++17 }
+
+class C {
+  static inline const long b = 0;
+  static inline const unsigned c = (b);
+};
+class D {
+  static inline const long b = 0;
+  static inline const unsigned c = b;
+};
+template <class> class A {
+  static inline const long b = 0;
+  static inline const unsigned c = (b);
+};
+template <class> class B {
+  static inline const long b = 0;
+  static inline const unsigned c = b;
+};
diff --git gcc/testsuite/g++.dg/template/static37.C gcc/testsuite/g++.dg/template/static37.C
index e69de29bb2d..90bc65d2fbc 100644
--- gcc/testsuite/g++.dg/template/static37.C
+++ gcc/testsuite/g++.dg/template/static37.C
@@ -0,0 +1,18 @@
+// PR c++/84582
+
+class C {
+  static const long b = 0;
+  static const unsigned c = (b);
+};
+class D {
+  static const long b = 0;
+  static const unsigned c = b;
+};
+template <class> class A {
+  static const long b = 0;
+  static const unsigned c = (b);
+};
+template <class> class B {
+  static const long b = 0;
+  static const unsigned c = b;
+};

	Marek
Jason Merrill Feb. 28, 2018, 9:50 p.m. | #5
On Wed, Feb 28, 2018 at 4:19 PM, Marek Polacek <polacek@redhat.com> wrote:
> On Wed, Feb 28, 2018 at 10:51:17AM -0500, Jason Merrill wrote:

>> On Wed, Feb 28, 2018 at 9:32 AM, Marek Polacek <polacek@redhat.com> wrote:

>> > On Tue, Feb 27, 2018 at 04:16:31PM -0500, Jason Merrill wrote:

>> >> On 02/27/2018 02:13 PM, Marek Polacek wrote:

>> >> > My recent change introducing cxx_constant_init caused this code

>> >> >

>> >> > template <class> class A {

>> >> >    static const long b = 0;

>> >> >    static const unsigned c = (b);

>> >> > };

>> >> >

>> >> > to be rejected.  The reason is that force_paren_expr turns "b" into "*(const

>> >> > long int &) &b", where the former is not value-dependent but the latter is

>> >> > value-dependent.  So when we get to maybe_constant_init_1:

>> >> > 5147   if (!is_nondependent_static_init_expression (t))

>> >> > 5148     /* Don't try to evaluate it.  */;

>> >> > it's not evaluated and we get the non-constant initialization error.

>> >> > (Before we'd always evaluated the expression.)

>> >> >

>> >> > Bootstrapped/regtested on x86_64-linux, ok for trunk?

>> >> >

>> >> > 2018-02-27  Marek Polacek  <polacek@redhat.com>

>> >> >

>> >> >     PR c++/84582

>> >> >     * semantics.c (force_paren_expr): Avoid creating a static cast

>> >> >     when processing a template.

>> >> >

>> >> >     * g++.dg/cpp1z/static1.C: New test.

>> >> >     * g++.dg/template/static37.C: New test.

>> >> >

>> >> > diff --git gcc/cp/semantics.c gcc/cp/semantics.c

>> >> > index 35569d0cb0d..b48de2df4e2 100644

>> >> > --- gcc/cp/semantics.c

>> >> > +++ gcc/cp/semantics.c

>> >> > @@ -1697,7 +1697,7 @@ force_paren_expr (tree expr)

>> >> >       expr = build1 (PAREN_EXPR, TREE_TYPE (expr), expr);

>> >> >     else if (VAR_P (expr) && DECL_HARD_REGISTER (expr))

>> >> >       /* We can't bind a hard register variable to a reference.  */;

>> >> > -  else

>> >> > +  else if (!processing_template_decl)

>> >>

>> >> Hmm, this means that we forget about the parentheses in a template.  I'm

>> >> surprised that this didn't break anything in the testsuite.  In particular,

>> >> auto-fn15.C.  I've attached an addition to auto-fn15.C to catch this issue.

>> >

>> > Thanks, you're right.  I'll use it.

>> >

>> >> Can we use PAREN_EXPR instead of the static_cast in a template?

>> >

>> > I don't think so, it would fix the issue you pointed out in auto-fn15.C but

>> > it wouldn't fix the original test.  The problem with using PAREN_EXPR in a

>> > template is that instantiate_non_dependent_expr will turn in into the

>> > static cast anyway; tsubst_copy_and_build has

>> >     case PAREN_EXPR:

>> >       RETURN (finish_parenthesized_expr (RECUR (TREE_OPERAND (t, 0))));

>> > so it calls force_paren_expr and this time we're not in a template.  And

>> > then when calling cxx_constant_init we have the same issue.

>>

>> Then maybe we need something like fold_non_dependent_expr, which

>> checks for dependency before substitution and then immediately

>> evaluates the result.

>

> I hope you meant something like this.  Further testing also revealed that

> maybe_undo_parenthesized_ref should be able to unwrap PAREN_EXPR (so that

> (fn1)(); in paren2.C is handled correctly), and that lvalue_kind should look

> into PAREN_EXPR so as to give the correct answer regarding lvalueness: we

> should accept

>

> template<typename T>

> void foo (int i)

> {

>   ++(i);

> }

>

> Apologies if I'm on the wrong track.

>

> Bootstrapped/regtested on x86_64-linux, ok for trunk?

>

> 2018-02-28  Marek Polacek  <polacek@redhat.com>

>             Jason Merrill  <jason@redhat.com>

>

>         PR c++/84582

>         * semantics.c (force_paren_expr): Avoid creating the static cast

>         when in a template.  Create a PAREN_EXPR when in a template.

>         (maybe_undo_parenthesized_ref): Unwrap PAREN_EXPR.

>         * typeck2.c (store_init_value): Call fold_non_dependent_expr instead

>         of instantiate_non_dependent_expr.

>         * tree.c (lvalue_kind): Handle PAREN_EXPR like NON_DEPENDENT_EXPR.

>

>         * g++.dg/cpp1y/auto-fn15.C: Extend testing.

>         * g++.dg/cpp1z/static1.C: New test.

>         * g++.dg/template/static37.C: New test.

>

> diff --git gcc/cp/semantics.c gcc/cp/semantics.c

> index 35569d0cb0d..722e3718a14 100644

> --- gcc/cp/semantics.c

> +++ gcc/cp/semantics.c

> @@ -1697,7 +1697,7 @@ force_paren_expr (tree expr)

>      expr = build1 (PAREN_EXPR, TREE_TYPE (expr), expr);

>    else if (VAR_P (expr) && DECL_HARD_REGISTER (expr))

>      /* We can't bind a hard register variable to a reference.  */;

> -  else

> +  else if (!processing_template_decl)

>      {

>        cp_lvalue_kind kind = lvalue_kind (expr);

>        if ((kind & ~clk_class) != clk_none)

> @@ -1713,6 +1713,8 @@ force_paren_expr (tree expr)

>             REF_PARENTHESIZED_P (expr) = true;

>         }

>      }

> +  else

> +    expr = build1 (PAREN_EXPR, TREE_TYPE (expr), expr);


There's already a branch for building PAREN_EXPR, let's just replace
its condition.

> -      value = instantiate_non_dependent_expr (value);

> +      value = fold_non_dependent_expr (value);


I was thinking that we want a parallel fold_non_dependent_init (that
hopefully shares most of the implementation).  Then we shouldn't need
the call to maybe_constant_init anymore.

Jason
Marek Polacek March 1, 2018, 1:17 p.m. | #6
On Wed, Feb 28, 2018 at 04:50:39PM -0500, Jason Merrill wrote:
> On Wed, Feb 28, 2018 at 4:19 PM, Marek Polacek <polacek@redhat.com> wrote:

> > On Wed, Feb 28, 2018 at 10:51:17AM -0500, Jason Merrill wrote:

> >> On Wed, Feb 28, 2018 at 9:32 AM, Marek Polacek <polacek@redhat.com> wrote:

> >> > On Tue, Feb 27, 2018 at 04:16:31PM -0500, Jason Merrill wrote:

> >> >> On 02/27/2018 02:13 PM, Marek Polacek wrote:

> >> >> > My recent change introducing cxx_constant_init caused this code

> >> >> >

> >> >> > template <class> class A {

> >> >> >    static const long b = 0;

> >> >> >    static const unsigned c = (b);

> >> >> > };

> >> >> >

> >> >> > to be rejected.  The reason is that force_paren_expr turns "b" into "*(const

> >> >> > long int &) &b", where the former is not value-dependent but the latter is

> >> >> > value-dependent.  So when we get to maybe_constant_init_1:

> >> >> > 5147   if (!is_nondependent_static_init_expression (t))

> >> >> > 5148     /* Don't try to evaluate it.  */;

> >> >> > it's not evaluated and we get the non-constant initialization error.

> >> >> > (Before we'd always evaluated the expression.)

> >> >> >

> >> >> > Bootstrapped/regtested on x86_64-linux, ok for trunk?

> >> >> >

> >> >> > 2018-02-27  Marek Polacek  <polacek@redhat.com>

> >> >> >

> >> >> >     PR c++/84582

> >> >> >     * semantics.c (force_paren_expr): Avoid creating a static cast

> >> >> >     when processing a template.

> >> >> >

> >> >> >     * g++.dg/cpp1z/static1.C: New test.

> >> >> >     * g++.dg/template/static37.C: New test.

> >> >> >

> >> >> > diff --git gcc/cp/semantics.c gcc/cp/semantics.c

> >> >> > index 35569d0cb0d..b48de2df4e2 100644

> >> >> > --- gcc/cp/semantics.c

> >> >> > +++ gcc/cp/semantics.c

> >> >> > @@ -1697,7 +1697,7 @@ force_paren_expr (tree expr)

> >> >> >       expr = build1 (PAREN_EXPR, TREE_TYPE (expr), expr);

> >> >> >     else if (VAR_P (expr) && DECL_HARD_REGISTER (expr))

> >> >> >       /* We can't bind a hard register variable to a reference.  */;

> >> >> > -  else

> >> >> > +  else if (!processing_template_decl)

> >> >>

> >> >> Hmm, this means that we forget about the parentheses in a template.  I'm

> >> >> surprised that this didn't break anything in the testsuite.  In particular,

> >> >> auto-fn15.C.  I've attached an addition to auto-fn15.C to catch this issue.

> >> >

> >> > Thanks, you're right.  I'll use it.

> >> >

> >> >> Can we use PAREN_EXPR instead of the static_cast in a template?

> >> >

> >> > I don't think so, it would fix the issue you pointed out in auto-fn15.C but

> >> > it wouldn't fix the original test.  The problem with using PAREN_EXPR in a

> >> > template is that instantiate_non_dependent_expr will turn in into the

> >> > static cast anyway; tsubst_copy_and_build has

> >> >     case PAREN_EXPR:

> >> >       RETURN (finish_parenthesized_expr (RECUR (TREE_OPERAND (t, 0))));

> >> > so it calls force_paren_expr and this time we're not in a template.  And

> >> > then when calling cxx_constant_init we have the same issue.

> >>

> >> Then maybe we need something like fold_non_dependent_expr, which

> >> checks for dependency before substitution and then immediately

> >> evaluates the result.

> >

> > I hope you meant something like this.  Further testing also revealed that

> > maybe_undo_parenthesized_ref should be able to unwrap PAREN_EXPR (so that

> > (fn1)(); in paren2.C is handled correctly), and that lvalue_kind should look

> > into PAREN_EXPR so as to give the correct answer regarding lvalueness: we

> > should accept

> >

> > template<typename T>

> > void foo (int i)

> > {

> >   ++(i);

> > }

> >

> > Apologies if I'm on the wrong track.

> >

> > Bootstrapped/regtested on x86_64-linux, ok for trunk?

> >

> > 2018-02-28  Marek Polacek  <polacek@redhat.com>

> >             Jason Merrill  <jason@redhat.com>

> >

> >         PR c++/84582

> >         * semantics.c (force_paren_expr): Avoid creating the static cast

> >         when in a template.  Create a PAREN_EXPR when in a template.

> >         (maybe_undo_parenthesized_ref): Unwrap PAREN_EXPR.

> >         * typeck2.c (store_init_value): Call fold_non_dependent_expr instead

> >         of instantiate_non_dependent_expr.

> >         * tree.c (lvalue_kind): Handle PAREN_EXPR like NON_DEPENDENT_EXPR.

> >

> >         * g++.dg/cpp1y/auto-fn15.C: Extend testing.

> >         * g++.dg/cpp1z/static1.C: New test.

> >         * g++.dg/template/static37.C: New test.

> >

> > diff --git gcc/cp/semantics.c gcc/cp/semantics.c

> > index 35569d0cb0d..722e3718a14 100644

> > --- gcc/cp/semantics.c

> > +++ gcc/cp/semantics.c

> > @@ -1697,7 +1697,7 @@ force_paren_expr (tree expr)

> >      expr = build1 (PAREN_EXPR, TREE_TYPE (expr), expr);

> >    else if (VAR_P (expr) && DECL_HARD_REGISTER (expr))

> >      /* We can't bind a hard register variable to a reference.  */;

> > -  else

> > +  else if (!processing_template_decl)

> >      {

> >        cp_lvalue_kind kind = lvalue_kind (expr);

> >        if ((kind & ~clk_class) != clk_none)

> > @@ -1713,6 +1713,8 @@ force_paren_expr (tree expr)

> >             REF_PARENTHESIZED_P (expr) = true;

> >         }

> >      }

> > +  else

> > +    expr = build1 (PAREN_EXPR, TREE_TYPE (expr), expr);

> 

> There's already a branch for building PAREN_EXPR, let's just replace

> its condition.


Sure.

> > -      value = instantiate_non_dependent_expr (value);

> > +      value = fold_non_dependent_expr (value);

> 

> I was thinking that we want a parallel fold_non_dependent_init (that

> hopefully shares most of the implementation).  Then we shouldn't need

> the call to maybe_constant_init anymore.


If you mean fold_non_dependent_init that would be like fold_non_dependent_expr
but with maybe_constant_init and not maybe_constant_value, then that would
break e.g.

const double d = 9.0;   // missing constexpr
constexpr double j = d; // should give error

because maybe_constant_value checks is_nondependent_constant_expression, and
"d" in the example above is not a constant expression, so we don't evaluate,
and "d" stays "d", so require_constant_expression gives the error.  On the
other hand, maybe_constant_init checks is_nondependent_static_init_expression,
and "d" is that, so we evaluate "d" to "9.0".  Then require_constant_expression
doesn't complain.

What problem do you see with using fold_non_dependent_expr?

Thanks,

	Marek
Jason Merrill March 1, 2018, 6:56 p.m. | #7
On Thu, Mar 1, 2018 at 8:17 AM, Marek Polacek <polacek@redhat.com> wrote:
> On Wed, Feb 28, 2018 at 04:50:39PM -0500, Jason Merrill wrote:

>> On Wed, Feb 28, 2018 at 4:19 PM, Marek Polacek <polacek@redhat.com> wrote:

>> > On Wed, Feb 28, 2018 at 10:51:17AM -0500, Jason Merrill wrote:

>> >> On Wed, Feb 28, 2018 at 9:32 AM, Marek Polacek <polacek@redhat.com> wrote:

>> >> > On Tue, Feb 27, 2018 at 04:16:31PM -0500, Jason Merrill wrote:

>> >> >> On 02/27/2018 02:13 PM, Marek Polacek wrote:

>> >> >> > My recent change introducing cxx_constant_init caused this code

>> >> >> >

>> >> >> > template <class> class A {

>> >> >> >    static const long b = 0;

>> >> >> >    static const unsigned c = (b);

>> >> >> > };

>> >> >> >

>> >> >> > to be rejected.  The reason is that force_paren_expr turns "b" into "*(const

>> >> >> > long int &) &b", where the former is not value-dependent but the latter is

>> >> >> > value-dependent.  So when we get to maybe_constant_init_1:

>> >> >> > 5147   if (!is_nondependent_static_init_expression (t))

>> >> >> > 5148     /* Don't try to evaluate it.  */;

>> >> >> > it's not evaluated and we get the non-constant initialization error.

>> >> >> > (Before we'd always evaluated the expression.)

>> >> >> >

>> >> >> > Bootstrapped/regtested on x86_64-linux, ok for trunk?

>> >> >> >

>> >> >> > 2018-02-27  Marek Polacek  <polacek@redhat.com>

>> >> >> >

>> >> >> >     PR c++/84582

>> >> >> >     * semantics.c (force_paren_expr): Avoid creating a static cast

>> >> >> >     when processing a template.

>> >> >> >

>> >> >> >     * g++.dg/cpp1z/static1.C: New test.

>> >> >> >     * g++.dg/template/static37.C: New test.

>> >> >> >

>> >> >> > diff --git gcc/cp/semantics.c gcc/cp/semantics.c

>> >> >> > index 35569d0cb0d..b48de2df4e2 100644

>> >> >> > --- gcc/cp/semantics.c

>> >> >> > +++ gcc/cp/semantics.c

>> >> >> > @@ -1697,7 +1697,7 @@ force_paren_expr (tree expr)

>> >> >> >       expr = build1 (PAREN_EXPR, TREE_TYPE (expr), expr);

>> >> >> >     else if (VAR_P (expr) && DECL_HARD_REGISTER (expr))

>> >> >> >       /* We can't bind a hard register variable to a reference.  */;

>> >> >> > -  else

>> >> >> > +  else if (!processing_template_decl)

>> >> >>

>> >> >> Hmm, this means that we forget about the parentheses in a template.  I'm

>> >> >> surprised that this didn't break anything in the testsuite.  In particular,

>> >> >> auto-fn15.C.  I've attached an addition to auto-fn15.C to catch this issue.

>> >> >

>> >> > Thanks, you're right.  I'll use it.

>> >> >

>> >> >> Can we use PAREN_EXPR instead of the static_cast in a template?

>> >> >

>> >> > I don't think so, it would fix the issue you pointed out in auto-fn15.C but

>> >> > it wouldn't fix the original test.  The problem with using PAREN_EXPR in a

>> >> > template is that instantiate_non_dependent_expr will turn in into the

>> >> > static cast anyway; tsubst_copy_and_build has

>> >> >     case PAREN_EXPR:

>> >> >       RETURN (finish_parenthesized_expr (RECUR (TREE_OPERAND (t, 0))));

>> >> > so it calls force_paren_expr and this time we're not in a template.  And

>> >> > then when calling cxx_constant_init we have the same issue.

>> >>

>> >> Then maybe we need something like fold_non_dependent_expr, which

>> >> checks for dependency before substitution and then immediately

>> >> evaluates the result.

>> >

>> > I hope you meant something like this.  Further testing also revealed that

>> > maybe_undo_parenthesized_ref should be able to unwrap PAREN_EXPR (so that

>> > (fn1)(); in paren2.C is handled correctly), and that lvalue_kind should look

>> > into PAREN_EXPR so as to give the correct answer regarding lvalueness: we

>> > should accept

>> >

>> > template<typename T>

>> > void foo (int i)

>> > {

>> >   ++(i);

>> > }

>> >

>> > Apologies if I'm on the wrong track.

>> >

>> > Bootstrapped/regtested on x86_64-linux, ok for trunk?

>> >

>> > 2018-02-28  Marek Polacek  <polacek@redhat.com>

>> >             Jason Merrill  <jason@redhat.com>

>> >

>> >         PR c++/84582

>> >         * semantics.c (force_paren_expr): Avoid creating the static cast

>> >         when in a template.  Create a PAREN_EXPR when in a template.

>> >         (maybe_undo_parenthesized_ref): Unwrap PAREN_EXPR.

>> >         * typeck2.c (store_init_value): Call fold_non_dependent_expr instead

>> >         of instantiate_non_dependent_expr.

>> >         * tree.c (lvalue_kind): Handle PAREN_EXPR like NON_DEPENDENT_EXPR.

>> >

>> >         * g++.dg/cpp1y/auto-fn15.C: Extend testing.

>> >         * g++.dg/cpp1z/static1.C: New test.

>> >         * g++.dg/template/static37.C: New test.

>> >

>> > diff --git gcc/cp/semantics.c gcc/cp/semantics.c

>> > index 35569d0cb0d..722e3718a14 100644

>> > --- gcc/cp/semantics.c

>> > +++ gcc/cp/semantics.c

>> > @@ -1697,7 +1697,7 @@ force_paren_expr (tree expr)

>> >      expr = build1 (PAREN_EXPR, TREE_TYPE (expr), expr);

>> >    else if (VAR_P (expr) && DECL_HARD_REGISTER (expr))

>> >      /* We can't bind a hard register variable to a reference.  */;

>> > -  else

>> > +  else if (!processing_template_decl)

>> >      {

>> >        cp_lvalue_kind kind = lvalue_kind (expr);

>> >        if ((kind & ~clk_class) != clk_none)

>> > @@ -1713,6 +1713,8 @@ force_paren_expr (tree expr)

>> >             REF_PARENTHESIZED_P (expr) = true;

>> >         }

>> >      }

>> > +  else

>> > +    expr = build1 (PAREN_EXPR, TREE_TYPE (expr), expr);

>>

>> There's already a branch for building PAREN_EXPR, let's just replace

>> its condition.

>

> Sure.

>

>> > -      value = instantiate_non_dependent_expr (value);

>> > +      value = fold_non_dependent_expr (value);

>>

>> I was thinking that we want a parallel fold_non_dependent_init (that

>> hopefully shares most of the implementation).  Then we shouldn't need

>> the call to maybe_constant_init anymore.

>

> If you mean fold_non_dependent_init that would be like fold_non_dependent_expr

> but with maybe_constant_init and not maybe_constant_value


And is_nondependent_static_init_expression, and different arguments to
cxx_eval_outermost_constant_expression, yes.

> then that would break e.g.

>

> const double d = 9.0;   // missing constexpr

> constexpr double j = d; // should give error

>

> because maybe_constant_value checks is_nondependent_constant_expression, and

> "d" in the example above is not a constant expression, so we don't evaluate,

> and "d" stays "d", so require_constant_expression gives the error.  On the

> other hand, maybe_constant_init checks is_nondependent_static_init_expression,

> and "d" is that, so we evaluate "d" to "9.0".  Then require_constant_expression

> doesn't complain.


Ah, I see.  You're right, let's stick with fold_non_dependent_expr.

Jason
Marek Polacek March 1, 2018, 9:40 p.m. | #8
On Thu, Mar 01, 2018 at 01:56:50PM -0500, Jason Merrill wrote:
> On Thu, Mar 1, 2018 at 8:17 AM, Marek Polacek <polacek@redhat.com> wrote:

> > On Wed, Feb 28, 2018 at 04:50:39PM -0500, Jason Merrill wrote:

> >> On Wed, Feb 28, 2018 at 4:19 PM, Marek Polacek <polacek@redhat.com> wrote:

> >> > On Wed, Feb 28, 2018 at 10:51:17AM -0500, Jason Merrill wrote:

> >> >> On Wed, Feb 28, 2018 at 9:32 AM, Marek Polacek <polacek@redhat.com> wrote:

> >> >> > On Tue, Feb 27, 2018 at 04:16:31PM -0500, Jason Merrill wrote:

> >> >> >> On 02/27/2018 02:13 PM, Marek Polacek wrote:

> >> >> >> > My recent change introducing cxx_constant_init caused this code

> >> >> >> >

> >> >> >> > template <class> class A {

> >> >> >> >    static const long b = 0;

> >> >> >> >    static const unsigned c = (b);

> >> >> >> > };

> >> >> >> >

> >> >> >> > to be rejected.  The reason is that force_paren_expr turns "b" into "*(const

> >> >> >> > long int &) &b", where the former is not value-dependent but the latter is

> >> >> >> > value-dependent.  So when we get to maybe_constant_init_1:

> >> >> >> > 5147   if (!is_nondependent_static_init_expression (t))

> >> >> >> > 5148     /* Don't try to evaluate it.  */;

> >> >> >> > it's not evaluated and we get the non-constant initialization error.

> >> >> >> > (Before we'd always evaluated the expression.)

> >> >> >> >

> >> >> >> > Bootstrapped/regtested on x86_64-linux, ok for trunk?

> >> >> >> >

> >> >> >> > 2018-02-27  Marek Polacek  <polacek@redhat.com>

> >> >> >> >

> >> >> >> >     PR c++/84582

> >> >> >> >     * semantics.c (force_paren_expr): Avoid creating a static cast

> >> >> >> >     when processing a template.

> >> >> >> >

> >> >> >> >     * g++.dg/cpp1z/static1.C: New test.

> >> >> >> >     * g++.dg/template/static37.C: New test.

> >> >> >> >

> >> >> >> > diff --git gcc/cp/semantics.c gcc/cp/semantics.c

> >> >> >> > index 35569d0cb0d..b48de2df4e2 100644

> >> >> >> > --- gcc/cp/semantics.c

> >> >> >> > +++ gcc/cp/semantics.c

> >> >> >> > @@ -1697,7 +1697,7 @@ force_paren_expr (tree expr)

> >> >> >> >       expr = build1 (PAREN_EXPR, TREE_TYPE (expr), expr);

> >> >> >> >     else if (VAR_P (expr) && DECL_HARD_REGISTER (expr))

> >> >> >> >       /* We can't bind a hard register variable to a reference.  */;

> >> >> >> > -  else

> >> >> >> > +  else if (!processing_template_decl)

> >> >> >>

> >> >> >> Hmm, this means that we forget about the parentheses in a template.  I'm

> >> >> >> surprised that this didn't break anything in the testsuite.  In particular,

> >> >> >> auto-fn15.C.  I've attached an addition to auto-fn15.C to catch this issue.

> >> >> >

> >> >> > Thanks, you're right.  I'll use it.

> >> >> >

> >> >> >> Can we use PAREN_EXPR instead of the static_cast in a template?

> >> >> >

> >> >> > I don't think so, it would fix the issue you pointed out in auto-fn15.C but

> >> >> > it wouldn't fix the original test.  The problem with using PAREN_EXPR in a

> >> >> > template is that instantiate_non_dependent_expr will turn in into the

> >> >> > static cast anyway; tsubst_copy_and_build has

> >> >> >     case PAREN_EXPR:

> >> >> >       RETURN (finish_parenthesized_expr (RECUR (TREE_OPERAND (t, 0))));

> >> >> > so it calls force_paren_expr and this time we're not in a template.  And

> >> >> > then when calling cxx_constant_init we have the same issue.

> >> >>

> >> >> Then maybe we need something like fold_non_dependent_expr, which

> >> >> checks for dependency before substitution and then immediately

> >> >> evaluates the result.

> >> >

> >> > I hope you meant something like this.  Further testing also revealed that

> >> > maybe_undo_parenthesized_ref should be able to unwrap PAREN_EXPR (so that

> >> > (fn1)(); in paren2.C is handled correctly), and that lvalue_kind should look

> >> > into PAREN_EXPR so as to give the correct answer regarding lvalueness: we

> >> > should accept

> >> >

> >> > template<typename T>

> >> > void foo (int i)

> >> > {

> >> >   ++(i);

> >> > }

> >> >

> >> > Apologies if I'm on the wrong track.

> >> >

> >> > Bootstrapped/regtested on x86_64-linux, ok for trunk?

> >> >

> >> > 2018-02-28  Marek Polacek  <polacek@redhat.com>

> >> >             Jason Merrill  <jason@redhat.com>

> >> >

> >> >         PR c++/84582

> >> >         * semantics.c (force_paren_expr): Avoid creating the static cast

> >> >         when in a template.  Create a PAREN_EXPR when in a template.

> >> >         (maybe_undo_parenthesized_ref): Unwrap PAREN_EXPR.

> >> >         * typeck2.c (store_init_value): Call fold_non_dependent_expr instead

> >> >         of instantiate_non_dependent_expr.

> >> >         * tree.c (lvalue_kind): Handle PAREN_EXPR like NON_DEPENDENT_EXPR.

> >> >

> >> >         * g++.dg/cpp1y/auto-fn15.C: Extend testing.

> >> >         * g++.dg/cpp1z/static1.C: New test.

> >> >         * g++.dg/template/static37.C: New test.

> >> >

> >> > diff --git gcc/cp/semantics.c gcc/cp/semantics.c

> >> > index 35569d0cb0d..722e3718a14 100644

> >> > --- gcc/cp/semantics.c

> >> > +++ gcc/cp/semantics.c

> >> > @@ -1697,7 +1697,7 @@ force_paren_expr (tree expr)

> >> >      expr = build1 (PAREN_EXPR, TREE_TYPE (expr), expr);

> >> >    else if (VAR_P (expr) && DECL_HARD_REGISTER (expr))

> >> >      /* We can't bind a hard register variable to a reference.  */;

> >> > -  else

> >> > +  else if (!processing_template_decl)

> >> >      {

> >> >        cp_lvalue_kind kind = lvalue_kind (expr);

> >> >        if ((kind & ~clk_class) != clk_none)

> >> > @@ -1713,6 +1713,8 @@ force_paren_expr (tree expr)

> >> >             REF_PARENTHESIZED_P (expr) = true;

> >> >         }

> >> >      }

> >> > +  else

> >> > +    expr = build1 (PAREN_EXPR, TREE_TYPE (expr), expr);

> >>

> >> There's already a branch for building PAREN_EXPR, let's just replace

> >> its condition.

> >

> > Sure.

> >

> >> > -      value = instantiate_non_dependent_expr (value);

> >> > +      value = fold_non_dependent_expr (value);

> >>

> >> I was thinking that we want a parallel fold_non_dependent_init (that

> >> hopefully shares most of the implementation).  Then we shouldn't need

> >> the call to maybe_constant_init anymore.

> >

> > If you mean fold_non_dependent_init that would be like fold_non_dependent_expr

> > but with maybe_constant_init and not maybe_constant_value

> 

> And is_nondependent_static_init_expression, and different arguments to

> cxx_eval_outermost_constant_expression, yes.


Ah.  Maybe it'll be useful sometime in the future.

> > then that would break e.g.

> >

> > const double d = 9.0;   // missing constexpr

> > constexpr double j = d; // should give error

> >

> > because maybe_constant_value checks is_nondependent_constant_expression, and

> > "d" in the example above is not a constant expression, so we don't evaluate,

> > and "d" stays "d", so require_constant_expression gives the error.  On the

> > other hand, maybe_constant_init checks is_nondependent_static_init_expression,

> > and "d" is that, so we evaluate "d" to "9.0".  Then require_constant_expression

> > doesn't complain.

> 

> Ah, I see.  You're right, let's stick with fold_non_dependent_expr.


Thanks, so this is the final patch then:

Bootstrapped/regtested on x86_64-linux, ok for trunk?

2018-03-01  Marek Polacek  <polacek@redhat.com>
	    Jason Merrill  <jason@redhat.com>

	PR c++/84582
	* semantics.c (force_paren_expr): Create a PAREN_EXPR when in
	a template.
	(maybe_undo_parenthesized_ref): Unwrap PAREN_EXPR.
	* typeck2.c (store_init_value): Call fold_non_dependent_expr instead
	of instantiate_non_dependent_expr.
	* tree.c (lvalue_kind): Handle PAREN_EXPR like NON_DEPENDENT_EXPR.

	* g++.dg/cpp1y/auto-fn15.C: Extend testing.
	* g++.dg/cpp1z/static1.C: New test.
	* g++.dg/template/static37.C: New test.

diff --git gcc/cp/semantics.c gcc/cp/semantics.c
index 87c5c669a55..1ac1d23e761 100644
--- gcc/cp/semantics.c
+++ gcc/cp/semantics.c
@@ -1693,7 +1693,8 @@ force_paren_expr (tree expr)
   if (TREE_CODE (expr) == COMPONENT_REF
       || TREE_CODE (expr) == SCOPE_REF)
     REF_PARENTHESIZED_P (expr) = true;
-  else if (type_dependent_expression_p (expr))
+  else if (type_dependent_expression_p (expr)
+	   || processing_template_decl)
     expr = build1 (PAREN_EXPR, TREE_TYPE (expr), expr);
   else if (VAR_P (expr) && DECL_HARD_REGISTER (expr))
     /* We can't bind a hard register variable to a reference.  */;
@@ -1724,9 +1725,10 @@ force_paren_expr (tree expr)
 tree
 maybe_undo_parenthesized_ref (tree t)
 {
-  if (cxx_dialect >= cxx14
-      && INDIRECT_REF_P (t)
-      && REF_PARENTHESIZED_P (t))
+  if (cxx_dialect < cxx14)
+    return t;
+
+  if (INDIRECT_REF_P (t) && REF_PARENTHESIZED_P (t))
     {
       t = TREE_OPERAND (t, 0);
       while (TREE_CODE (t) == NON_LVALUE_EXPR
@@ -1737,6 +1739,8 @@ maybe_undo_parenthesized_ref (tree t)
 		  || TREE_CODE (t) == STATIC_CAST_EXPR);
       t = TREE_OPERAND (t, 0);
     }
+  else if (TREE_CODE (t) == PAREN_EXPR)
+    t = TREE_OPERAND (t, 0);
 
   return t;
 }
diff --git gcc/cp/tree.c gcc/cp/tree.c
index 9b9e36a1173..19f1c0629c9 100644
--- gcc/cp/tree.c
+++ gcc/cp/tree.c
@@ -239,6 +239,7 @@ lvalue_kind (const_tree ref)
       return lvalue_kind (BASELINK_FUNCTIONS (CONST_CAST_TREE (ref)));
 
     case NON_DEPENDENT_EXPR:
+    case PAREN_EXPR:
       return lvalue_kind (TREE_OPERAND (ref, 0));
 
     case VIEW_CONVERT_EXPR:
diff --git gcc/cp/typeck2.c gcc/cp/typeck2.c
index 153b46cca77..583c65d4d0a 100644
--- gcc/cp/typeck2.c
+++ gcc/cp/typeck2.c
@@ -822,7 +822,7 @@ store_init_value (tree decl, tree init, vec<tree, va_gc>** cleanups, int flags)
   if (decl_maybe_constant_var_p (decl) || TREE_STATIC (decl))
     {
       bool const_init;
-      value = instantiate_non_dependent_expr (value);
+      value = fold_non_dependent_expr (value);
       if (DECL_DECLARED_CONSTEXPR_P (decl)
 	  || (DECL_IN_AGGR_P (decl) && !DECL_VAR_DECLARED_INLINE_P (decl)))
 	{
diff --git gcc/testsuite/g++.dg/cpp1y/auto-fn15.C gcc/testsuite/g++.dg/cpp1y/auto-fn15.C
index ba9f3579f62..0db428f7270 100644
--- gcc/testsuite/g++.dg/cpp1y/auto-fn15.C
+++ gcc/testsuite/g++.dg/cpp1y/auto-fn15.C
@@ -22,6 +22,8 @@ template <class T>
 decltype(auto) h5(T t) { return t.i; }
 template <class T>
 decltype(auto) h6(T t) { return (t.i); }
+template <class T>
+decltype(auto) h7(T t) { return (i); }
 
 int main()
 {
@@ -48,4 +50,5 @@ int main()
   same_type<decltype(h4()),int&>();
   same_type<decltype(h5(a)),int>();
   same_type<decltype(h6(a)),int&>();
+  same_type<decltype(h7(a)),int&>();
 }
diff --git gcc/testsuite/g++.dg/cpp1z/static1.C gcc/testsuite/g++.dg/cpp1z/static1.C
index e69de29bb2d..cb872997c5a 100644
--- gcc/testsuite/g++.dg/cpp1z/static1.C
+++ gcc/testsuite/g++.dg/cpp1z/static1.C
@@ -0,0 +1,19 @@
+// PR c++/84582
+// { dg-options -std=c++17 }
+
+class C {
+  static inline const long b = 0;
+  static inline const unsigned c = (b);
+};
+class D {
+  static inline const long b = 0;
+  static inline const unsigned c = b;
+};
+template <class> class A {
+  static inline const long b = 0;
+  static inline const unsigned c = (b);
+};
+template <class> class B {
+  static inline const long b = 0;
+  static inline const unsigned c = b;
+};
diff --git gcc/testsuite/g++.dg/template/static37.C gcc/testsuite/g++.dg/template/static37.C
index e69de29bb2d..90bc65d2fbc 100644
--- gcc/testsuite/g++.dg/template/static37.C
+++ gcc/testsuite/g++.dg/template/static37.C
@@ -0,0 +1,18 @@
+// PR c++/84582
+
+class C {
+  static const long b = 0;
+  static const unsigned c = (b);
+};
+class D {
+  static const long b = 0;
+  static const unsigned c = b;
+};
+template <class> class A {
+  static const long b = 0;
+  static const unsigned c = (b);
+};
+template <class> class B {
+  static const long b = 0;
+  static const unsigned c = b;
+};

	Marek
Jason Merrill March 1, 2018, 9:57 p.m. | #9
Ok.

On Mar 1, 2018 4:40 PM, "Marek Polacek" <polacek@redhat.com> wrote:

> On Thu, Mar 01, 2018 at 01:56:50PM -0500, Jason Merrill wrote:

> > On Thu, Mar 1, 2018 at 8:17 AM, Marek Polacek <polacek@redhat.com>

> wrote:

> > > On Wed, Feb 28, 2018 at 04:50:39PM -0500, Jason Merrill wrote:

> > >> On Wed, Feb 28, 2018 at 4:19 PM, Marek Polacek <polacek@redhat.com>

> wrote:

> > >> > On Wed, Feb 28, 2018 at 10:51:17AM -0500, Jason Merrill wrote:

> > >> >> On Wed, Feb 28, 2018 at 9:32 AM, Marek Polacek <polacek@redhat.com>

> wrote:

> > >> >> > On Tue, Feb 27, 2018 at 04:16:31PM -0500, Jason Merrill wrote:

> > >> >> >> On 02/27/2018 02:13 PM, Marek Polacek wrote:

> > >> >> >> > My recent change introducing cxx_constant_init caused this

> code

> > >> >> >> >

> > >> >> >> > template <class> class A {

> > >> >> >> >    static const long b = 0;

> > >> >> >> >    static const unsigned c = (b);

> > >> >> >> > };

> > >> >> >> >

> > >> >> >> > to be rejected.  The reason is that force_paren_expr turns

> "b" into "*(const

> > >> >> >> > long int &) &b", where the former is not value-dependent but

> the latter is

> > >> >> >> > value-dependent.  So when we get to maybe_constant_init_1:

> > >> >> >> > 5147   if (!is_nondependent_static_init_expression (t))

> > >> >> >> > 5148     /* Don't try to evaluate it.  */;

> > >> >> >> > it's not evaluated and we get the non-constant initialization

> error.

> > >> >> >> > (Before we'd always evaluated the expression.)

> > >> >> >> >

> > >> >> >> > Bootstrapped/regtested on x86_64-linux, ok for trunk?

> > >> >> >> >

> > >> >> >> > 2018-02-27  Marek Polacek  <polacek@redhat.com>

> > >> >> >> >

> > >> >> >> >     PR c++/84582

> > >> >> >> >     * semantics.c (force_paren_expr): Avoid creating a static

> cast

> > >> >> >> >     when processing a template.

> > >> >> >> >

> > >> >> >> >     * g++.dg/cpp1z/static1.C: New test.

> > >> >> >> >     * g++.dg/template/static37.C: New test.

> > >> >> >> >

> > >> >> >> > diff --git gcc/cp/semantics.c gcc/cp/semantics.c

> > >> >> >> > index 35569d0cb0d..b48de2df4e2 100644

> > >> >> >> > --- gcc/cp/semantics.c

> > >> >> >> > +++ gcc/cp/semantics.c

> > >> >> >> > @@ -1697,7 +1697,7 @@ force_paren_expr (tree expr)

> > >> >> >> >       expr = build1 (PAREN_EXPR, TREE_TYPE (expr), expr);

> > >> >> >> >     else if (VAR_P (expr) && DECL_HARD_REGISTER (expr))

> > >> >> >> >       /* We can't bind a hard register variable to a

> reference.  */;

> > >> >> >> > -  else

> > >> >> >> > +  else if (!processing_template_decl)

> > >> >> >>

> > >> >> >> Hmm, this means that we forget about the parentheses in a

> template.  I'm

> > >> >> >> surprised that this didn't break anything in the testsuite.  In

> particular,

> > >> >> >> auto-fn15.C.  I've attached an addition to auto-fn15.C to catch

> this issue.

> > >> >> >

> > >> >> > Thanks, you're right.  I'll use it.

> > >> >> >

> > >> >> >> Can we use PAREN_EXPR instead of the static_cast in a template?

> > >> >> >

> > >> >> > I don't think so, it would fix the issue you pointed out in

> auto-fn15.C but

> > >> >> > it wouldn't fix the original test.  The problem with using

> PAREN_EXPR in a

> > >> >> > template is that instantiate_non_dependent_expr will turn in

> into the

> > >> >> > static cast anyway; tsubst_copy_and_build has

> > >> >> >     case PAREN_EXPR:

> > >> >> >       RETURN (finish_parenthesized_expr (RECUR (TREE_OPERAND (t,

> 0))));

> > >> >> > so it calls force_paren_expr and this time we're not in a

> template.  And

> > >> >> > then when calling cxx_constant_init we have the same issue.

> > >> >>

> > >> >> Then maybe we need something like fold_non_dependent_expr, which

> > >> >> checks for dependency before substitution and then immediately

> > >> >> evaluates the result.

> > >> >

> > >> > I hope you meant something like this.  Further testing also

> revealed that

> > >> > maybe_undo_parenthesized_ref should be able to unwrap PAREN_EXPR

> (so that

> > >> > (fn1)(); in paren2.C is handled correctly), and that lvalue_kind

> should look

> > >> > into PAREN_EXPR so as to give the correct answer regarding

> lvalueness: we

> > >> > should accept

> > >> >

> > >> > template<typename T>

> > >> > void foo (int i)

> > >> > {

> > >> >   ++(i);

> > >> > }

> > >> >

> > >> > Apologies if I'm on the wrong track.

> > >> >

> > >> > Bootstrapped/regtested on x86_64-linux, ok for trunk?

> > >> >

> > >> > 2018-02-28  Marek Polacek  <polacek@redhat.com>

> > >> >             Jason Merrill  <jason@redhat.com>

> > >> >

> > >> >         PR c++/84582

> > >> >         * semantics.c (force_paren_expr): Avoid creating the static

> cast

> > >> >         when in a template.  Create a PAREN_EXPR when in a template.

> > >> >         (maybe_undo_parenthesized_ref): Unwrap PAREN_EXPR.

> > >> >         * typeck2.c (store_init_value): Call

> fold_non_dependent_expr instead

> > >> >         of instantiate_non_dependent_expr.

> > >> >         * tree.c (lvalue_kind): Handle PAREN_EXPR like

> NON_DEPENDENT_EXPR.

> > >> >

> > >> >         * g++.dg/cpp1y/auto-fn15.C: Extend testing.

> > >> >         * g++.dg/cpp1z/static1.C: New test.

> > >> >         * g++.dg/template/static37.C: New test.

> > >> >

> > >> > diff --git gcc/cp/semantics.c gcc/cp/semantics.c

> > >> > index 35569d0cb0d..722e3718a14 100644

> > >> > --- gcc/cp/semantics.c

> > >> > +++ gcc/cp/semantics.c

> > >> > @@ -1697,7 +1697,7 @@ force_paren_expr (tree expr)

> > >> >      expr = build1 (PAREN_EXPR, TREE_TYPE (expr), expr);

> > >> >    else if (VAR_P (expr) && DECL_HARD_REGISTER (expr))

> > >> >      /* We can't bind a hard register variable to a reference.  */;

> > >> > -  else

> > >> > +  else if (!processing_template_decl)

> > >> >      {

> > >> >        cp_lvalue_kind kind = lvalue_kind (expr);

> > >> >        if ((kind & ~clk_class) != clk_none)

> > >> > @@ -1713,6 +1713,8 @@ force_paren_expr (tree expr)

> > >> >             REF_PARENTHESIZED_P (expr) = true;

> > >> >         }

> > >> >      }

> > >> > +  else

> > >> > +    expr = build1 (PAREN_EXPR, TREE_TYPE (expr), expr);

> > >>

> > >> There's already a branch for building PAREN_EXPR, let's just replace

> > >> its condition.

> > >

> > > Sure.

> > >

> > >> > -      value = instantiate_non_dependent_expr (value);

> > >> > +      value = fold_non_dependent_expr (value);

> > >>

> > >> I was thinking that we want a parallel fold_non_dependent_init (that

> > >> hopefully shares most of the implementation).  Then we shouldn't need

> > >> the call to maybe_constant_init anymore.

> > >

> > > If you mean fold_non_dependent_init that would be like

> fold_non_dependent_expr

> > > but with maybe_constant_init and not maybe_constant_value

> >

> > And is_nondependent_static_init_expression, and different arguments to

> > cxx_eval_outermost_constant_expression, yes.

>

> Ah.  Maybe it'll be useful sometime in the future.

>

> > > then that would break e.g.

> > >

> > > const double d = 9.0;   // missing constexpr

> > > constexpr double j = d; // should give error

> > >

> > > because maybe_constant_value checks is_nondependent_constant_expression,

> and

> > > "d" in the example above is not a constant expression, so we don't

> evaluate,

> > > and "d" stays "d", so require_constant_expression gives the error.  On

> the

> > > other hand, maybe_constant_init checks is_nondependent_static_init_

> expression,

> > > and "d" is that, so we evaluate "d" to "9.0".  Then

> require_constant_expression

> > > doesn't complain.

> >

> > Ah, I see.  You're right, let's stick with fold_non_dependent_expr.

>

> Thanks, so this is the final patch then:

>

> Bootstrapped/regtested on x86_64-linux, ok for trunk?

>

> 2018-03-01  Marek Polacek  <polacek@redhat.com>

>             Jason Merrill  <jason@redhat.com>

>

>         PR c++/84582

>         * semantics.c (force_paren_expr): Create a PAREN_EXPR when in

>         a template.

>         (maybe_undo_parenthesized_ref): Unwrap PAREN_EXPR.

>         * typeck2.c (store_init_value): Call fold_non_dependent_expr

> instead

>         of instantiate_non_dependent_expr.

>         * tree.c (lvalue_kind): Handle PAREN_EXPR like NON_DEPENDENT_EXPR.

>

>         * g++.dg/cpp1y/auto-fn15.C: Extend testing.

>         * g++.dg/cpp1z/static1.C: New test.

>         * g++.dg/template/static37.C: New test.

>

> diff --git gcc/cp/semantics.c gcc/cp/semantics.c

> index 87c5c669a55..1ac1d23e761 100644

> --- gcc/cp/semantics.c

> +++ gcc/cp/semantics.c

> @@ -1693,7 +1693,8 @@ force_paren_expr (tree expr)

>    if (TREE_CODE (expr) == COMPONENT_REF

>        || TREE_CODE (expr) == SCOPE_REF)

>      REF_PARENTHESIZED_P (expr) = true;

> -  else if (type_dependent_expression_p (expr))

> +  else if (type_dependent_expression_p (expr)

> +          || processing_template_decl)

>      expr = build1 (PAREN_EXPR, TREE_TYPE (expr), expr);

>    else if (VAR_P (expr) && DECL_HARD_REGISTER (expr))

>      /* We can't bind a hard register variable to a reference.  */;

> @@ -1724,9 +1725,10 @@ force_paren_expr (tree expr)

>  tree

>  maybe_undo_parenthesized_ref (tree t)

>  {

> -  if (cxx_dialect >= cxx14

> -      && INDIRECT_REF_P (t)

> -      && REF_PARENTHESIZED_P (t))

> +  if (cxx_dialect < cxx14)

> +    return t;

> +

> +  if (INDIRECT_REF_P (t) && REF_PARENTHESIZED_P (t))

>      {

>        t = TREE_OPERAND (t, 0);

>        while (TREE_CODE (t) == NON_LVALUE_EXPR

> @@ -1737,6 +1739,8 @@ maybe_undo_parenthesized_ref (tree t)

>                   || TREE_CODE (t) == STATIC_CAST_EXPR);

>        t = TREE_OPERAND (t, 0);

>      }

> +  else if (TREE_CODE (t) == PAREN_EXPR)

> +    t = TREE_OPERAND (t, 0);

>

>    return t;

>  }

> diff --git gcc/cp/tree.c gcc/cp/tree.c

> index 9b9e36a1173..19f1c0629c9 100644

> --- gcc/cp/tree.c

> +++ gcc/cp/tree.c

> @@ -239,6 +239,7 @@ lvalue_kind (const_tree ref)

>        return lvalue_kind (BASELINK_FUNCTIONS (CONST_CAST_TREE (ref)));

>

>      case NON_DEPENDENT_EXPR:

> +    case PAREN_EXPR:

>        return lvalue_kind (TREE_OPERAND (ref, 0));

>

>      case VIEW_CONVERT_EXPR:

> diff --git gcc/cp/typeck2.c gcc/cp/typeck2.c

> index 153b46cca77..583c65d4d0a 100644

> --- gcc/cp/typeck2.c

> +++ gcc/cp/typeck2.c

> @@ -822,7 +822,7 @@ store_init_value (tree decl, tree init, vec<tree,

> va_gc>** cleanups, int flags)

>    if (decl_maybe_constant_var_p (decl) || TREE_STATIC (decl))

>      {

>        bool const_init;

> -      value = instantiate_non_dependent_expr (value);

> +      value = fold_non_dependent_expr (value);

>        if (DECL_DECLARED_CONSTEXPR_P (decl)

>           || (DECL_IN_AGGR_P (decl) && !DECL_VAR_DECLARED_INLINE_P (decl)))

>         {

> diff --git gcc/testsuite/g++.dg/cpp1y/auto-fn15.C

> gcc/testsuite/g++.dg/cpp1y/auto-fn15.C

> index ba9f3579f62..0db428f7270 100644

> --- gcc/testsuite/g++.dg/cpp1y/auto-fn15.C

> +++ gcc/testsuite/g++.dg/cpp1y/auto-fn15.C

> @@ -22,6 +22,8 @@ template <class T>

>  decltype(auto) h5(T t) { return t.i; }

>  template <class T>

>  decltype(auto) h6(T t) { return (t.i); }

> +template <class T>

> +decltype(auto) h7(T t) { return (i); }

>

>  int main()

>  {

> @@ -48,4 +50,5 @@ int main()

>    same_type<decltype(h4()),int&>();

>    same_type<decltype(h5(a)),int>();

>    same_type<decltype(h6(a)),int&>();

> +  same_type<decltype(h7(a)),int&>();

>  }

> diff --git gcc/testsuite/g++.dg/cpp1z/static1.C

> gcc/testsuite/g++.dg/cpp1z/static1.C

> index e69de29bb2d..cb872997c5a 100644

> --- gcc/testsuite/g++.dg/cpp1z/static1.C

> +++ gcc/testsuite/g++.dg/cpp1z/static1.C

> @@ -0,0 +1,19 @@

> +// PR c++/84582

> +// { dg-options -std=c++17 }

> +

> +class C {

> +  static inline const long b = 0;

> +  static inline const unsigned c = (b);

> +};

> +class D {

> +  static inline const long b = 0;

> +  static inline const unsigned c = b;

> +};

> +template <class> class A {

> +  static inline const long b = 0;

> +  static inline const unsigned c = (b);

> +};

> +template <class> class B {

> +  static inline const long b = 0;

> +  static inline const unsigned c = b;

> +};

> diff --git gcc/testsuite/g++.dg/template/static37.C

> gcc/testsuite/g++.dg/template/static37.C

> index e69de29bb2d..90bc65d2fbc 100644

> --- gcc/testsuite/g++.dg/template/static37.C

> +++ gcc/testsuite/g++.dg/template/static37.C

> @@ -0,0 +1,18 @@

> +// PR c++/84582

> +

> +class C {

> +  static const long b = 0;

> +  static const unsigned c = (b);

> +};

> +class D {

> +  static const long b = 0;

> +  static const unsigned c = b;

> +};

> +template <class> class A {

> +  static const long b = 0;

> +  static const unsigned c = (b);

> +};

> +template <class> class B {

> +  static const long b = 0;

> +  static const unsigned c = b;

> +};

>

>         Marek

>
Jason Merrill March 2, 2018, 6:17 p.m. | #10
On Mar 1, 2018 4:57 PM, "Jason Merrill" <jason@redhat.com> wrote:

> Ok.

>

> On Mar 1, 2018 4:40 PM, "Marek Polacek" <polacek@redhat.com> wrote:

>

>> On Thu, Mar 01, 2018 at 01:56:50PM -0500, Jason Merrill wrote:

>> > On Thu, Mar 1, 2018 at 8:17 AM, Marek Polacek <polacek@redhat.com>

>> wrote:

>> > > On Wed, Feb 28, 2018 at 04:50:39PM -0500, Jason Merrill wrote:

>> > >> On Wed, Feb 28, 2018 at 4:19 PM, Marek Polacek <polacek@redhat.com>

>> wrote:

>> > >> > On Wed, Feb 28, 2018 at 10:51:17AM -0500, Jason Merrill wrote:

>> > >> >> On Wed, Feb 28, 2018 at 9:32 AM, Marek Polacek <

>> polacek@redhat.com> wrote:

>> > >> >> > On Tue, Feb 27, 2018 at 04:16:31PM -0500, Jason Merrill wrote:

>> > >> >> >> On 02/27/2018 02:13 PM, Marek Polacek wrote:

>> > >> >> >> > My recent change introducing cxx_constant_init caused this

>> code

>> > >> >> >> >

>> > >> >> >> > template <class> class A {

>> > >> >> >> >    static const long b = 0;

>> > >> >> >> >    static const unsigned c = (b);

>> > >> >> >> > };

>> > >> >> >> >

>> > >> >> >> > to be rejected.  The reason is that force_paren_expr turns

>> "b" into "*(const

>> > >> >> >> > long int &) &b", where the former is not value-dependent but

>> the latter is

>> > >> >> >> > value-dependent.  So when we get to maybe_constant_init_1:

>> > >> >> >> > 5147   if (!is_nondependent_static_init_expression (t))

>> > >> >> >> > 5148     /* Don't try to evaluate it.  */;

>> > >> >> >> > it's not evaluated and we get the non-constant

>> initialization error.

>> > >> >> >> > (Before we'd always evaluated the expression.)

>> > >> >> >> >

>> > >> >> >> > Bootstrapped/regtested on x86_64-linux, ok for trunk?

>> > >> >> >> >

>> > >> >> >> > 2018-02-27  Marek Polacek  <polacek@redhat.com>

>> > >> >> >> >

>> > >> >> >> >     PR c++/84582

>> > >> >> >> >     * semantics.c (force_paren_expr): Avoid creating a

>> static cast

>> > >> >> >> >     when processing a template.

>> > >> >> >> >

>> > >> >> >> >     * g++.dg/cpp1z/static1.C: New test.

>> > >> >> >> >     * g++.dg/template/static37.C: New test.

>> > >> >> >> >

>> > >> >> >> > diff --git gcc/cp/semantics.c gcc/cp/semantics.c

>> > >> >> >> > index 35569d0cb0d..b48de2df4e2 100644

>> > >> >> >> > --- gcc/cp/semantics.c

>> > >> >> >> > +++ gcc/cp/semantics.c

>> > >> >> >> > @@ -1697,7 +1697,7 @@ force_paren_expr (tree expr)

>> > >> >> >> >       expr = build1 (PAREN_EXPR, TREE_TYPE (expr), expr);

>> > >> >> >> >     else if (VAR_P (expr) && DECL_HARD_REGISTER (expr))

>> > >> >> >> >       /* We can't bind a hard register variable to a

>> reference.  */;

>> > >> >> >> > -  else

>> > >> >> >> > +  else if (!processing_template_decl)

>> > >> >> >>

>> > >> >> >> Hmm, this means that we forget about the parentheses in a

>> template.  I'm

>> > >> >> >> surprised that this didn't break anything in the testsuite.

>> In particular,

>> > >> >> >> auto-fn15.C.  I've attached an addition to auto-fn15.C to

>> catch this issue.

>> > >> >> >

>> > >> >> > Thanks, you're right.  I'll use it.

>> > >> >> >

>> > >> >> >> Can we use PAREN_EXPR instead of the static_cast in a template?

>> > >> >> >

>> > >> >> > I don't think so, it would fix the issue you pointed out in

>> auto-fn15.C but

>> > >> >> > it wouldn't fix the original test.  The problem with using

>> PAREN_EXPR in a

>> > >> >> > template is that instantiate_non_dependent_expr will turn in

>> into the

>> > >> >> > static cast anyway; tsubst_copy_and_build has

>> > >> >> >     case PAREN_EXPR:

>> > >> >> >       RETURN (finish_parenthesized_expr (RECUR (TREE_OPERAND

>> (t, 0))));

>> > >> >> > so it calls force_paren_expr and this time we're not in a

>> template.  And

>> > >> >> > then when calling cxx_constant_init we have the same issue.

>> > >> >>

>> > >> >> Then maybe we need something like fold_non_dependent_expr, which

>> > >> >> checks for dependency before substitution and then immediately

>> > >> >> evaluates the result.

>> > >> >

>> > >> > I hope you meant something like this.  Further testing also

>> revealed that

>> > >> > maybe_undo_parenthesized_ref should be able to unwrap PAREN_EXPR

>> (so that

>> > >> > (fn1)(); in paren2.C is handled correctly), and that lvalue_kind

>> should look

>> > >> > into PAREN_EXPR so as to give the correct answer regarding

>> lvalueness: we

>> > >> > should accept

>> > >> >

>> > >> > template<typename T>

>> > >> > void foo (int i)

>> > >> > {

>> > >> >   ++(i);

>> > >> > }

>> > >> >

>> > >> > Apologies if I'm on the wrong track.

>> > >> >

>> > >> > Bootstrapped/regtested on x86_64-linux, ok for trunk?

>> > >> >

>> > >> > 2018-02-28  Marek Polacek  <polacek@redhat.com>

>> > >> >             Jason Merrill  <jason@redhat.com>

>> > >> >

>> > >> >         PR c++/84582

>> > >> >         * semantics.c (force_paren_expr): Avoid creating the

>> static cast

>> > >> >         when in a template.  Create a PAREN_EXPR when in a

>> template.

>> > >> >         (maybe_undo_parenthesized_ref): Unwrap PAREN_EXPR.

>> > >> >         * typeck2.c (store_init_value): Call

>> fold_non_dependent_expr instead

>> > >> >         of instantiate_non_dependent_expr.

>> > >> >         * tree.c (lvalue_kind): Handle PAREN_EXPR like

>> NON_DEPENDENT_EXPR.

>> > >> >

>> > >> >         * g++.dg/cpp1y/auto-fn15.C: Extend testing.

>> > >> >         * g++.dg/cpp1z/static1.C: New test.

>> > >> >         * g++.dg/template/static37.C: New test.

>> > >> >

>> > >> > diff --git gcc/cp/semantics.c gcc/cp/semantics.c

>> > >> > index 35569d0cb0d..722e3718a14 100644

>> > >> > --- gcc/cp/semantics.c

>> > >> > +++ gcc/cp/semantics.c

>> > >> > @@ -1697,7 +1697,7 @@ force_paren_expr (tree expr)

>> > >> >      expr = build1 (PAREN_EXPR, TREE_TYPE (expr), expr);

>> > >> >    else if (VAR_P (expr) && DECL_HARD_REGISTER (expr))

>> > >> >      /* We can't bind a hard register variable to a reference.  */;

>> > >> > -  else

>> > >> > +  else if (!processing_template_decl)

>> > >> >      {

>> > >> >        cp_lvalue_kind kind = lvalue_kind (expr);

>> > >> >        if ((kind & ~clk_class) != clk_none)

>> > >> > @@ -1713,6 +1713,8 @@ force_paren_expr (tree expr)

>> > >> >             REF_PARENTHESIZED_P (expr) = true;

>> > >> >         }

>> > >> >      }

>> > >> > +  else

>> > >> > +    expr = build1 (PAREN_EXPR, TREE_TYPE (expr), expr);

>> > >>

>> > >> There's already a branch for building PAREN_EXPR, let's just replace

>> > >> its condition.

>> > >

>> > > Sure.

>> > >

>> > >> > -      value = instantiate_non_dependent_expr (value);

>> > >> > +      value = fold_non_dependent_expr (value);

>> > >>

>> > >> I was thinking that we want a parallel fold_non_dependent_init (that

>> > >> hopefully shares most of the implementation).  Then we shouldn't need

>> > >> the call to maybe_constant_init anymore.

>> > >

>> > > If you mean fold_non_dependent_init that would be like

>> fold_non_dependent_expr

>> > > but with maybe_constant_init and not maybe_constant_value

>> >

>> > And is_nondependent_static_init_expression, and different arguments to

>> > cxx_eval_outermost_constant_expression, yes.

>>

>> Ah.  Maybe it'll be useful sometime in the future.

>>

>> > > then that would break e.g.

>> > >

>> > > const double d = 9.0;   // missing constexpr

>> > > constexpr double j = d; // should give error

>> > >

>> > > because maybe_constant_value checks is_nondependent_constant_expression,

>> and

>> > > "d" in the example above is not a constant expression, so we don't

>> evaluate,

>> > > and "d" stays "d", so require_constant_expression gives the error.

>> On the

>> > > other hand, maybe_constant_init checks is_nondependent_static_init_ex

>> pression,

>> > > and "d" is that, so we evaluate "d" to "9.0".  Then

>> require_constant_expression

>> > > doesn't complain.

>> >

>> > Ah, I see.  You're right, let's stick with fold_non_dependent_expr.

>>

>> Thanks, so this is the final patch then:

>>

>> Bootstrapped/regtested on x86_64-linux, ok for trunk?

>>

>> 2018-03-01  Marek Polacek  <polacek@redhat.com>

>>             Jason Merrill  <jason@redhat.com>

>>

>>         PR c++/84582

>>         * semantics.c (force_paren_expr): Create a PAREN_EXPR when in

>>         a template.

>>         (maybe_undo_parenthesized_ref): Unwrap PAREN_EXPR.

>>         * typeck2.c (store_init_value): Call fold_non_dependent_expr

>> instead

>>         of instantiate_non_dependent_expr.

>>         * tree.c (lvalue_kind): Handle PAREN_EXPR like NON_DEPENDENT_EXPR.

>>

>>         * g++.dg/cpp1y/auto-fn15.C: Extend testing.

>>         * g++.dg/cpp1z/static1.C: New test.

>>         * g++.dg/template/static37.C: New test.

>>

>> diff --git gcc/cp/semantics.c gcc/cp/semantics.c

>> index 87c5c669a55..1ac1d23e761 100644

>> --- gcc/cp/semantics.c

>> +++ gcc/cp/semantics.c

>> @@ -1693,7 +1693,8 @@ force_paren_expr (tree expr)

>>    if (TREE_CODE (expr) == COMPONENT_REF

>>        || TREE_CODE (expr) == SCOPE_REF)

>>      REF_PARENTHESIZED_P (expr) = true;

>> -  else if (type_dependent_expression_p (expr))

>> +  else if (type_dependent_expression_p (expr)

>> +          || processing_template_decl)

>

>

Actually, this is redundant; an expression can only be dependent if
processing_template_decl.  I'll fix.

Jason
Marek Polacek March 2, 2018, 6:20 p.m. | #11
On Fri, Mar 02, 2018 at 01:17:55PM -0500, Jason Merrill wrote:
> Actually, this is redundant; an expression can only be dependent if

> processing_template_decl.  I'll fix.


Ah, that makes a lot of sense.  Thanks,

	Marek

Patch

diff --git gcc/cp/semantics.c gcc/cp/semantics.c
index 35569d0cb0d..b48de2df4e2 100644
--- gcc/cp/semantics.c
+++ gcc/cp/semantics.c
@@ -1697,7 +1697,7 @@  force_paren_expr (tree expr)
     expr = build1 (PAREN_EXPR, TREE_TYPE (expr), expr);
   else if (VAR_P (expr) && DECL_HARD_REGISTER (expr))
     /* We can't bind a hard register variable to a reference.  */;
-  else
+  else if (!processing_template_decl)
     {
       cp_lvalue_kind kind = lvalue_kind (expr);
       if ((kind & ~clk_class) != clk_none)
diff --git gcc/testsuite/g++.dg/cpp1z/static1.C gcc/testsuite/g++.dg/cpp1z/static1.C
index e69de29bb2d..cb872997c5a 100644
--- gcc/testsuite/g++.dg/cpp1z/static1.C
+++ gcc/testsuite/g++.dg/cpp1z/static1.C
@@ -0,0 +1,19 @@ 
+// PR c++/84582
+// { dg-options -std=c++17 }
+
+class C {
+  static inline const long b = 0;
+  static inline const unsigned c = (b);
+};
+class D {
+  static inline const long b = 0;
+  static inline const unsigned c = b;
+};
+template <class> class A {
+  static inline const long b = 0;
+  static inline const unsigned c = (b);
+};
+template <class> class B {
+  static inline const long b = 0;
+  static inline const unsigned c = b;
+};
diff --git gcc/testsuite/g++.dg/template/static37.C gcc/testsuite/g++.dg/template/static37.C
index e69de29bb2d..90bc65d2fbc 100644
--- gcc/testsuite/g++.dg/template/static37.C
+++ gcc/testsuite/g++.dg/template/static37.C
@@ -0,0 +1,18 @@ 
+// PR c++/84582
+
+class C {
+  static const long b = 0;
+  static const unsigned c = (b);
+};
+class D {
+  static const long b = 0;
+  static const unsigned c = b;
+};
+template <class> class A {
+  static const long b = 0;
+  static const unsigned c = (b);
+};
+template <class> class B {
+  static const long b = 0;
+  static const unsigned c = b;
+};