| Message ID | 20180118231315.GM2063@tucnak |
|---|---|
| State | New |
| Headers | show |
| Series | [C++] Fix ICE in potential_constant_expression_1 (PR c++/83918) | expand |
On Thu, Jan 18, 2018 at 6:13 PM, Jakub Jelinek <jakub@redhat.com> wrote: > Before location wrappers were introduced, the potential_constant_expression_1 > assumption that a VIEW_CONVERT_EXPR must have non-NULL type was right, > if it is a type dependent reinterpret cast, it would be > REINTERPRET_CAST_EXPR instead. > > Location wrappers around decls can have NULL type, if the decl they wrap > also has NULL type. Hmm, why would a decl have NULL type? Are we wrapping a CONST_DECL in VIEW_CONVERT_EXPR? They should get NON_LVALUE_EXPR, since they aren't lvalues (except the objc++ ones that have TREE_STATIC set). > Fixed thusly, bootstrapped/regtested on x86_64-linux and i686-linux, ok for > trunk? Alternatively it could if (TREE_TYPE (t) && ... > > On a related note, I wonder if the > return (RECUR (from, TREE_CODE (t) != VIEW_CONVERT_EXPR)); > a few lines below is correct, VCE isn't always an lvalue, only if it is used > in non-rvalue contexts. So, shouldn't it be instead: > return (RECUR (from, (TREE_CODE (t) == VIEW_CONVERT_EXPR > ? want_rval : rval))); A VCE should always be an lvalue. If it's used in an rvalue context, then it's an lvalue used in an rvalue context and so the larger expression is an rvalue, but the VCE is still an lvalue. Jason
On Mon, Jan 22, 2018 at 04:49:34PM -0500, Jason Merrill wrote: > On Thu, Jan 18, 2018 at 6:13 PM, Jakub Jelinek <jakub@redhat.com> wrote: > > Before location wrappers were introduced, the potential_constant_expression_1 > > assumption that a VIEW_CONVERT_EXPR must have non-NULL type was right, > > if it is a type dependent reinterpret cast, it would be > > REINTERPRET_CAST_EXPR instead. > > > > Location wrappers around decls can have NULL type, if the decl they wrap > > also has NULL type. > > Hmm, why would a decl have NULL type? Are we wrapping a CONST_DECL in > VIEW_CONVERT_EXPR? They should get NON_LVALUE_EXPR, since they aren't > lvalues (except the objc++ ones that have TREE_STATIC set). So, do you want following instead, if it passes bootstrap/regtest? 2018-01-22 Jakub Jelinek <jakub@redhat.com> PR c++/83918 * tree.c (maybe_wrap_with_location): Use NON_LVALUE_EXPR rather than VIEW_CONVERT_EXPR to wrap CONST_DECLs. * g++.dg/cpp1z/pr83918.C: New test. --- gcc/tree.c.jj 2018-01-16 16:16:37.124694238 +0100 +++ gcc/tree.c 2018-01-22 23:53:58.171837029 +0100 @@ -14085,8 +14085,10 @@ maybe_wrap_with_location (tree expr, loc if (EXCEPTIONAL_CLASS_P (expr)) return expr; - tree_code code = (CONSTANT_CLASS_P (expr) && TREE_CODE (expr) != STRING_CST - ? NON_LVALUE_EXPR : VIEW_CONVERT_EXPR); + tree_code code + = (((CONSTANT_CLASS_P (expr) && TREE_CODE (expr) != STRING_CST) + || (TREE_CODE (expr) == CONST_DECL && !TREE_STATIC (expr))) + ? NON_LVALUE_EXPR : VIEW_CONVERT_EXPR); tree wrapper = build1_loc (loc, code, TREE_TYPE (expr), expr); /* Mark this node as being a wrapper. */ EXPR_LOCATION_WRAPPER_P (wrapper) = 1; --- gcc/testsuite/g++.dg/cpp1z/pr83918.C.jj 2018-01-22 23:54:40.824824960 +0100 +++ gcc/testsuite/g++.dg/cpp1z/pr83918.C 2018-01-22 23:54:40.824824960 +0100 @@ -0,0 +1,32 @@ +// PR c++/83918 +// { dg-do compile } +// { dg-options "-std=c++17" } + +constexpr unsigned +foo (unsigned x, unsigned y) +{ + return x > y ? x : y; +} + +template <typename, typename> struct A; +template <auto ...> struct B; +template <auto S, auto ... T, auto U, auto ... V> +struct A <B <S, T...>, B <U, V...>> +{ + enum : unsigned + { + u = foo (sizeof (S), sizeof (U)), + v = A <B <T...>, B <V...>>::w, + w = foo (u, v) + }; +}; + +template <> +struct A <B <>, B <>> +{ + enum : unsigned { w = 0 }; +}; + +constexpr static const auto v { A <B <1,2,3,4,5,6,7,8,9>, + B <9,8,7,6,5,4,3,2,1>>::w }; +static_assert (v == sizeof (int)); Jakub
On Mon, Jan 22, 2018 at 6:14 PM, Jakub Jelinek <jakub@redhat.com> wrote: > On Mon, Jan 22, 2018 at 04:49:34PM -0500, Jason Merrill wrote: >> On Thu, Jan 18, 2018 at 6:13 PM, Jakub Jelinek <jakub@redhat.com> wrote: >> > Before location wrappers were introduced, the potential_constant_expression_1 >> > assumption that a VIEW_CONVERT_EXPR must have non-NULL type was right, >> > if it is a type dependent reinterpret cast, it would be >> > REINTERPRET_CAST_EXPR instead. >> > >> > Location wrappers around decls can have NULL type, if the decl they wrap >> > also has NULL type. >> >> Hmm, why would a decl have NULL type? Are we wrapping a CONST_DECL in >> VIEW_CONVERT_EXPR? They should get NON_LVALUE_EXPR, since they aren't >> lvalues (except the objc++ ones that have TREE_STATIC set). > > So, do you want following instead, if it passes bootstrap/regtest? Looks good. Jason
--- gcc/cp/constexpr.c.jj 2018-01-17 22:00:13.624228171 +0100 +++ gcc/cp/constexpr.c 2018-01-18 10:38:07.357469094 +0100 @@ -5385,7 +5385,8 @@ potential_constant_expression_1 (tree t, may change to something more specific to type-punning (DR 1312). */ { tree from = TREE_OPERAND (t, 0); - if (POINTER_TYPE_P (TREE_TYPE (t)) + if (!location_wrapper_p (t) + && POINTER_TYPE_P (TREE_TYPE (t)) && TREE_CODE (from) == INTEGER_CST && !integer_zerop (from)) { --- gcc/testsuite/g++.dg/cpp1z/pr83918.C.jj 2018-01-18 10:46:04.405439296 +0100 +++ gcc/testsuite/g++.dg/cpp1z/pr83918.C 2018-01-18 10:45:42.934442290 +0100 @@ -0,0 +1,32 @@ +// PR c++/83918 +// { dg-do compile } +// { dg-options "-std=c++17" } + +constexpr unsigned +foo (unsigned x, unsigned y) +{ + return x > y ? x : y; +} + +template <typename, typename> struct A; +template <auto ...> struct B; +template <auto S, auto ... T, auto U, auto ... V> +struct A <B <S, T...>, B <U, V...>> +{ + enum : unsigned + { + u = foo (sizeof (S), sizeof (U)), + v = A <B <T...>, B <V...>>::w, + w = foo (u, v) + }; +}; + +template <> +struct A <B <>, B <>> +{ + enum : unsigned { w = 0 }; +}; + +constexpr static const auto v { A <B <1,2,3,4,5,6,7,8,9>, + B <9,8,7,6,5,4,3,2,1>>::w }; +static_assert (v == sizeof (int));