Skip to content

Report error on shadowing state variables#7817

Merged
chriseth merged 1 commit intodevelop_060from
bail-on-shadowing-state-vars
Dec 3, 2019
Merged

Report error on shadowing state variables#7817
chriseth merged 1 commit intodevelop_060from
bail-on-shadowing-state-vars

Conversation

@christianparpart
Copy link
Copy Markdown
Contributor

@christianparpart christianparpart commented Nov 27, 2019

Goal:

Notes

I still feel it kind of conflicts with the work with @Marenz. So maybe you can have a look too (that's why some tests (of yours) are failing).

Maybe we should clarify here a bit. I see perfect reasoning in the way shadowed variables are reported (in this PR), and I thought that the variable override logic were about to override a base function with a state var in the child contract, but apparently some tests fail with some interesting new syntax that I don't understand yet.

@christianparpart christianparpart changed the base branch from develop to develop_060 November 27, 2019 16:32
@christianparpart christianparpart force-pushed the bail-on-shadowing-state-vars branch from a8b780c to afec573 Compare November 27, 2019 16:33
Comment thread libsolidity/analysis/TypeChecker.cpp Outdated
@chriseth
Copy link
Copy Markdown
Contributor

Please add changelog entry, documentation and entry to "0.6.0" change list"

@christianparpart christianparpart force-pushed the bail-on-shadowing-state-vars branch 3 times, most recently from b34c7d3 to 08c9912 Compare November 27, 2019 17:07
Comment thread libsolidity/analysis/TypeChecker.cpp Outdated
@Marenz
Copy link
Copy Markdown
Contributor

Marenz commented Nov 28, 2019

I just found this https://github.com/ethereum/solidity/blob/develop/libsolidity/analysis/NameAndTypeResolver.cpp#L349
this would probably then be a better place to do this no?

@christianparpart christianparpart force-pushed the bail-on-shadowing-state-vars branch from 08c9912 to fc1272a Compare November 30, 2019 00:29
@leonardoalt leonardoalt force-pushed the bail-on-shadowing-state-vars branch from fc1272a to 6cf4805 Compare December 3, 2019 14:15
@leonardoalt
Copy link
Copy Markdown

Rebased, updated docs, tests and comments

@leonardoalt leonardoalt marked this pull request as ready for review December 3, 2019 14:16
@leonardoalt leonardoalt force-pushed the bail-on-shadowing-state-vars branch from 6cf4805 to 9403839 Compare December 3, 2019 14:21
Comment thread Changelog.md Outdated
Comment thread docs/060-breaking-changes.rst Outdated
Comment thread docs/contracts/inheritance.rst Outdated
Comment thread test/libsolidity/syntaxTests/inheritance/override/override.sol Outdated
Comment thread test/libsolidity/syntaxTests/inheritance/override/override_multiple.sol Outdated
Comment thread test/libsolidity/smtCheckerTests/inheritance/state_variables_2.sol
@chriseth
Copy link
Copy Markdown
Contributor

chriseth commented Dec 3, 2019

Do we have a test that shows a clash between a variable and a function?

@leonardoalt leonardoalt force-pushed the bail-on-shadowing-state-vars branch from 9403839 to 9c5e6a1 Compare December 3, 2019 19:58
@leonardoalt leonardoalt force-pushed the bail-on-shadowing-state-vars branch from 9c5e6a1 to 7bbdfe0 Compare December 3, 2019 20:20
@chriseth chriseth merged commit 2d42da3 into develop_060 Dec 3, 2019
@chriseth chriseth deleted the bail-on-shadowing-state-vars branch December 3, 2019 20:22
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.

5 participants