Skip to content

Add analyzer checks for shadowing of built-ins and generic arg lists where they don't belong - #539

Merged
g-r-a-n-t merged 3 commits into
argotorg:masterfrom
sbillig:analyzer-error-stuff
Sep 24, 2021
Merged

g-r-a-n-t merged 3 commits into
argotorg:masterfrom
sbillig:analyzer-error-stuff

Conversation

@sbillig

@sbillig sbillig commented Sep 6, 2021 •

Copy link
Copy Markdown
Collaborator

What was wrong?

The analyzer allowed things like:

type u8 = u256
let u256: u8 = 10
fn u8(u8: u256) -> u8:
let keccak256: u256 = keccak256(x)
let x: u256 = u256<1, 2, 3>(10)

How was it fixed?

Added more checks to type, function, and var defs. This implementation isn't super satisfying; I was able to consolidate some duplicate logic, but we still have to perform this kind of check in 3 places, which doesn't seem ideal.

To-Do

  • Add entry to the release notes (may forgo for trivial changes)

  • Clean up commit history

@codecov-commenter

codecov-commenter commented Sep 6, 2021 •

Copy link
Copy Markdown

Codecov Report

Merging #539 (2d8cd99) into master (8ee4ed7) will decrease coverage by 0.68%.
The diff coverage is 77.91%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #539      +/-   ##
==========================================
- Coverage   87.91%   87.23%   -0.69%     
==========================================
  Files          87       88       +1     
  Lines        5653     5931     +278     
==========================================
+ Hits         4970     5174     +204     
- Misses        683      757      +74     
Impacted Files Coverage Δ
crates/analyzer/src/errors.rs 80.95% <ø> (ø)
crates/analyzer/src/traversal/assignments.rs 66.66% <ø> (ø)
crates/analyzer/src/namespace/items.rs 89.78% <40.00%> (-1.38%) ⬇️
crates/parser/src/ast.rs 71.42% <69.68%> (+7.49%) ⬆️
crates/analyzer/src/namespace/scopes.rs 81.51% <73.33%> (-1.35%) ⬇️
crates/analyzer/src/traversal/utils.rs 81.81% <82.35%> (-2.19%) ⬇️
crates/analyzer/src/traversal/types.rs 94.36% <96.61%> (-1.09%) ⬇️
crates/analyzer/src/builtins.rs 100.00% <100.00%> (ø)
crates/analyzer/src/db/queries/contracts.rs 97.76% <100.00%> (+0.12%) ⬆️
crates/analyzer/src/db/queries/module.rs 100.00% <100.00%> (ø)
... and 7 more

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 0eb0898...2d8cd99. Read the comment docs.

@sbillig
sbillig force-pushed the analyzer-error-stuff branch from 9dfcb34 to 2d8cd99 Compare September 23, 2021 02:52
@sbillig sbillig changed the title Analyzer error stuff Add analyzer checks for shadowing of built-ins and generic arg lists where they don't belong Sep 24, 2021
@sbillig
sbillig requested a review from g-r-a-n-t September 24, 2021 01:00

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

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

🔥 💯

@g-r-a-n-t
g-r-a-n-t merged commit df9520a into argotorg:master Sep 24, 2021
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