Skip to content

Conversation

@CasperN
Copy link
Collaborator

@CasperN CasperN commented Jan 24, 2021

First steps towards #6053:

  • Strings can have any string as a default
  • vectors can only have the empty vector as a default

This should go in after #6420

@aardappel
Copy link
Collaborator

Please rebase.

@CasperN
Copy link
Collaborator Author

CasperN commented Feb 2, 2021

Rebased and added more tests. @aardappel, PTAL again

Copy link
Collaborator

@aardappel aardappel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This generally looks good! Though not sure if this should go in without at least 1 language that uses this data.. I understand you're wanting to keep commits small, but it also has value to see new data actually used.

@aardappel
Copy link
Collaborator

Can you undo the unrelated clang-format changes?

Are you using our script, or at least git-clang-format ? We typically only want changed code to be formatted, not the whole file.

@github-actions github-actions bot added the rust label Feb 8, 2021
@CasperN CasperN changed the title Parser support for default strings and vectors Default strings and vectors: Parser + Rust support Feb 8, 2021
@aardappel
Copy link
Collaborator

This looks great.. ready to merge, unless you want anyone else to review it.

@CasperN
Copy link
Collaborator Author

CasperN commented Feb 12, 2021

@krojew, can I also get your review?

@CasperN CasperN merged commit 86401e0 into google:master Feb 12, 2021
@CasperN CasperN deleted the def branch February 12, 2021 14:41
@vglavnyy
Copy link
Contributor

@CasperN
After this commit this test is failed:

  using flatbuffers::Parser;
  TEST_EQ(false, Parser().Parse("enum e:e{}table T{p:[e]=[]"));

with assertion:

Assertion failed: field->value.constant == "0", file E:\github\flatbuffers\src\idl_parser.cpp, line 917

Could you take a look at this problem?

@aardappel
Copy link
Collaborator

Should probably be fixed before #6353

@CasperN
Copy link
Collaborator Author

CasperN commented Mar 3, 2021

dang it, I made the same mistake while doing optional scalars 🙄

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 rust

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants