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

Fix Rust codegen escaping field in tables. #7659

Merged
merged 19 commits into from Dec 15, 2022
Merged

Conversation

CasperN
Copy link
Collaborator

@CasperN CasperN commented Nov 22, 2022

Fixes #7617 - the problem is that if you have a union in a table with the field name "type", the Rust code-gen escaped "type" to "type_" before appending "_type"` which does not equal the magic field "type_type" which is generated by flatc.

@github-actions github-actions bot added c# c++ codegen Involving generating code from schema dart rust labels Nov 22, 2022
@CasperN
Copy link
Collaborator Author

CasperN commented Nov 28, 2022

@dbaileychess hey can you take a look at the CI failures? I can't quite figure out what's broken, though it appears unrelated to Rust changes

@enum-class
Copy link
Contributor

Hi @CasperN ,
The problem is when we have a field that match with one of keyword, the namer add suffix (underline here). As in the comments describe the problem previously.
On the other hand, the parser add an auto-generated field for union type. That means, we have 2 field per Union usage. one of them has exact name of the field, the other have name + suffix (_type) link.
So we have 2 method for union, one return Union type, one pointer to union field.

#[inline]
  pub fn type_type(&self) -> KeywordsInUnion {
    // Safety:
    // Created from valid Table for this object
    // which contains a valid value in this slot
    unsafe { self._tab.get::<KeywordsInUnion>(Table2::VT_TYPE_TYPE, Some(KeywordsInUnion::NONE)).unwrap()}
  }
  #[inline]
  pub fn type_(&self) -> Option<flatbuffers::Table<'a>> {
    // Safety:
    // Created from valid Table for this object
    // which contains a valid value in this slot
    unsafe { self._tab.get::<flatbuffers::ForwardsUOffset<flatbuffers::Table<'a>>>(Table2::VT_TYPE_, None)}
  }

As you can see the second one match with keyword and get an underline suffix.

Now, When code generator wants to use first method to get union type in code, by mistake, take the second one, as result we add "_type" suffix to fix the problem, but we should have use the second one and it does not need to add suffix and use another variable name DISCRIMINAT.

 #[inline]
  #[allow(non_snake_case)]
  pub fn type__as_static_(&self) -> Option<KeywordsInTable<'a>> {
    if self.type_type() == KeywordsInUnion::static_ {
      self.type_().map(|t| {
       // Safety:
       // Created from a valid Table for this object
       // Which contains a valid union in this slot
       unsafe { KeywordsInTable::init_from_table(t) }
     })
    } else {
      None
    }
  }

The same problem exist in c++ code generator. example of fbs:

table Null {                                                                    
}                                                                               
                                                                                
union DType {                                                                   
  Null,                                                                         
}                                                                               
                                                                                
table Field {                                                                   
  /// This is the type of the decoded value if the field is dictionary encoded. 
  static: DType;                                                                
  saman: DType;                                                                 
} 

the result should be something like this:

  DType static_type() const {                                                   
    return static_cast<DType>(GetField<uint8_t>(VT_STATIC_TYPE, 0));            
  }                                                                             
  /// This is the type of the decoded value if the field is dictionary encoded. 
  const void *static_() const {                                                 
    return GetPointer<const void *>(VT_STATIC_);                                
  } 
  template<typename T> const T *static__as() const;                             
  const Null *static__as_Null() const {                                         
    // <= static__type is WRONG !!!
    return static__type() == DType_Null ? static_cast<const Null *>(static_()) : nullptr;
  }

IMO the best way to fix these problems is to pick the correct method.
cc: @dbaileychess

@dbaileychess
Copy link
Collaborator

@dbaileychess hey can you take a look at the CI failures? I can't quite figure out what's broken, though it appears unrelated to Rust changes

I made some fixes to the MacOS CI yesterday, you just have to update (rebase/merge) to trigger it again.

@CasperN
Copy link
Collaborator Author

CasperN commented Dec 8, 2022

@dbaileychess can you approve this?

@CasperN CasperN enabled auto-merge (squash) December 8, 2022 05:10
@CasperN
Copy link
Collaborator Author

CasperN commented Dec 13, 2022

Ping @dbaileychess for LGTM

@dbaileychess
Copy link
Collaborator

Sorry for the delay!

@CasperN CasperN merged commit a078130 into google:master Dec 15, 2022
sunwen18 pushed a commit to sunwen18/flatbuffers that referenced this pull request Dec 25, 2022
* Fix Rust codegen escaping  field in tables.

* other gencode

* gencode

* removed a debug print

* regen code

Co-authored-by: Casper Neo <cneo@google.com>
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
c# c++ codegen Involving generating code from schema dart rust
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[rust] Unions named after reserved words result in code that won't compile
3 participants