-
Notifications
You must be signed in to change notification settings - Fork 536
[Rust] Invalid codegen for composite type with multiple fields and different sinceVersion #1057
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
Comments
I implemented the change to get the greatest version of inner fields instead of the first and it fixed the problem: PR #1058 |
@mward Can you have a look at the proposed fix? Thanks. |
I'll take a look, hopefully I'll have some time in the next day or two |
Thanks @marciorasf, this looks great! Sorry it took me so long to look at. |
PR #1058 was merged. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
For the sample schema:
CLI tool 1.34.1 generates Rust code that doesn't compile due to:
OutboundBusinessHeaderDecoder does implement
self.acting_version()
, but just for aP: Reader<'a> + ActingVersion + Default
:And error occurs because it's trying to use
self.acting_version()
for aP
that does not implementActingVersion
:It's the same error as the issue 1028, but for a slightly different schema.
On issue 1028, the type
OutboundBusinessHeader
had just one field:eventIndicator
. In this new issue,OutboundBusinessHeader
has two fields:sessionId
andeventIndicator
. It's the addition ofsessionID
that causes the error. More specifically, it's the addition ofsessionID
as the first field that breaks it, when I moved it to be the second field, the generated code compiled successfully.Looking at the PR that solved the other issue, the solution was to get the version of the first field instead of the version of the type itself. I guess it always works when there's just one field, but for the scenario where we have multiple fields AND the one with
version > 0
is not the first, that solution doesn't work.It seems to me that the solution is to get the version from the field with the greatest version instead of the first one, but I'm not sure about how this could impact other parts of code generation. Is that indeed the actual solution? Is there a better alternative?
The text was updated successfully, but these errors were encountered: