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

Object API packs arrays of structs in reverse order [TS, master] #7021

Closed
AndyCampbellNvidia opened this issue Jan 14, 2022 · 4 comments
Closed
Labels

Comments

@AndyCampbellNvidia
Copy link

Generated TypeScript code in object-model API mostly does array reversal automatically to handle the backwards insertion order when packing, but for arrays-of-structs it does not.

Calls are generated to builder.createStructOffsetList with no reversal wrappers, and the function doesn't do any reversal logic internally.

The unit test code which should catch this failure is flawed, and does not verify correct ordering after pack/unpack round-trip (summing array elements is commutative):
https://github.com/google/flatbuffers/blob/master/tests/JavaScriptTest.js#L168

  let test_0 = monster.test4[0];
  let test_1 = monster.test4[1];
  assert.strictEqual(monster.test4.length, 2);
  assert.strictEqual(test_0.a + test_0.b + test_1.a + test_1.b, 100);
@dbaileychess
Copy link
Collaborator

@krojew do you want to take a look?

@dbaileychess
Copy link
Collaborator

@AndyCampbellNvidia Would you want to make a PR that fixes this? Ideally it would be the code gen that changes, but you could also edit createStructOffsetList to iterate backwards.

bulentv added a commit to zdrealityhub/flatbuffers that referenced this issue Oct 10, 2022
…google#7021)

    * Typescript / Javascript don't have fixed arrays but it is important to support for compatibility.

    * Generated TS code checks the length of the given array and do truncating / padding to conform to schema
bulentv added a commit to zdrealityhub/flatbuffers that referenced this issue Oct 10, 2022
…google#7021)

    * Typescript / Javascript don't have fixed arrays but it is important to support for compatibility.

    * Generated TS code checks the length of the given array and do truncating / padding to conform to schema
bulentv added a commit to zdrealityhub/flatbuffers that referenced this issue Oct 12, 2022
…oogle#7021)

* Typescript / Javascript don't have fixed arrays but it is important to support these languages for compatibility.

* Generated TS code checks the length of the given array and do truncating / padding to conform to the schema.

* Supports the both standard API and Object Based API.

* Added a test.

Co-authored-by: Mehmet Baker <mehmet.baker@zerodensity.tv>
Signed-off-by: Bulent Vural <bulent.vural@zerodensity.tv>
bulentv added a commit to zdrealityhub/flatbuffers that referenced this issue Oct 18, 2022
…oogle#7021)

* Typescript / Javascript don't have fixed arrays but it is important to support these languages for compatibility.

* Generated TS code checks the length of the given array and do truncating / padding to conform to the schema.

* Supports the both standard API and Object Based API.

* Added a test.

Co-authored-by: Mehmet Baker <mehmet.baker@zerodensity.tv>
Signed-off-by: Bulent Vural <bulent.vural@zerodensity.tv>
bulentv added a commit to zdrealityhub/flatbuffers that referenced this issue Oct 18, 2022
…oogle#7021)

* Typescript / Javascript don't have fixed arrays but it is important to support these languages for compatibility.

* Generated TS code checks the length of the given array and do truncating / padding to conform to the schema.

* Supports the both standard API and Object Based API.

* Added a test.

Co-authored-by: Mehmet Baker <mehmet.baker@zerodensity.tv>
Signed-off-by: Bulent Vural <bulent.vural@zerodensity.tv>
bulentv added a commit to zdrealityhub/flatbuffers that referenced this issue Oct 19, 2022
…oogle#7021)

* Typescript / Javascript don't have fixed arrays but it is important to support these languages for compatibility.

* Generated TS code checks the length of the given array and do truncating / padding to conform to the schema.

* Supports the both standard API and Object Based API.

* Added a test.

Co-authored-by: Mehmet Baker <mehmet.baker@zerodensity.tv>
Signed-off-by: Bulent Vural <bulent.vural@zerodensity.tv>
bulentv added a commit to zdrealityhub/flatbuffers that referenced this issue Oct 19, 2022
…oogle#7021)

