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

[Rust] #7659 broke union-typed fields with number in name #7782

Closed
cloneable opened this issue Jan 12, 2023 · 9 comments
Closed

[Rust] #7659 broke union-typed fields with number in name #7782

cloneable opened this issue Jan 12, 2023 · 9 comments
Labels

Comments

@cloneable
Copy link

cloneable commented Jan 12, 2023

The new flatc 23.1.4 generates broken Rust code when a field with a union type contains a number.

namespace test;

table FieldTable {}

union FieldUnion {
  f :FieldTable (id: 0),
}

table RootTable {
  field42 :FieldUnion (id: 1);
}

root_type RootTable;

This produces references to field_42_type() in the generated code, but the method is correctly called field42_type().

$ grep -i -B3 -A3 field_42_type src/test_generated.rs
        #[inline]
        #[allow(non_snake_case)]
        pub fn field42_as_f(&self) -> Option<FieldTable<'a>> {
            if self.field_42_type() == FieldUnion::f {
                self.field42().map(|t| {
                    // Safety:
                    // Created from a valid Table for this object
--
            use self::flatbuffers::Verifiable;
            v.visit_table(pos)?
                .visit_union::<FieldUnion, _>(
                    "field_42_type",
                    Self::VT_FIELD42_TYPE,
                    "field42",
                    Self::VT_FIELD42,
--
        fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
            let mut ds = f.debug_struct("RootTable");
            ds.field("field42_type", &self.field42_type());
            match self.field_42_type() {
                FieldUnion::f => {
                    if let Some(x) = self.field42_as_f() {
                        ds.field("field42", &x)

I pinpointed this to commit a078130 from #7659. flatc built from parent commit produces correct code.

@cloneable
Copy link
Author

@CasperN, could you take a look?

@cloneable
Copy link
Author

Hi @dbaileychess! You reviewed the change. Could you take a look? Maybe this is an easy fix? This is a regression in the code generator for rust.

@TethysSvensson
Copy link
Contributor

Ping @CasperN @dbaileychess.

@TJKoury
Copy link
Contributor

TJKoury commented Mar 3, 2023

#7111 Related

@TethysSvensson
Copy link
Contributor

@dbaileychess Since this has been fixed now and v23.5.26 is out, could you also publish that release to crates.io?

@dbaileychess
Copy link
Collaborator

@CasperN you do this right?

@CasperN
Copy link
Collaborator

CasperN commented Jun 3, 2023

yeah, I can do that

Done

Copy link

github-actions bot commented Dec 2, 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 Dec 2, 2023
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 Dec 16, 2023
jochenparm pushed a commit to jochenparm/flatbuffers that referenced this issue Oct 29, 2024
… (google#7964)

Co-authored-by: Derek Bailey <derekbailey@google.com>
jochenparm pushed a commit to jochenparm/flatbuffers that referenced this issue Oct 29, 2024
… (google#7964)

Co-authored-by: Derek Bailey <derekbailey@google.com>
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

5 participants