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.

Sign up for GitHub

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

Unused variable detection #429

Merged
merged 2 commits into from
Jul 9, 2021
Merged

Unused variable detection #429

merged 2 commits into from
Jul 9, 2021

Conversation

LucasSte
Copy link
Contributor

@LucasSte LucasSte commented Jul 2, 2021 •
edited
Loading

This pull request implements unused variable detection for the Solang compiler.
It generates warnings for unused state variables, global constants and contract storage.
As a bonus, I implemented the detection of events that have never been emitted.

This PR also implements tests for the new code.

This is the first milestone for the Linux Foundation mentorship on the Solang compiler.
For more information, please refer to the project plan.

seanyoung
seanyoung requested changes Jul 4, 2021
View reviewed changes
Copy link
Contributor

@seanyoung seanyoung left a comment

Choose a reason for hiding this comment

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

Looks great! Just some minor comments, I think other than some details it looks god.

I would say it is good practise to write tests as are you coding, so you have all the edge cases in mind and don't forget about them when writing tests.

src/lib.rs Outdated Show resolved Hide resolved
src/sema/unused_variable.rs Outdated Show resolved Hide resolved
src/sema/unused_variable.rs Outdated Show resolved Hide resolved
@seanyoung
Copy link
Contributor

Also please squash all your work into a single commit once you are done - or a few commits if each commit makes logical sense.

@LucasSte LucasSte force-pushed the unused-variable-detection branch 2 times, most recently from 571b295 to 5c2b19b Compare July 5, 2021 17:36
@LucasSte
Implement unused variable detection
033963d 
Signed-off-by: LucasSte <lucas.tnagel@gmail.com>
@LucasSte LucasSte force-pushed the unused-variable-detection branch 5 times, most recently from e3a2cee to 8edac3e Compare July 7, 2021 20:05
@LucasSte LucasSte marked this pull request as ready for review July 7, 2021 20:05
seanyoung
seanyoung requested changes Jul 8, 2021
View reviewed changes
Copy link
Contributor

@seanyoung seanyoung left a comment

Choose a reason for hiding this comment

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

Fantastic work! I have some very minor comments.

Please also note that the clippy warnings need to be fixed. You can probably enable clippy in your ide.

src/sema/statements.rs Outdated Show resolved Hide resolved
.gitignore Outdated Show resolved Hide resolved
src/sema/unused_variable.rs Show resolved Hide resolved
src/sema/unused_variable.rs Outdated Show resolved Hide resolved
src/sema/unused_variable.rs Outdated Show resolved Hide resolved
src/sema/mod.rs Outdated Show resolved Hide resolved
src/sema/statements.rs Outdated Show resolved Hide resolved
LucasSte
LucasSte commented Jul 8, 2021
View reviewed changes
src/sema/unused_variable.rs Outdated Show resolved Hide resolved
seanyoung
seanyoung requested changes Jul 8, 2021
View reviewed changes
Copy link
Contributor

@seanyoung seanyoung left a comment

Choose a reason for hiding this comment

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

Looks good, just a few very minor comments.

src/sema/mod.rs Outdated Show resolved Hide resolved
src/sema/mod.rs Outdated Show resolved Hide resolved
src/sema/statements.rs Outdated Show resolved Hide resolved
src/sema/variables.rs Outdated Show resolved Hide resolved
@LucasSte LucasSte requested a review from seanyoung July 9, 2021 15:36
@seanyoung
Copy link
Contributor

Would you mind rebasing against latest main, so it can be merged and also squashing the commits into one. Thanks.

@LucasSte
Implement tests for unused variable detection
76e520c 
Signed-off-by: LucasSte <lucas.tnagel@gmail.com>
@LucasSte
Copy link
Contributor Author

LucasSte commented Jul 9, 2021

Would you mind rebasing against latest main, so it can be merged and also squashing the commits into one. Thanks.

