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

Jump to bottom

Prevent several cases of making duplicate definitions under the same name #317

Merged
merged 8 commits into from Mar 19, 2021
Merged

Prevent several cases of making duplicate definitions under the same name #317

merged 8 commits into from Mar 19, 2021

Conversation

cburgdorf
Copy link
Collaborator

@cburgdorf cburgdorf commented Mar 19, 2021 •
edited

What was wrong?

  1. It is currently possible to define structs with multiple conflicting fields (Yul compilation failed: DeclarationError: Function name struct_House_get_price_ptr already taken in this scope #310)
  2. It is currently possible to define multiple type definitions, structs or contracts in the same module with conflicting names
  3. It is currently possible to define a var in a block scope that is already defined in a parent scope (Yul compilation failed: Variable name $sum already taken in this scope. [NEW VARIANT of #223] #275)

How was it fixed?

  • Made add_type_def return Result<(), SemanticError> and consume it everywhere where it is used
  • Made add_field on struct return Result<(), SemanticError> and consume it everywhere where it is used
  • Made add_var first check get_variable_def(name) which also looks up in parent scopes
  • Added a bunch of tests also for existing but untested behavior

@cburgdorf cburgdorf force-pushed the christoph/fix/duplicate_definitions branch from 5f860b2 to e55cbbf Compare March 19, 2021 12:26
g-r-a-n-t
g-r-a-n-t approved these changes Mar 19, 2021
View reviewed changes
Copy link
Member

@g-r-a-n-t g-r-a-n-t left a comment

Choose a reason for hiding this comment

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

Great! looking forward to seeing more PRs like this.

@cburgdorf cburgdorf merged commit 7b9ef12 into ethereum:master Mar 19, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@g-r-a-n-t g-r-a-n-t g-r-a-n-t approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@cburgdorf @g-r-a-n-t