C++ port_def.inc: don't set PROTOBUF_CONSTINIT on mingw-w64

[1480c1]

Currently errors out any clang compilation on windows with mingw-w64 as std::mutex is not set with constexpr

full error:

../../src/google/protobuf/struct.pb.cc:42:76: error: variable does not have a constant initializer
PROTOBUF_CONSTINIT PROTOBUF_ATTRIBUTE_NO_DESTROY StructDefaultTypeInternal _Struct_default_instance_;
                                                                           ^~~~~~~~~~~~~~~~~~~~~~~~~
../../src/google/protobuf/struct.pb.cc:42:1: note: required by 'require_constant_initialization' attribute here
PROTOBUF_CONSTINIT PROTOBUF_ATTRIBUTE_NO_DESTROY StructDefaultTypeInternal _Struct_default_instance_;
^~~~~~~~~~~~~~~~~~
../../src\google/protobuf/port_def.inc:580:30: note: expanded from macro 'PROTOBUF_CONSTINIT'
\#define PROTOBUF_CONSTINIT [[clang::require_constant_initialization]]
                             ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../../src\google/protobuf/stubs/mutex.h:132:17: note: non-constexpr constructor 'mutex' cannot be used in a constant expression
  std::mutex mu_{};
                ^
../../src\google/protobuf/map_field.h:336:9: note: in call to 'WrappedMutex()'
        mutex_(GOOGLE_PROTOBUF_LINKER_INITIALIZED),
        ^
../../src\google/protobuf/map_field.h:485:9: note: in call to 'MapFieldBase({})'
      : MapFieldBase(tag) {}
        ^
../../src\google/protobuf/map_field.h:546:9: note: in call to 'TypeDefinedMapFieldBase({})'
      : TypeDefinedMapFieldBase<Key, T>(tag), impl_() {}
        ^
../../src/google/protobuf/struct.pb.cc:33:5: note: in call to 'MapField({})'
  : fields_(::PROTOBUF_NAMESPACE_ID::internal::ConstantInitialized{}){}
    ^
../../src/google/protobuf/struct.pb.cc:36:7: note: in call to 'Struct({})'
    : _instance(::PROTOBUF_NAMESPACE_ID::internal::ConstantInitialized{}) {}
      ^
../../src/google/protobuf/struct.pb.cc:42:76: note: in call to 'StructDefaultTypeInternal()'
PROTOBUF_CONSTINIT PROTOBUF_ATTRIBUTE_NO_DESTROY StructDefaultTypeInternal _Struct_default_instance_;
                                                                           ^
D:\mabs\msys64\mingw64\include\c++\10.2.0\bits/std_mutex.h:91:5: note: declared here
    mutex() noexcept = default;
    ^
1 error generated.

[1480c1]

How can I set a tag to fixed the Mergeable issue?

[acozzette]

@1480c1 I added the tags to fix the Mergeable error and run the CI tests. However I wonder if there is a better solution. Ideally we want to always set PROTOBUF_CONSTINIT to the best thing that the compiler supports. I'm not an expert on this but isn't std::mutex supposed to have a constexpr constructor? If the constructor isn't constexpr then is that a bug with MinGW?

[1480c1]

Based on my limited understanding of https://sourceforge.net/p/mingw-w64/mailman/mingw-w64-public/thread/2a2daec1.69321.14d0ff963e9.Coremail.lh_mouse%40126.com/ and https://sourceforge.net/p/mingw-w64/mailman/mingw-w64-public/thread/1360a75.a67e5.15412d233d0.Coremail.lh_mouse%40126.com/, this seems to be an issue inside the version of windows that gcc is compatible with, but @lhmouse can probably elaborate a bit more

edit notes: https://devblogs.microsoft.com/cppblog/constexpr-complete-for-vs-2015-rtm-c-11-compiler-c-17-stl/

In the STL, we’ve implemented every occurrence of constexpr in the current C++17 Working Paper N4527, with a very small number of exceptions:
...
mutex’s default constructor (needs to be significantly overhauled after we can drop support for XP targeting)

which leads me to assume gcc has std::mutex to not be constexpr to allow for xp targetting

[lhmouse]

That has been fixed since couple of years ago: mingw-w64/mingw-w64@8bba2a9

[1480c1]

Then is msys2's mingw-w64/clang broken?

[lhmouse]

It's because libstdc++'s mutex has a non-trivial destructor and is not a literal type. Why would you want a constexpr mutex?

[acozzette]

I'm not sure why we need a constexpr mutex, but we have this same problem with MSVC and work around it here:

protobuf/src/google/protobuf/stubs/mutex.h

Line 93 in 133e5e7

// In MSVC std::mutex does not have a constexpr constructor.

[sbenzaquen]

It's because libstdc++'s mutex has a non-trivial destructor and is not a literal type. Why would you want a constexpr mutex?

A constexpr constructor allows you to do constant initialization of the object, even if it is not a literal type.
Basically, you should be able to write:

// This is constant initialized, no problem with accessing the object before dynamic initialization.
constinit std::mutex x;

Note that constant initialization with constructors marked constexpr comes all the way from C++11.
C++20 added the constinit keyword so that you can guarantee that (and fail to compile otherwise). Without the keyword the failure mode is to just do dynamic initialization which could silently make code incorrect.

[acozzette]

I tried an alternative fix in #8318. @1480c1 Does that pull request fix the problem for you?

[1480c1]

That pr fixes compilation issues on both gcc and clang

[lhmouse]

It's because libstdc++'s mutex has a non-trivial destructor and is not a literal type. Why would you want a constexpr mutex?

A constexpr constructor allows you to do constant initialization of the object, even if it is not a literal type.

A class with a constexpr constructor is not necessarily a literal type that is required by a constexpr variable. The former does not request a trivial destructor, but the latter does.

You may declare the object (which has a non-static mutex member) as const instead of constexpr. If its constructor is constexpr the compiler will perform constant initialization as needed, and will not require it to have a trivial destructor.

Basically, you should be able to write:

// This is constant initialized, no problem with accessing the object before dynamic initialization.
constinit std::mutex x;

Note that constant initialization with constructors marked constexpr comes all the way from C++11.
C++20 added the constinit keyword so that you can guarantee that (and fail to compile otherwise). Without the keyword the failure mode is to just do dynamic initialization which could silently make code incorrect.

You don't tell the compiler to do the right thing (https://gcc.godbolt.org/z/b3c9Ts).