I left two commits: one for the code itself and another for the tests.

@seanyoung seanyoung merged commit be73b62 into hyperledger-solang:main Jul 9, 2021
@LucasSte LucasSte deleted the unused-variable-detection branch July 12, 2021 19:34
This was referenced Jul 12, 2021
Merged
Merged
seanyoung added a commit to seanyoung/solang that referenced this pull request Aug 27, 2021
@seanyoung
Uniswap works for Solana and Substrate
c8416eb 
Added
- Added a strength reduce pass to eliminate 256/128 bit multiply, division,
  and modulo where possible.
- Visual Studio Code extension can download the Solang binary from github
  releases, so the user is not required to download it themselves
- The Solana target now has support for arrays and mapping in contract
  storage
- The Solana target has support for the keccak256(), ripemd160(), and
  sha256() builtin hash functions.
- The Solana target has support for the builtins this and block.timestamp.
- Implement abi.encodePacked() for the ethereum abi encoder
- The Solana target now compiles all contracts to a single `bundle.so` BPF
  program.
- Any unused variables, events, or contract variables are now detected and
  warnings are given, thanks to [LucasSte](hyperledger-solang#429)
- The `immutable` attribute on contract storage variables is now supported.
- The `override` attribute on public contract storage variables is now supported.
- The `unchecked {}` code block is now parsed and supported. Math overflow still
  is unsupported for types larger than 64 bit.
- `assembly {}` blocks are now parsed and give a friendly error message.
- Any variable use before it is given a value is now detected and results in
  a undefined variable diagnostic, thanks to [LucasSte](hyperledger-solang#468)

Changed
- Solang now uses LLVM 12.0, based on the [Solana LLVM tree](https://github.com/solana-labs/llvm-project/)

Fixed
- Fix a number of issues with parsing the uniswap v2 contracts
- ewasm: staticcall() and delegatecall() cannot take value argument
- Fixed array support in the ethereum abi encoder and decoder
- Fixed issues in arithmetic on non-power-of-2 types (e.g. uint112)

Signed-off-by: Sean Young <sean@mess.org>
seanyoung added a commit that referenced this pull request Aug 27, 2021
@seanyoung
Uniswap works for Solana and Substrate
43237d2 
Added
- Added a strength reduce pass to eliminate 256/128 bit multiply, division,
  and modulo where possible.
- Visual Studio Code extension can download the Solang binary from github
  releases, so the user is not required to download it themselves
- The Solana target now has support for arrays and mapping in contract
  storage
- The Solana target has support for the keccak256(), ripemd160(), and
  sha256() builtin hash functions.
- The Solana target has support for the builtins this and block.timestamp.
- Implement abi.encodePacked() for the ethereum abi encoder
- The Solana target now compiles all contracts to a single `bundle.so` BPF
  program.
- Any unused variables, events, or contract variables are now detected and
  warnings are given, thanks to [LucasSte](#429)
- The `immutable` attribute on contract storage variables is now supported.
- The `override` attribute on public contract storage variables is now supported.
- The `unchecked {}` code block is now parsed and supported. Math overflow still
  is unsupported for types larger than 64 bit.
- `assembly {}` blocks are now parsed and give a friendly error message.
- Any variable use before it is given a value is now detected and results in
  a undefined variable diagnostic, thanks to [LucasSte](#468)

Changed
- Solang now uses LLVM 12.0, based on the [Solana LLVM tree](https://github.com/solana-labs/llvm-project/)

Fixed
- Fix a number of issues with parsing the uniswap v2 contracts
- ewasm: staticcall() and delegatecall() cannot take value argument
- Fixed array support in the ethereum abi encoder and decoder
- Fixed issues in arithmetic on non-power-of-2 types (e.g. uint112)

Signed-off-by: Sean Young <sean@mess.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@seanyoung seanyoung seanyoung 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.
2 participants
@LucasSte @seanyoung