Hello Thrift folks,
I think that the Thrift 0.17.0 C++ generator might generate - even for
simple examples - code that contains undefined behavior, and which
indeed stops compiling with Clang 15 in C++20 mode. Consider this simple
thrift specification:
┌────
│ struct StructA {
│ 11:required list<StructB> myBs
│ }
│
│ struct StructB
│ {
│ 1:required string someString
│ }
└────
I translate this to C++ using this command (and thrift v. 0.17.0):
┌────
│ thrift --gen cpp:no_skeleton -o out ./file.thrift
└────
The interesting parts are in the generated file `file_types.h'. You can
find the full generated file at
<https://gist.github.com/tinloaf/1f7ae57df383d7b87221598e4f7a700d>, but
these are the crucial parts:
┌────
│ class StructA;
│
│ class StructB;
│
│
│ class StructA : … {
│ public:
│
│ …
│ StructA() noexcept {
│ }
│
│ …
│ std::vector<StructB> myBs;
│ …
│ };
│
│ …
│
│ class StructB : … {
│ …
│ };
└────
The important part is that there is a _definition_ (not just a
declaration) of `StructA''s default constructor *before* `StructB' is
complete. This default constructor will implicitly call
`std::vector<StructB>''s default constructor.
The C++20 standard states in [vector.overview]/4 [1]:
An incomplete type T may be used when instantiating vector
if the allocator meets the allocator completeness
requirements. T shall be complete before any member of the
resulting specialization of vector is referenced.
So, having a member of type `std::vector<StructB>' (with `StructB' being
an incomplete type at that point) should be fine. However, implicitly
using its default constructor in `StructA()' is probably not. This is
also what Clang 15 tells me when I try to compile this with
`--std=c++20':
┌────
│ In file included from ./file_types.h:12:
│ In file included from
/opt/mentz/packages/x64-linux/include/thrift/Thrift.h:41:
│ In file included from
/usr/bin/../lib/gcc/x86_64-linux-gnu/12/../../../../include/c++/12/vector:64:
│
/usr/bin/../lib/gcc/x86_64-linux-gnu/12/../../../../include/c++/12/bits/stl_vector.h:367:35:
error: arithmetic on a pointer to an incomplete type 'StructB'
│ _M_impl._M_end_of_storage -
_M_impl._M_start);
│ ~~~~~~~~~~~~~~~~~~~~~~~~~ ^
│
/usr/bin/../lib/gcc/x86_64-linux-gnu/12/../../../../include/c++/12/bits/stl_vector.h:526:7:
note: in instantiation of member function 'std::_Vector_base<StructB,
std::allocator<StructB>>::~_Vector_base' requested here
│ vector() = default;
│ ^
│ ./file_types.h:34:3: note: in defaulted default constructor for
'std::vector<StructB>' first required here
│ StructA() noexcept {
│ ^
│ ./file_types.h:26:7: note: forward declaration of 'StructB'
│ class StructB;
└────
Here is a (closed as invalid) LLVM issue that is more or less the same
problem (it's the move constructor being used, not the default
constructor): <https://github.com/llvm/llvm-project/issues/57700>
I think this should be fixed by moving all member function definitions
from `*_types.h' into `*_types.cpp' (all types should be complete in the
`.cpp' files). It is not sufficient to just move the default
constructor, in my generated file I see at least `StructA::operator=='
and `StructA::operator!=' which also use members of
`std::vector<StructB>'.
I will gladly open an issue for this, but I think I would need somebody
to create a Jira account for me.
All the best, Lukas
[1]: <https://eel.is/c++draft/vector#overview-4>
--
________________________________________________________
Lukas Barth
MENTZ GmbH, Grillparzerstr. 18, 81675 Munich
P:+49 (0)89 4 18 68-258, F:+49 (0)89 4 18 68-160
E: ba...@mentz.net, www.mentz.net
Registered Office: Grillparzerstraße 18, 81675 Munich, Germany
President: Christoph Mentz, District Court Munich, HRB 91898
________________________________________________________
