Bug 105491 - [10/11 Regression] Usage of __constinit with -std=c++11 does is rejected
Summary: [10/11 Regression] Usage of __constinit with -std=c++11 does is rejected
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: c++ (show other bugs)
Version: 12.0
: P2 normal
Target Milestone: 12.2
Assignee: Patrick Palka
URL:
Keywords: rejects-valid
Depends on:
Blocks:
 
Reported: 2022-05-05 09:57 UTC by Martin Liška
Modified: 2023-05-21 15:05 UTC (History)
5 users (show)

See Also:
Host:
Target:
Build:
Known to work: 13.0
Known to fail: 10.4.0, 11.3.0
Last reconfirmed: 2022-05-05 00:00:00


Attachments
test-case (270.62 KB, application/zstd)
2022-05-05 09:57 UTC, Martin Liška
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Martin Liška 2022-05-05 09:57:50 UTC
Created attachment 52930 [details]
test-case

Noticed that in protobuf package:
https://github.com/protocolbuffers/protobuf/issues/9916

$ g++ 12.ii -c -std=c++11
x.cc:39:123: error: ‘constinit’ variable ‘_ProtobufCFileOptions_default_instance_’ does not have a constant initializer
   39 | PROTOBUF_ATTRIBUTE_NO_DESTROY PROTOBUF_CONSTINIT PROTOBUF_ATTRIBUTE_INIT_PRIORITY1 ProtobufCFileOptionsDefaultTypeInternal _ProtobufCFileOptions_default_instance_;
      |                                                                                                                           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
x.cc:39:123: error: ‘ProtobufCFileOptionsDefaultTypeInternal{ProtobufCFileOptionsDefaultTypeInternal::<unnamed union>{ProtobufCFileOptions{google::protobuf::Message{google::protobuf::MessageLite{((& ProtobufCFileOptions::_ZTV20ProtobufCFileOptions) + 16), google::protobuf::internal::InternalMetadata{0}}}, google::protobuf::internal::HasBits<1>{uint32_t [1]()}, google::protobuf::internal::CachedSize{std::atomic<int>{std::__atomic_base<int>{0}}}, google::protobuf::internal::ArenaStringPtr{google::protobuf::internal::TaggedStringPtr{((void*)(& google::protobuf::internal::fixed_address_empty_string))}}, false, false, false, true, true}}}’ is not a constant expression
x.cc:53:126: error: ‘constinit’ variable ‘_ProtobufCMessageOptions_default_instance_’ does not have a constant initializer
   53 | PROTOBUF_ATTRIBUTE_NO_DESTROY PROTOBUF_CONSTINIT PROTOBUF_ATTRIBUTE_INIT_PRIORITY1 ProtobufCMessageOptionsDefaultTypeInternal _ProtobufCMessageOptions_default_instance_;
      |                                                                                                                              ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
x.cc:53:126: error: ‘ProtobufCMessageOptionsDefaultTypeInternal{ProtobufCMessageOptionsDefaultTypeInternal::<unnamed union>{ProtobufCMessageOptions{google::protobuf::Message{google::protobuf::MessageLite{((& ProtobufCMessageOptions::_ZTV23ProtobufCMessageOptions) + 16), google::protobuf::internal::InternalMetadata{0}}}, google::protobuf::internal::HasBits<1>{uint32_t [1]()}, google::protobuf::internal::CachedSize{std::atomic<int>{std::__atomic_base<int>{0}}}, google::protobuf::internal::ArenaStringPtr{google::protobuf::internal::TaggedStringPtr{0}}, false, true}}}’ is not a constant expression
x.cc:65:124: error: ‘constinit’ variable ‘_ProtobufCFieldOptions_default_instance_’ does not have a constant initializer
   65 | PROTOBUF_ATTRIBUTE_NO_DESTROY PROTOBUF_CONSTINIT PROTOBUF_ATTRIBUTE_INIT_PRIORITY1 ProtobufCFieldOptionsDefaultTypeInternal _ProtobufCFieldOptions_default_instance_;
      |                                                                                                                            ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