* Typescript / Javascript don't have fixed arrays but it is important to support these languages for compatibility.

* Generated TS code checks the length of the given array and do truncating / padding to conform to the schema.

* Supports the both standard API and Object Based API.

* Added a test.

Co-authored-by: Mehmet Baker <mehmet.baker@zerodensity.tv>
Signed-off-by: Bulent Vural <bulent.vural@zerodensity.tv>
bulentv added a commit to zdrealityhub/flatbuffers that referenced this issue Oct 21, 2022
…oogle#7021)

* Typescript / Javascript don't have fixed arrays but it is important to support these languages for compatibility.

* Generated TS code checks the length of the given array and do truncating / padding to conform to the schema.

* Supports the both standard API and Object Based API.

* Added a test.

Co-authored-by: Mehmet Baker <mehmet.baker@zerodensity.tv>
Signed-off-by: Bulent Vural <bulent.vural@zerodensity.tv>
bulentv added a commit to zdrealityhub/flatbuffers that referenced this issue Oct 21, 2022
…oogle#7021)

* Typescript / Javascript don't have fixed arrays but it is important to support these languages for compatibility.

* Generated TS code checks the length of the given array and do truncating / padding to conform to the schema.

* Supports the both standard API and Object Based API.

* Added a test.

Co-authored-by: Mehmet Baker <mehmet.baker@zerodensity.tv>
Signed-off-by: Bulent Vural <bulent.vural@zerodensity.tv>
bulentv added a commit to zdrealityhub/flatbuffers that referenced this issue Oct 21, 2022
…oogle#7021)

* Typescript / Javascript don't have fixed arrays but it is important to support these languages for compatibility.

* Generated TS code checks the length of the given array and do truncating / padding to conform to the schema.

* Supports the both standard API and Object Based API.

* Added a test.

Co-authored-by: Mehmet Baker <mehmet.baker@zerodensity.tv>
Signed-off-by: Bulent Vural <bulent.vural@zerodensity.tv>
bulentv added a commit to zdrealityhub/flatbuffers that referenced this issue Oct 21, 2022
…oogle#7021)

* Typescript / Javascript don't have fixed arrays but it is important to support these languages for compatibility.

* Generated TS code checks the length of the given array and do truncating / padding to conform to the schema.

* Supports the both standard API and Object Based API.

* Added a test.

Co-authored-by: Mehmet Baker <mehmet.baker@zerodensity.tv>
Signed-off-by: Bulent Vural <bulent.vural@zerodensity.tv>
bulentv added a commit to zdrealityhub/flatbuffers that referenced this issue Oct 21, 2022
…oogle#7021)

    * Typescript / Javascript don't have fixed arrays but it is important to support these languages for compatibility.

    * Generated TS code checks the length of the given array and do truncating / padding to conform to the schema.

    * Supports the both standard API and Object Based API.

    * Added a test.

    Co-authored-by: Mehmet Baker <mehmet.baker@zerodensity.tv>
    Signed-off-by: Bulent Vural <bulent.vural@zerodensity.tv>

Signed-off-by: Bülent Vural <bulent.vural@zerodensity.tv>
bulentv added a commit to zdrealityhub/flatbuffers that referenced this issue Oct 26, 2022
…oogle#7021)

    * Typescript / Javascript don't have fixed arrays but it is important to support these languages for compatibility.

    * Generated TS code checks the length of the given array and do truncating / padding to conform to the schema.

    * Supports the both standard API and Object Based API.

    * Added a test.

    Co-authored-by: Mehmet Baker <mehmet.baker@zerodensity.tv>
    Signed-off-by: Bulent Vural <bulent.vural@zerodensity.tv>

