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

Need Support for Recursively Defined Message Types #311

Open
SallyTYT opened this issue Sep 15, 2023 · 2 comments
Open

Need Support for Recursively Defined Message Types #311

SallyTYT opened this issue Sep 15, 2023 · 2 comments
Labels
Affects Scripted Code Will change how generated code looks type: enhancement New feature or request

Comments

@SallyTYT
Copy link

SallyTYT commented Sep 15, 2023

grpc-labview now cannot support recursively defined message types , for example, the below message.

https://github.com/protocolbuffers/protobuf/blob/main/src/google/protobuf/descriptor.proto, line 120 - 153

message DescriptorProto {
  ... ...
  repeated DescriptorProto nested_type = 3;
  ... ...
}

When generating code using this proto file, it will report the below error,

image

I've confirmed in my Python and C++ test environments for gRPC that this recursively defined struct should use pass-by-reference rather than pass-by-value. The latter would result in infinite recursion and memory leaks. LabVIEW typedefs also have the capability to achieve a similar definition using control references, but currently, our toolchain doesn't support this. Could we add support for this feature?

Thank you!
Yongtao Tan

AB#2523253

@Rohith-Grandhi Rohith-Grandhi added type: enhancement New feature or request Affects Scripted Code Will change how generated code looks labels Sep 20, 2023
@sreekantham
Copy link

@Rohith-Grandhi, this is blocking development in MeasurementLink as well as we are wanting to include annotations.proto which includes this descriptors.proto. I will reach out to you on directly as well.

@jasonmreding
Copy link
Collaborator

In the case of annotations.proto, it would be nice if grpc-labview could just ignore the imports rather than throwing an error. In this case, the annotations are just markup to the proto file, and they don't really impact the generated code at all. Or at least they wouldn't have to impact codegen for grpc-labview since the annotations are usually used as part of reflection APIs utilized by more advanced features provided by web frameworks like ASP.NET Core.

This might be related to another item which I would consider a bug which is grpc-labview shouldn't always produce generated code for .proto files that are imported from other .proto files. Instead, I should have the option to link against a standard package which already includes the generated code. For example, this is how the C# language integration with grpc works. This would probably also require grpc-labview to infer or mandate the generated library name based on the proto package name rather than allowing the user to choose the name. Otherwise, there is no definitive way to generate/define the types since the library name is part of the type name in LV. This would also help minimize coercion dots for common imports/types that are generated as dependencies from multiple proto files.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Affects Scripted Code Will change how generated code looks type: enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

4 participants