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

FeSized trait and data opertions. #93

Merged
merged 1 commit into from
Oct 15, 2020

Conversation

g-r-a-n-t
Copy link
Member

@g-r-a-n-t g-r-a-n-t commented Oct 14, 2020

What was wrong?

Partially addresses #19 and something that was mentioned in #39.

There are still some things that need to be refactored, but for the sake of keeping this PR at a reasonable size, they were ignored. The best example of this is that we still use methods defined within our types module to form Yul expressions and statements. These Yul elements should instead be formed exclusively in the mapper pass.

How was it fixed?

Created a new FeSized trait that is analogous to Rust's very own Sized trait, then used this trait to implement a set of data operations and refactored the compiler to use these operations.

To-Do

  • OPTIONAL: Update Spec if applicable
  • Clean up commit history

@codecov-io
Copy link

codecov-io commented Oct 14, 2020

Codecov Report

Merging #93 into master will increase coverage by 0.03%.
The diff coverage is 62.85%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #93      +/-   ##
==========================================
+ Coverage   87.67%   87.71%   +0.03%     
==========================================
  Files          37       37              
  Lines        2296     2287       -9     
==========================================
- Hits         2013     2006       -7     
+ Misses        283      281       -2     
Impacted Files Coverage Δ
compiler/src/yul/mappers/declarations.rs 100.00% <ø> (ø)
compiler/src/yul/mappers/functions.rs 0.00% <ø> (ø)
compiler/src/yul/mappers/assignments.rs 88.37% <44.44%> (-11.63%) ⬇️
compiler/src/yul/mappers/operations.rs 66.66% <62.50%> (-33.34%) ⬇️
semantics/src/namespace/types.rs 46.23% <66.66%> (-3.37%) ⬇️
compiler/src/yul/mappers/expressions.rs 96.59% <85.71%> (+0.07%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 703e31b...8048699. Read the comment docs.

@g-r-a-n-t g-r-a-n-t marked this pull request as draft October 14, 2020 03:32
@g-r-a-n-t g-r-a-n-t force-pushed the data-operations branch 3 times, most recently from 10cadf2 to f67359d Compare October 14, 2020 21:45
@g-r-a-n-t g-r-a-n-t marked this pull request as ready for review October 14, 2020 22:03
@g-r-a-n-t g-r-a-n-t changed the title data operations FeSized trait and data opertions. Oct 14, 2020
Copy link
Collaborator

@cburgdorf cburgdorf left a comment

Choose a reason for hiding this comment

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

LGTM

@g-r-a-n-t g-r-a-n-t merged commit de379dc into ethereum:master Oct 15, 2020
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.

3 participants