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

[Feature] Add custom union ID syntax #65

Merged
merged 5 commits into from
Feb 20, 2023

Conversation

eval-exec
Copy link
Collaborator

@eval-exec eval-exec commented Feb 7, 2023

This PR want to close #59

working in progress.

  • changes for Pest grammar, Parser and IR
  • code generator for rust
  • code generator for C
  • unit test
  • integration test
  • back compatibility test

@eval-exec
Copy link
Collaborator Author

eval-exec commented Feb 7, 2023

These schema are equal:

union Foo {
	a,
	b,
	c,
}

union Foo {
	a : 0,
	b : 1,
	c : 2,
}


union Foo {
	a,
	b : 1,
	c,
}

These are equal too:

union Foo {
	a : 1,
	b,
	c : 10,
        d,
}
union Foo {
	c : 10,
        d,
	a : 1,
	b,
}
union Foo {
	a : 1,
	b : 2,
	c : 10,
        d : 11,
}

These schema are bad syntax:

union Foo {
	a : 9,
	b : 9,
}

union Foo {
	a : 10,
	b,
	c,
        d : 9,
        e,
        f,
}

Implementation Detail

For compatibility with older versions, I added a pest code block for customize union_item_decl:

custom_union_item_decl       =  {
                                    identifier ~ (brk)* ~ ":" ~ (brk)* ~
                                    number_greater_or_equal_than_zero     ~ (brk)* ~
                                    field_end
                                }

And added a new struct for ast::raw::UnionDecl:

#[derive(Debug, Property)]
pub(crate) struct CustomUnionItemDecl {
    typ: String,
    id: usize,
}
#[derive(Debug, Property)]
pub(crate) struct UnionDecl {
    name: String,
    items: Vec<CustomUnionItemDecl>,
    imported_depth: usize,
}

Then, parser can recognize molecule schema like these:

union Foo {
	a,
        b,
}

union Foo {
	a : 0,
	b : 1,
}
union Foo {
	a,
	b : 10,
}

@eval-exec eval-exec force-pushed the feat/customize-union-id branch 3 times, most recently from 0426e2e to bcfbf29 Compare February 13, 2023 05:13
@eval-exec
Copy link
Collaborator Author

eval-exec commented Feb 13, 2023

Compatibility test and integration test haven't been completed, but the main logic and unit test of "custom union ID feature" are ready for review.
@yangby-cryptape @driftluo @zhangsoledad

@eval-exec eval-exec marked this pull request as ready for review February 13, 2023 05:19
@quake
Copy link
Member

quake commented Feb 13, 2023

can we follow other struct field definition convention by using : instead of = ?

union Foo {
	a: 0,
	b: 1,
	c: 2,
}

@eval-exec eval-exec force-pushed the feat/customize-union-id branch 2 times, most recently from aaac172 to 6ad5e07 Compare February 13, 2023 23:21
@eval-exec eval-exec force-pushed the feat/customize-union-id branch 2 times, most recently from 57a4e05 to 5b86f9d Compare February 14, 2023 05:46
@eval-exec eval-exec changed the title [WIP] add customization union id syntax [Feature] Add custom union ID syntax Feb 14, 2023
@eval-exec
Copy link
Collaborator Author

eval-exec commented Feb 16, 2023

@yangby-cryptape The CI jobs are using v1.58.1, Do you think it's necessary to upgrade molecule's rust toolchain to v1.67.1 ?

https://github.com/nervosnetwork/molecule/blob/master/.github/workflows/ci.yaml#L24

@quake
Copy link
Member

quake commented Feb 16, 2023

@yangby-cryptape The CI jobs are using v1.58.1, Do you think it's necessary to upgrade molecule's rust toolchain to v1.67.1 ?

https://github.com/nervosnetwork/molecule/blob/master/.github/workflows/ci.yaml#L24

Let's do it with a new pr

tools/codegen/src/ir/mod.rs Outdated Show resolved Hide resolved
@eval-exec eval-exec requested review from quake and removed request for yangby-cryptape February 16, 2023 09:12
quake
quake previously approved these changes Feb 16, 2023
@eval-exec
Copy link
Collaborator Author

eval-exec commented Feb 17, 2023

@quake I didn't change any code after your review, but I rebased the code to make commits message more neat and more convenient for review. Please re-trigger the github CI action.

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.

Proposed new syntax to support custom implementation of union id
3 participants