x.cc:65:124: error: ‘ProtobufCFieldOptionsDefaultTypeInternal{ProtobufCFieldOptionsDefaultTypeInternal::<unnamed union>{ProtobufCFieldOptions{google::protobuf::Message{google::protobuf::MessageLite{((& ProtobufCFieldOptions::_ZTV21ProtobufCFieldOptions) + 16), google::protobuf::internal::InternalMetadata{0}}}, google::protobuf::internal::HasBits<1>{uint32_t [1]()}, google::protobuf::internal::CachedSize{std::atomic<int>{std::__atomic_base<int>{0}}}, false}}}’ is not a constant expression
x.cc: In member function ‘virtual uint8_t* ProtobufCMessageOptions::_InternalSerialize(uint8_t*, google::protobuf::io::EpsCopyOutputStream*) const’:
x.cc:499:1: warning: no return statement in function returning non-void [-Wreturn-type]
  499 | }
      | ^

$ g++ 12.ii -c -std=c++17
x.cc: In member function ‘virtual uint8_t* ProtobufCMessageOptions::_InternalSerialize(uint8_t*, google::protobuf::io::EpsCopyOutputStream*) const’:
x.cc:499:1: warning: no return statement in function returning non-void [-Wreturn-type]
  499 | }
      | ^
Comment 1 Jonathan Wakely 2022-05-05 11:55:11 UTC
The constexpr rules in C++11 are much stricter, this is probably user error (i.e. C++20 constinit can't be used here).
Comment 2 Marek Polacek 2022-05-05 14:38:43 UTC
Reduced.  Works with -std=c++17 but not 14.  It's about the union in ProtobufCFileOptionsDefaultTypeInternal.


class Message {
  virtual int GetMetadata();
};
class ProtobufCFileOptions : Message {
public:
  constexpr ProtobufCFileOptions(int);
  bool no_generate_;
  bool const_strings_;
  bool use_oneof_field_name_;
  bool gen_pack_helpers_;
  bool gen_init_helpers_;
};
constexpr ProtobufCFileOptions::ProtobufCFileOptions(int)
    : no_generate_(), const_strings_(), use_oneof_field_name_(),
      gen_pack_helpers_(), gen_init_helpers_() {}
struct ProtobufCFileOptionsDefaultTypeInternal {
  constexpr ProtobufCFileOptionsDefaultTypeInternal() : _instance({}) {}
  union {
    ProtobufCFileOptions _instance;
  };
} __constinit _ProtobufCFileOptions_default_instance_;
Comment 3 Jakub Jelinek 2022-05-05 14:48:37 UTC
Related
class Message {
  virtual int GetMetadata();
};
class ProtobufCFileOptions : Message {
public:
  constexpr ProtobufCFileOptions(int);
  bool no_generate_;
  bool const_strings_;
  bool use_oneof_field_name_;
  bool gen_pack_helpers_;
  bool gen_init_helpers_;
};
constexpr ProtobufCFileOptions::ProtobufCFileOptions(int)
    : no_generate_(), const_strings_(), use_oneof_field_name_(),
      gen_pack_helpers_(), gen_init_helpers_() {}
struct ProtobufCFileOptionsDefaultTypeInternal {
  constexpr ProtobufCFileOptionsDefaultTypeInternal() : _instance({}) {}
  union {
    ProtobufCFileOptions _instance;
  };
};
constexpr ProtobufCFileOptionsDefaultTypeInternal _ProtobufCFileOptions_default_instance_;
is rejected starting with r10-7313-gb599bf9d6d1e180d350b71e51e08a66a1bb1546a
when using -std=c++11 or -std=c++14.
Though, I don't see how it could be changing the active union member when there is just one member...
Comment 4 Marek Polacek 2022-05-05 14:51:03 UTC
I think this is a genuine bug that started with r10-7313-gb599bf9d6d1e18.
Comment 5 Marek Polacek 2022-05-05 14:57:51 UTC
// PR c++/105491

struct V {
  virtual int foo();
};
struct S : V {
  constexpr S(int) : b() { }
  bool b;
};
struct W {
  constexpr W() : s({}) {}
  union {
    S s;
  };
};
constexpr W w;
Comment 6 Marek Polacek 2022-05-05 15:01:33 UTC
And since it only happens with a polymorphic class, my bet is that we think there are two members because one is the artificial vtable for S and the other is the bool.  I can poke more.
Comment 7 Patrick Palka 2022-05-06 13:47:38 UTC
This seems to fix it:

--- a/gcc/cp/constexpr.cc
+++ b/gcc/cp/constexpr.cc
@@ -1886,7 +1886,8 @@ cxx_eval_internal_function (const constexpr_ctx *ctx, tree t,
 static void
 clear_no_implicit_zero (tree ctor)
 {
-  if (CONSTRUCTOR_NO_CLEARING (ctor))
+  if (CONSTRUCTOR_NO_CLEARING (ctor)
+      || TREE_CODE (TREE_TYPE (ctor)) == UNION_TYPE)
     {
       CONSTRUCTOR_NO_CLEARING (ctor) = false;
       for (auto &e: CONSTRUCTOR_ELTS (ctor))


When recursively clearing CONSTRUCTOR_NO_CLEARING, it could be the case that a union's initializer has CONSTRUCTOR_NO_CLEARING already cleared but its sub-aggregates don't, because for unions CONSTRUCTOR_NO_CLEARING is overloaded to also track whether we're in the process of initializing a union member.
Comment 8 GCC Commits 2022-05-09 13:53:50 UTC
The master branch has been updated by Patrick Palka <ppalka@gcc.gnu.org>:

https://gcc.gnu.org/g:0c7bce0ac184c057bacad9c8e615ce82923835fd

commit r13-211-g0c7bce0ac184c057bacad9c8e615ce82923835fd
Author: Patrick Palka <ppalka@redhat.com>
Date:   Mon May 9 09:53:27 2022 -0400

    c++: constexpr init of union sub-aggr w/ base [PR105491]
    
    Here ever since r10-7313-gb599bf9d6d1e18, reduced_constant_expression_p
    in C++11/14 is rejecting the marked sub-aggregate initializer (of type S)
    
      W w = {.D.2445={.s={.D.2387={.m=0}, .b=0}}};
                         ^
    ultimately because said initializer has CONSTRUCTOR_NO_CLEARING set,
    hence the function must verify that all fields of S are initialized.
    And before C++17 it doesn't expect to see base class fields (since
    next_initializable_field skips over them), so the presence thereof
    causes r_c_e_p to return false.
    
    The reason r10-7313-gb599bf9d6d1e18 causes this is because in that
    commit we began using CONSTRUCTOR_NO_CLEARING to precisely track whether
    we're in middle of activating a union member.  This ends up affecting
    clear_no_implicit_zero, which recurses into sub-aggregate initializers
    only if the outer initializer has CONSTRUCTOR_NO_CLEARING set.  After
    that commit, the outer union initializer above no longer has the flag
    set at this point and so clear_no_implicit_zero no longer recurses into
    the marked inner initializer.
    
    But arguably r_c_e_p should be able to accept the marked initializer
    regardless of whether CONSTRUCTOR_NO_CLEARING is set.  The primary bug
    therefore seems to be that r_c_e_p relies on next_initializable_field
    which skips over base class fields in C++11/14.  To fix this, this patch
    introduces a new helper function next_subobject_field which is like
    next_initializable_field except that it never skips base class fields,
    and makes r_c_e_p use it.  This patch then renames next_initializable_field
    to next_aggregate_field (and makes it skip over vptr fields again).
    
            PR c++/105491
    
    gcc/cp/ChangeLog:
    
            * call.cc (field_in_pset): Adjust after next_initializable_field
            renaming.
            (build_aggr_conv): Likewise.
            (convert_like_internal): Likewise.
            (type_has_extended_temps): Likewise.
            * class.cc (default_init_uninitialized_part): Likewise.
            (finish_struct): Likewise.
            * constexpr.cc (cx_check_missing_mem_inits): Likewise.
            (reduced_constant_expression_p): Use next_subobject_field
            instead.
            * cp-gimplify.cc (get_source_location_impl_type): Adjust after
            next_initializable_field renaming.
            (fold_builtin_source_location): Likewise.
            * cp-tree.h (next_initializable_field): Rename to ...
            (next_aggregate_field): ... this.
            (next_subobject_field): Declare.
            * decl.cc (next_aggregate_field): Renamed from ...
            (next_initializable_field): ... this.  Skip over vptr fields
            again.
            (next_subobject_field): Define.
            (reshape_init_class): Adjust after next_initializable_field
            renaming.
            * init.cc (build_value_init_noctor): Likewise.
            (emit_mem_initializers): Likewise.
            * lambda.cc (build_capture_proxy): Likewise.
            * method.cc (build_comparison_op): Likewise.
            * pt.cc (maybe_aggr_guide): Likewise.
            * tree.cc (structural_type_p): Likewise.
            * typeck2.cc (split_nonconstant_init_1): Likewise.
            (digest_init_r): Likewise.
    
    gcc/testsuite/ChangeLog:
    
            * g++.dg/cpp0x/constexpr-union7.C: New test.
            * g++.dg/cpp0x/constexpr-union7a.C: New test.
            * g++.dg/cpp2a/constinit17.C: New test.
Comment 9 Martin Liška 2022-05-16 08:46:13 UTC
Fixed on master.
Comment 10 GCC Commits 2022-06-01 12:51:26 UTC
The releases/gcc-12 branch has been updated by Patrick Palka <ppalka@gcc.gnu.org>:

https://gcc.gnu.org/g:e30b73bad9486f11b6b0022ae4a3edfc0f9da4bb

commit r12-8445-ge30b73bad9486f11b6b0022ae4a3edfc0f9da4bb
Author: Patrick Palka <ppalka@redhat.com>
Date:   Wed Jun 1 08:47:25 2022 -0400

    c++: constexpr init of union sub-aggr w/ base [PR105491]
    
    Here ever since r10-7313-gb599bf9d6d1e18, reduced_constant_expression_p
    in C++11/14 is rejecting the marked sub-aggregate initializer (of type S)
    
      W w = {.D.2445={.s={.D.2387={.m=0}, .b=0}}};
                         ^
    ultimately because said initializer has CONSTRUCTOR_NO_CLEARING set,
    hence the function must verify that all fields of S are initialized.
    And before C++17 it doesn't expect to see base class fields (since
    next_initializable_field skips over them), so the presence thereof
    causes r_c_e_p to return false.
    
    The reason r10-7313-gb599bf9d6d1e18 causes this is because in that
    commit we began using CONSTRUCTOR_NO_CLEARING to precisely track whether
    we're in middle of activating a union member.  This ends up affecting
    clear_no_implicit_zero, which recurses into sub-aggregate initializers
    only if the outer initializer has CONSTRUCTOR_NO_CLEARING set.  After
    that commit, the outer union initializer above no longer has the flag
    set at this point and so clear_no_implicit_zero no longer recurses into
    the marked inner initializer.
    
    But arguably r_c_e_p should be able to accept the marked initializer
    regardless of whether CONSTRUCTOR_NO_CLEARING is set.  The primary bug
    therefore seems to be that r_c_e_p relies on next_initializable_field
    which skips over base class fields in C++11/14.  To fix this, this patch
    introduces a new helper function next_subobject_field which is like
    next_initializable_field except that it never skips base class fields,
    and makes r_c_e_p use it.  This patch then renames next_initializable_field
    to next_aggregate_field (and makes it skip over vptr fields again).
    
    NB: This minimal backport of r13-211-g0c7bce0ac184c0 for 12.2 just adds
    next_subobject_field and makes reduced_constant_expression_p use it.
    
            PR c++/105491
    
    gcc/cp/ChangeLog:
    
            * constexpr.cc (reduced_constant_expression_p): Use
            next_subobject_field instead.
            * cp-tree.h (next_subobject_field): Declare.
            * decl.cc (next_subobject_field): Define.
    
    gcc/testsuite/ChangeLog:
    
            * g++.dg/cpp0x/constexpr-union7.C: New test.
            * g++.dg/cpp0x/constexpr-union7a.C: New test.
            * g++.dg/cpp2a/constinit17.C: New test.
    
    (cherry picked from commit 0c7bce0ac184c057bacad9c8e615ce82923835fd)
Comment 11 Patrick Palka 2022-06-01 12:53:15 UTC
Now fixed for 12.2 as well.
Comment 12 Jakub Jelinek 2022-06-28 10:49:15 UTC
GCC 10.4 is being released, retargeting bugs to GCC 10.5.
Comment 13 Patrick Palka 2023-05-21 15:05:25 UTC
Fixed for GCC 12.2+.  The patch unfortunately doesn't apply cleanly to older release branches, and would be non-trivial to adapt.