Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

protoc grpc plugin generates invalid c++ code because of name colisions #29617

Open
DS-Serafin opened this issue May 9, 2022 · 3 comments
Open

Comments

@DS-Serafin
Copy link

What version of protobuf and what language are you using?
Version: v3.129.2.
Language: C++

What operating system (Linux, Windows, ...) and version?
Ubuntu 20.04

What runtime / compiler are you using (e.g., python version or gcc version)
clang 12 with stdlibc++ (gcc-11)

What did you do?
create a proto service definition with service Service

What did you expect to see
Generated code compiles

What did you see instead?
The c++ code is generated but the generated code is not valid c++

hello-api.grpc.pb.h:93:9: error: member 'Service' has the same name as its class
  class Service : public ::grpc::Service {
        ^

A class Service is Generated (line 13) and this calss contains a nested class also called Service (line 93).
Same problem would be if you called your Service Stub.

I think all those internal classes should be prefixes with _ like _Service or something similar. Or protoc should refuse to compile the service.

As it is now you can have a running Service for example in go. Which no c++ client can connect.

See the protofile (remove .txt):
hello.proto.txt

And the generated gprc.pb.h (remove.txt):
hello.grpc.pb.h.txt

First tried to raise this against protobuf, but they say this should be raised here: protocolbuffers/protobuf#9925

@yashykt
Copy link
Member

yashykt commented May 10, 2022

We believe there are going to be keywords. We might look into warnings when certain keywords are used in 'protos'

@nicolasnoble
Copy link
Member

I'm tempted to flag this as "not a bug": every language will have a list of reserved keywords. Given the nature of protobuf and its plugins, it's going to be difficult to create a full list. You only stumbled upon a reserved keyword.

@DS-Serafin
Copy link
Author

DS-Serafin commented May 11, 2022

I disagree this is not a c++ keyword. The name of the generated class is 100% in control of protoc. I think it it common pratice to prefix generated code with something to minimize colisions. But giving warnings for possible bad names would be a good first step.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants