Skip to content

Fixes for Rust Composites #921

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

Closed

Conversation

adam-talos
Copy link
Contributor

@adam-talos adam-talos commented Dec 18, 2022

Fixed several issues:
1: LibRsDef explicitly use crate:: to disambiguate from built in mods like bool
2: Added acting_version field to Composite structs to fix compilation errors when using Composite structs. This is an incomplete implementation because the parent doesn't pass the acting_version to the composite because you need to change the signature of wrap(parent, offset) to include the acting_version, so this version just ensures that if the acting_version isn't set on the composite, it disregards the version check.
3: fixed primitiveArrayDecoder to return an empty array of the right size if less than version required.
4: If a name of an element clashes with a rust type, prefix it with r#, which is the preferred way of dealing with clashes with keywords

aeron-io#1: LibRsDef explicitly us crate:: to disambiguate from built in mods like bool
aeron-io#2: Added acting_version field to Composite structs to fix compilation errors when using Composite structs.  This is an incomplete implementation because the parent doesn't pass the acting_version to the composite because you need to change the signature of wrap(parent, offset) to include the acting_version, so this version just ensures that if the acting_version isn't set on the composite, it disregards the version check.
aeron-io#3: fixed primitiveArrayDecoder to return an empty array of the right size if less than version required.
@mikeb01
Copy link
Contributor

mikeb01 commented Jan 3, 2023

This is failing the CI tests, please could you look into the failures and make the necessary corrections.

@vyazelenko
Copy link
Contributor

@adam-talos The build fails with the CheckStyle errors:

$ ./gradlew
[ant:checkstyle] [ERROR] simple-binary-encoding/sbe-tool/src/main/java/uk/co/real_logic/sbe/generation/rust/RustGenerator.java:942: Line is longer than 120 characters (found 128). [LineLength]
[ant:checkstyle] [ERROR] simple-binary-encoding/sbe-tool/src/main/java/uk/co/real_logic/sbe/generation/rust/RustGenerator.java:957: Line is longer than 120 characters (found 128). [LineLength]
[ant:checkstyle] [ERROR] simple-binary-encoding/sbe-tool/src/main/java/uk/co/real_logic/sbe/generation/rust/RustGenerator.java:976: Line is longer than 120 characters (found 124). [LineLength]

@vyazelenko
Copy link
Contributor

@adam-talos This PR has merge conflicts and failing tests. Are you going to fix it or shall we close it?

@adam-talos
Copy link
Contributor Author

@adam-talos This PR has merge conflicts and failing tests. Are you going to fix it or shall we close it?

I've switched projects and don't have time to address this issue so you can close it. If I have time to polish it, I can resubmit.

@mjpt777 mjpt777 closed this Aug 8, 2023
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

Successfully merging this pull request may close these issues.

4 participants