Message ID | 20180118223919.GE3914@redhat.com |
---|---|
State | New |
Headers | show |
Series | C++ PATCH to fix bogus error with constexpr and empty class (PR c++/81933) | expand |
On Thu, Jan 18, 2018 at 5:39 PM, Marek Polacek <polacek@redhat.com> wrote: > The problem in this PR is that we get > error: constexpr call flows off the end of the function > for a valid testcase, but not in C++17 mode. That's because in C++17 we > execute: > > 4321 if (cxx_dialect >= cxx17 && !BINFO_VIRTUAL_P (binfo)) > 4322 { > 4323 tree decl = build_base_field_1 (t, basetype, next_field); > 4324 DECL_FIELD_OFFSET (decl) = BINFO_OFFSET (binfo); > 4325 DECL_FIELD_BIT_OFFSET (decl) = bitsize_zero_node; > 4326 SET_DECL_OFFSET_ALIGN (decl, BITS_PER_UNIT); > 4327 } > > which puts some additional fields into some classes. This makes a difference > for split_nonconstant_init: > > 751 if (split_nonconstant_init_1 (dest, init)) > 752 init = NULL_TREE; > > where split_nonconstant_init_1 is true so that means that the DECL_INITIAL will > be null. Why is split_nonconstant_init_1 true? Because it ends up calling > complete_ctor_at_level_p with > > 6114 return count_type_elements (type, true) == num_elts; > > but the count_type_elements result differs between c++14/c++17 precisely > because of the fields added in build_base_field. But we should only add those > fields when initializing aggregates with bases, which is not what's happening > in this testcase. > > With DECL_INITIAL being null, we error here (result is NULL): > > 1702 result = *ctx->values->get (slot ? slot : res); > 1703 if (result == NULL_TREE && !*non_constant_p) > 1704 { > 1705 if (!ctx->quiet) > 1706 error ("%<constexpr%> call flows off the end " > 1707 "of the function"); > 1708 *non_constant_p = true; > 1709 } > > I think the problem is that we're dealing with an empty class here, for which > we should just generate an empty ctor, as in few other spots. This seems like a workaround rather than a fix; we should fix split_nonconstant_init to work properly rather than override it. Specifically, it seems odd to return true if num_elts is 0; we shouldn't override DECL_INITIAL if we didn't actually split anything out. Jason
On Mon, Jan 22, 2018 at 09:40:50AM -0500, Jason Merrill wrote: > This seems like a workaround rather than a fix; we should fix > split_nonconstant_init to work properly rather than override it. > Specifically, it seems odd to return true if num_elts is 0; we > shouldn't override DECL_INITIAL if we didn't actually split anything > out. Ah, ok, here's a patch for that, then. Thanks. Bootstrapped/regtested on x86_64-linux, ok for trunk? 2018-01-22 Marek Polacek <polacek@redhat.com> PR c++/81933 * typeck2.c (split_nonconstant_init_1): Return false if we didn't split out anything. * g++.dg/cpp1y/constexpr-empty4.C: New test. diff --git gcc/cp/typeck2.c gcc/cp/typeck2.c index 8d933257f5f..8cb0e8811d3 100644 --- gcc/cp/typeck2.c +++ gcc/cp/typeck2.c @@ -725,6 +725,11 @@ split_nonconstant_init_1 (tree dest, tree init) /* The rest of the initializer is now a constant. */ TREE_CONSTANT (init) = 1; + + /* We didn't split out anything. */ + if (num_split_elts == 0) + return false; + return complete_p && complete_ctor_at_level_p (TREE_TYPE (init), num_split_elts, inner_type); } diff --git gcc/testsuite/g++.dg/cpp1y/constexpr-empty4.C gcc/testsuite/g++.dg/cpp1y/constexpr-empty4.C index e69de29bb2d..059220f8268 100644 --- gcc/testsuite/g++.dg/cpp1y/constexpr-empty4.C +++ gcc/testsuite/g++.dg/cpp1y/constexpr-empty4.C @@ -0,0 +1,51 @@ +// PR c++/81933 +// { dg-do compile { target c++14 } } + +namespace std { +template <typename _Tp> struct __decay_and_strip { typedef _Tp __type; }; +template <int> struct enable_if { typedef int type; }; +template <typename _Head> struct _Head_base { + constexpr _Head_base(_Head) {} +}; +template <unsigned long, typename...> struct _Tuple_impl; +template <unsigned long _Idx, typename _Head, typename... _Tail> +struct _Tuple_impl<_Idx, _Head, _Tail...> : _Tuple_impl<1, _Tail...>, // { dg-warning "direct base" } + _Head_base<_Head> { + typedef _Tuple_impl<1, _Tail...> _Inherited; + typedef _Head_base<_Head> _Base; + constexpr _Tuple_impl(_Head __head, _Tail... __tail) + : _Inherited(__tail...), _Base(__head) {} + _Tuple_impl(const _Tuple_impl &) = default; + _Tuple_impl(_Tuple_impl &&); +}; +template <unsigned long _Idx, typename _Head> +struct _Tuple_impl<_Idx, _Head> : _Head_base<_Head> { + typedef _Head_base<_Head> _Base; + constexpr _Tuple_impl(_Head __head) : _Base(__head) {} +}; +template <int> struct _TC { + static constexpr bool _NotSameTuple() { return true; } +}; +template <typename... _Elements> class tuple : _Tuple_impl<0, _Elements...> { + typedef _Tuple_impl<0, _Elements...> _Inherited; + +public: + template <typename... _UElements, + enable_if<_TC<1>::_NotSameTuple()>::type = false> + constexpr tuple(_UElements... __elements) : _Inherited(__elements...) {} + tuple(const tuple &) = default; +}; +template <typename... _Elements> +constexpr tuple<typename __decay_and_strip<_Elements>::__type...> + make_tuple(_Elements... __args) { + typedef tuple<typename __decay_and_strip<_Elements>::__type...> __result_type; + return __result_type(__args...); +} +} +struct any_udt {}; +template <typename... Tuples> constexpr auto flatten(Tuples... tuples) { + auto all = std::make_tuple(tuples...); + auto flat(all); + return flat; +} +constexpr auto fail = flatten(any_udt{}, any_udt{}); Marek
OK, thanks. On Mon, Jan 22, 2018 at 10:44 AM, Marek Polacek <polacek@redhat.com> wrote: > On Mon, Jan 22, 2018 at 09:40:50AM -0500, Jason Merrill wrote: >> This seems like a workaround rather than a fix; we should fix >> split_nonconstant_init to work properly rather than override it. >> Specifically, it seems odd to return true if num_elts is 0; we >> shouldn't override DECL_INITIAL if we didn't actually split anything >> out. > > Ah, ok, here's a patch for that, then. Thanks. > > Bootstrapped/regtested on x86_64-linux, ok for trunk? > > 2018-01-22 Marek Polacek <polacek@redhat.com> > > PR c++/81933 > * typeck2.c (split_nonconstant_init_1): Return false if we didn't > split out anything. > > * g++.dg/cpp1y/constexpr-empty4.C: New test. > > diff --git gcc/cp/typeck2.c gcc/cp/typeck2.c > index 8d933257f5f..8cb0e8811d3 100644 > --- gcc/cp/typeck2.c > +++ gcc/cp/typeck2.c > @@ -725,6 +725,11 @@ split_nonconstant_init_1 (tree dest, tree init) > > /* The rest of the initializer is now a constant. */ > TREE_CONSTANT (init) = 1; > + > + /* We didn't split out anything. */ > + if (num_split_elts == 0) > + return false; > + > return complete_p && complete_ctor_at_level_p (TREE_TYPE (init), > num_split_elts, inner_type); > } > diff --git gcc/testsuite/g++.dg/cpp1y/constexpr-empty4.C gcc/testsuite/g++.dg/cpp1y/constexpr-empty4.C > index e69de29bb2d..059220f8268 100644 > --- gcc/testsuite/g++.dg/cpp1y/constexpr-empty4.C > +++ gcc/testsuite/g++.dg/cpp1y/constexpr-empty4.C > @@ -0,0 +1,51 @@ > +// PR c++/81933 > +// { dg-do compile { target c++14 } } > + > +namespace std { > +template <typename _Tp> struct __decay_and_strip { typedef _Tp __type; }; > +template <int> struct enable_if { typedef int type; }; > +template <typename _Head> struct _Head_base { > + constexpr _Head_base(_Head) {} > +}; > +template <unsigned long, typename...> struct _Tuple_impl; > +template <unsigned long _Idx, typename _Head, typename... _Tail> > +struct _Tuple_impl<_Idx, _Head, _Tail...> : _Tuple_impl<1, _Tail...>, // { dg-warning "direct base" } > + _Head_base<_Head> { > + typedef _Tuple_impl<1, _Tail...> _Inherited; > + typedef _Head_base<_Head> _Base; > + constexpr _Tuple_impl(_Head __head, _Tail... __tail) > + : _Inherited(__tail...), _Base(__head) {} > + _Tuple_impl(const _Tuple_impl &) = default; > + _Tuple_impl(_Tuple_impl &&); > +}; > +template <unsigned long _Idx, typename _Head> > +struct _Tuple_impl<_Idx, _Head> : _Head_base<_Head> { > + typedef _Head_base<_Head> _Base; > + constexpr _Tuple_impl(_Head __head) : _Base(__head) {} > +}; > +template <int> struct _TC { > + static constexpr bool _NotSameTuple() { return true; } > +}; > +template <typename... _Elements> class tuple : _Tuple_impl<0, _Elements...> { > + typedef _Tuple_impl<0, _Elements...> _Inherited; > + > +public: > + template <typename... _UElements, > + enable_if<_TC<1>::_NotSameTuple()>::type = false> > + constexpr tuple(_UElements... __elements) : _Inherited(__elements...) {} > + tuple(const tuple &) = default; > +}; > +template <typename... _Elements> > +constexpr tuple<typename __decay_and_strip<_Elements>::__type...> > + make_tuple(_Elements... __args) { > + typedef tuple<typename __decay_and_strip<_Elements>::__type...> __result_type; > + return __result_type(__args...); > +} > +} > +struct any_udt {}; > +template <typename... Tuples> constexpr auto flatten(Tuples... tuples) { > + auto all = std::make_tuple(tuples...); > + auto flat(all); > + return flat; > +} > +constexpr auto fail = flatten(any_udt{}, any_udt{}); > > Marek
diff --git gcc/cp/constexpr.c gcc/cp/constexpr.c index 8984613aa41..0ff74b98437 100644 --- gcc/cp/constexpr.c +++ gcc/cp/constexpr.c @@ -1701,7 +1701,15 @@ cxx_eval_call_expression (const constexpr_ctx *ctx, tree t, result = void_node; else { - result = *ctx->values->get (slot ? slot : res); + tree retval = slot ? slot : res; + /* If the class is empty, we aren't actually loading anything. */ + if (is_really_empty_class (TREE_TYPE (retval))) + { + result = build_constructor (TREE_TYPE (retval), NULL); + TREE_CONSTANT (result) = true; + } + else + result = *ctx->values->get (retval); if (result == NULL_TREE && !*non_constant_p) { if (!ctx->quiet) diff --git gcc/testsuite/g++.dg/cpp1y/constexpr-empty4.C gcc/testsuite/g++.dg/cpp1y/constexpr-empty4.C index e69de29bb2d..059220f8268 100644 --- gcc/testsuite/g++.dg/cpp1y/constexpr-empty4.C +++ gcc/testsuite/g++.dg/cpp1y/constexpr-empty4.C @@ -0,0 +1,51 @@ +// PR c++/81933 +// { dg-do compile { target c++14 } } + +namespace std { +template <typename _Tp> struct __decay_and_strip { typedef _Tp __type; }; +template <int> struct enable_if { typedef int type; }; +template <typename _Head> struct _Head_base { + constexpr _Head_base(_Head) {} +}; +template <unsigned long, typename...> struct _Tuple_impl; +template <unsigned long _Idx, typename _Head, typename... _Tail> +struct _Tuple_impl<_Idx, _Head, _Tail...> : _Tuple_impl<1, _Tail...>, // { dg-warning "direct base" } + _Head_base<_Head> { + typedef _Tuple_impl<1, _Tail...> _Inherited; + typedef _Head_base<_Head> _Base; + constexpr _Tuple_impl(_Head __head, _Tail... __tail) + : _Inherited(__tail...), _Base(__head) {} + _Tuple_impl(const _Tuple_impl &) = default; + _Tuple_impl(_Tuple_impl &&); +}; +template <unsigned long _Idx, typename _Head> +struct _Tuple_impl<_Idx, _Head> : _Head_base<_Head> { + typedef _Head_base<_Head> _Base; + constexpr _Tuple_impl(_Head __head) : _Base(__head) {} +}; +template <int> struct _TC { + static constexpr bool _NotSameTuple() { return true; } +}; +template <typename... _Elements> class tuple : _Tuple_impl<0, _Elements...> { + typedef _Tuple_impl<0, _Elements...> _Inherited; + +public: + template <typename... _UElements, + enable_if<_TC<1>::_NotSameTuple()>::type = false> + constexpr tuple(_UElements... __elements) : _Inherited(__elements...) {} + tuple(const tuple &) = default; +}; +template <typename... _Elements> +constexpr tuple<typename __decay_and_strip<_Elements>::__type...> + make_tuple(_Elements... __args) { + typedef tuple<typename __decay_and_strip<_Elements>::__type...> __result_type; + return __result_type(__args...); +} +} +struct any_udt {}; +template <typename... Tuples> constexpr auto flatten(Tuples... tuples) { + auto all = std::make_tuple(tuples...); + auto flat(all); + return flat; +} +constexpr auto fail = flatten(any_udt{}, any_udt{});