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

Repeated field names in union are accepted #133

Open
jonjove opened this issue Aug 18, 2022 · 2 comments
Open

Repeated field names in union are accepted #133

jonjove opened this issue Aug 18, 2022 · 2 comments

Comments

@jonjove
Copy link

jonjove commented Aug 18, 2022

What problem does your feature solve?

union Test switch (int v)
{
case 0:
     int x;
case 1:
     int x;
}

compiles successfully but breaks xdrc due to the repeated x. The fix for a real example that broke stellar-core can be seen in stellar/stellar-xdr#20.

What would you like to see?

This shouldn't compile.

What alternatives are there?

TODO

@jonjove
Copy link
Author

jonjove commented Aug 18, 2022

This is specifically mentioned in RFC-4506 section 6.4 (4)

Similarly, variable names must be unique within the scope of struct and union declarations. Nested struct and union declarations create new scopes.

@leighmcculloch
Copy link
Member

leighmcculloch commented Aug 18, 2022

I don't think it is worth making it not compile, as it has no material impact on the generated Rust code. Xdrgen is not focused on being an exact-perfect implementation of XDR, but a practical / product one. There are also a many other ways that xdrgen and xdrc differ in what they allow and support (non-option circular refs, enum variants, union default arms, and others), so making this one case not compile would not holistically address the real issue.

compiles successfully but breaks xdrc due to the repeated x

The real issue of why this is inconvenient is that we make changes to the XDR without checking if the change compiles across all the generators we use in the Stellar ecosystem. It might be worth addressing that.

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

No branches or pull requests

2 participants