Signed-off-by: Bülent Vural <bulent.vural@zerodensity.tv>
bulentv added a commit to zdrealityhub/flatbuffers that referenced this issue Oct 27, 2022
…oogle#7021)

    * Typescript / Javascript don't have fixed arrays but it is important to support these languages for compatibility.

    * Generated TS code checks the length of the given array and do truncating / padding to conform to the schema.

    * Supports the both standard API and Object Based API.

    * Added a test.

    Co-authored-by: Mehmet Baker <mehmet.baker@zerodensity.tv>
    Signed-off-by: Bulent Vural <bulent.vural@zerodensity.tv>

Signed-off-by: Bülent Vural <bulent.vural@zerodensity.tv>
bulentv added a commit to zdrealityhub/flatbuffers that referenced this issue Oct 27, 2022
…oogle#7021)

    * Typescript / Javascript don't have fixed arrays but it is important to support these languages for compatibility.

    * Generated TS code checks the length of the given array and do truncating / padding to conform to the schema.

    * Supports the both standard API and Object Based API.

    * Added a test.

    Co-authored-by: Mehmet Baker <mehmet.baker@zerodensity.tv>
    Signed-off-by: Bulent Vural <bulent.vural@zerodensity.tv>

Signed-off-by: Bülent Vural <bulent.vural@zerodensity.tv>
bulentv added a commit to zdrealityhub/flatbuffers that referenced this issue Oct 27, 2022
…oogle#7021)

    * Typescript / Javascript don't have fixed arrays but it is important to support these languages for compatibility.

    * Generated TS code checks the length of the given array and do truncating / padding to conform to the schema.

    * Supports the both standard API and Object Based API.

    * Added a test.

    Co-authored-by: Mehmet Baker <mehmet.baker@zerodensity.tv>
    Signed-off-by: Bulent Vural <bulent.vural@zerodensity.tv>

Signed-off-by: Bülent Vural <bulent.vural@zerodensity.tv>
bulentv added a commit to zdrealityhub/flatbuffers that referenced this issue Oct 27, 2022
…oogle#7021)

    * Typescript / Javascript don't have fixed arrays but it is important to support these languages for compatibility.

    * Generated TS code checks the length of the given array and do truncating / padding to conform to the schema.

    * Supports the both standard API and Object Based API.

    * Added a test.

    Co-authored-by: Mehmet Baker <mehmet.baker@zerodensity.tv>
    Signed-off-by: Bulent Vural <bulent.vural@zerodensity.tv>

Signed-off-by: Bülent Vural <bulent.vural@zerodensity.tv>
dbaileychess pushed a commit that referenced this issue Oct 29, 2022
#7581)

* [TS] Add support for fixed length arrays on Typescript (#5864) (#7021)

    * Typescript / Javascript don't have fixed arrays but it is important to support these languages for compatibility.

    * Generated TS code checks the length of the given array and do truncating / padding to conform to the schema.

    * Supports the both standard API and Object Based API.

    * Added a test.

    Co-authored-by: Mehmet Baker <mehmet.baker@zerodensity.tv>
    Signed-off-by: Bulent Vural <bulent.vural@zerodensity.tv>

Signed-off-by: Bülent Vural <bulent.vural@zerodensity.tv>

* Formatting & readability fixes on idl_gen_ts.cpp

Signed-off-by: Bülent Vural <bulent.vural@zerodensity.tv>

* Added array_test_complex.bfbs

Signed-off-by: Bülent Vural <bulent.vural@zerodensity.tv>

* TS arrays_test_complex: Remove bfbs and use  fbs directly

Signed-off-by: Bülent Vural <bulent.vural@zerodensity.tv>

Signed-off-by: Bülent Vural <bulent.vural@zerodensity.tv>
@github-actions
Copy link

github-actions bot commented Mar 4, 2023

This issue is stale because it has been open 6 months with no activity. Please comment or label not-stale, or this will be closed in 14 days.

@github-actions github-actions bot added the stale label Mar 4, 2023
@github-actions
Copy link

This issue was automatically closed due to no activity for 6 months plus the 14 day notice period.

@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Mar 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants