Skip to content

Conversation

@georgthegreat
Copy link
Contributor

@georgthegreat georgthegreat commented Jul 19, 2021

Apparently, #8532 was incorrect if applied to 3.17.x branch.

3.17.x changed code generation to mark SharedCtor() and SharedDtor() as inline in .pb.cc.

It looks like we have a compile-time undefined behavior in C++ now. As cppreference.com says:

The definition of an inline function <...> must be reachable in the translation unit where it is accessed (not necessarily before the point of access).

As protobuf allows custom plugins to generate custom code, there is no limitation on where SharedCtor couble be possible referenced from. In our case we have SharedCtor invoked from corresponding .pb.h code, thus triggering:

ld: error: undefined symbol: package::Message::SharedCtor()`
>>> referenced by file.pb.h:$$$$

If this patch is not applicable, I can take a look into changing the code generation, but doing this will be harder than removing inline.

@google-cla google-cla bot added the cla: yes label Jul 19, 2021
@georgthegreat
Copy link
Contributor Author

NB: the same problem applies to SharedDtor() though it is not broken in our case.

@acozzette
Copy link

@georgthegreat Would you mind running ./generate_descriptor_proto.sh? That will rebuild the checked-in generated code and should fix the failing test runs.

Apparently, protocolbuffers#8532 was incorrect if applied to 3.17.x branch.

3.17.x changed code generation to mark `SharedCtor()` and `SharedDtor()` as inline in .pb.cc.

It looks like we have a compile-time undefined behavior in C++ now. As cppreference.com [says](https://en.cppreference.com/w/cpp/language/inline):

_The definition of an inline function <...> must be reachable in the translation unit where it is accessed (not necessarily before the point of access)._

As protobuf allows custom plugins to generate custom code, there is no limitation on where SharedCtor couble be possible referenced from. In our case we have SharedCtor invoked from corresponding `.pb.h` code, thus triggering:
```
ld: error: undefined symbol: package::Message::SharedCtor()`
>>> referenced by file.pb.h:$$$$
```

If this patch is not applicable, I can take a look into changing the code generation, but doing this will be harder then removing _inline_.
@georgthegreat
Copy link
Contributor Author

@deannagarcia, @acozzette, can we proceed with merging of this PR?

@acozzette acozzette merged commit 7326b57 into protocolbuffers:master Jul 29, 2021
@georgthegreat georgthegreat deleted the patch-2 branch July 29, 2021 21:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants