Skip to content

solidity grammar: add a new rule usingAliases for usingDirective #14788

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

Merged

Conversation

jeasonstudio
Copy link
Contributor

Thank you very much for providing the perfect solidity antlr4 grammar in the official repository, I encountered some problems when using it to build the solidity-antlr4 project (ast parser). This Pull-Request fixes the issue.

In the original grammar definition, usingDirective is hard to parse by the parser runtime:

/**
* Using directive to attach library functions and free functions to types.
* Can occur within contracts and libraries and at the file level.
*/
usingDirective: Using (identifierPath | (LBrace identifierPath (As userDefinableOperator)? (Comma identifierPath (As userDefinableOperator)?)* RBrace)) For (Mul | typeName) Global? Semicolon;

For example, my code is like this:

using { A, B as + } for uint;

Here is how my parser handles it (using antlr4 typescript as an example):

const visitUsingDirective = (ctx: UsingDirectiveContext) => {
  const isUsingAlias = !!ctx.LBrace() && !!ctx.RBrace(); // not good.
  if (!isUsingAlias) {
    const name = ctx.identifierPath(0); // shouldn't be an array.
  } else {
    const names = ctx.identifierPath(); // symbols.
    const alias = ctx.userDefinableOperator(); // aliases(operators), but it can't correspond with symbol one by one.
  }
};

So I added a new rule usingAliases to solve this problem, and the rule designed refer to the definition of importDirective:

usingDirective:
  Using (
    identifierPath
    | (LBrace usingAliases (Comma usingAliases)* RBrace)
  ) For (Mul | typeName) Global? Semicolon;

usingAliases: identifierPath (As userDefinableOperator)?;

Hope it can be processed soon, thanks.

Copy link

Thank you for your contribution to the Solidity compiler! A team member will follow up shortly.

If you haven't read our contributing guidelines and our review checklist before, please do it now, this makes the reviewing process and accepting your contribution smoother.

If you have any questions or need our help, feel free to post them in the PR or talk to us directly on the #solidity-dev channel on Matrix.

Copy link
Member

@r0qs r0qs left a comment

Choose a reason for hiding this comment

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

Hi @jeasonstudio thanks for the contribution. LGTM, could you please rebase it?

@jeasonstudio jeasonstudio force-pushed the grammar-provide-using-aliases branch from 724e6b1 to 2871b10 Compare February 5, 2024 09:55
@jeasonstudio
Copy link
Contributor Author

jeasonstudio commented Feb 5, 2024

Hi @jeasonstudio thanks for the contribution. LGTM, could you please rebase it?

I have rebased this branch, but ci reported an error, and I don‘t think it has anything to do with my pr.

Thank you for your review. @r0qs

Copy link
Member

@r0qs r0qs left a comment

Choose a reason for hiding this comment

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

Thanks! Yes, it is indeed unrelated (I did a workaround here: #14833). So I'm approving this anyway. But you may need to rebase it again if we merge the other PR first ;)

@jeasonstudio jeasonstudio force-pushed the grammar-provide-using-aliases branch from 2871b10 to a3e5f3f Compare February 5, 2024 14:00
@jeasonstudio
Copy link
Contributor Author

Thanks! Yes, it is indeed unrelated (I did a workaround here: #14833). So I'm approving this anyway. But you may need to rebase it again if we merge the other PR first ;)

done

@ekpyron ekpyron merged commit 1183284 into ethereum:develop Feb 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants