diff mbox series

C++ PATCH to fix bogus error with constexpr and empty class (PR c++/81933)

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

Commit Message

Marek Polacek Jan. 18, 2018, 10:39 p.m. UTC 
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.

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

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

	PR c++/81933
	* constexpr.c (cxx_eval_call_expression): When evaluating a call whose
	result's type is an empty class, return empty constructor.

	* g++.dg/cpp1y/constexpr-empty4.C: New test.


	Marek

Comments

Jason Merrill Jan. 22, 2018, 2:40 p.m. UTC | #1 
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
Marek Polacek Jan. 22, 2018, 3:44 p.m. UTC | #2 
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
Jason Merrill Jan. 22, 2018, 3:52 p.m. UTC | #3 
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 mbox series

Patch

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{});