Skip to content

Fixed inline assembly external identifier access. #11741

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
Merged
merged 1 commit into from
Aug 12, 2021
Merged

Fixed inline assembly external identifier access. #11741

merged 1 commit into from
Aug 12, 2021

Conversation

@chriseth
Copy link
Contributor

@chriseth chriseth commented Aug 4, 2021 •
edited
Loading

Fixes #11734

@chriseth chriseth requested a review from hrkrshnn August 4, 2021 16:11
@chriseth chriseth mentioned this pull request Aug 4, 2021
Merged
1 task
@chriseth chriseth requested a review from ekpyron August 10, 2021 09:05
@ekpyron
Copy link
Collaborator

ekpyron commented Aug 10, 2021

I haven't thought the change through entirely, but what happens for

contract C {
function f() public {
    uint x; 
    assembly {
        function g() -> x {
            x := 42
        }
        x := g()
    }
}
}

?
Generally, a few test cases would be nice.

@chriseth
Copy link
Contributor Author

chriseth commented Aug 11, 2021

This is weird, I thought I had tests in this PR...

@chriseth
Copy link
Contributor Author

chriseth commented Aug 11, 2021

Ah, the tests are here: https://github.com/ethereum/solidity/pull/11735/files

@chriseth chriseth force-pushed the shadowing_inlineasm_fix_yul_codegen branch from bc06e1a to 67a5cd8 Compare August 11, 2021 12:51
@chriseth
Copy link
Contributor Author

chriseth commented Aug 11, 2021

Added two tests. Changelog not necessary since it only fixes an issue with the sol yul code gen.

@chriseth chriseth force-pushed the shadowing_inlineasm_fix_yul_codegen branch from 67a5cd8 to a4c94a1 Compare August 11, 2021 16:18
ekpyron
ekpyron approved these changes Aug 12, 2021
View reviewed changes
test/libsolidity/syntaxTests/inlineAssembly/external_identifier_access_shadowing.sol
}
}
// ----
// DeclarationError 6578: (123-124): Cannot access local Solidity variables from inside an inline assembly function.
Copy link
Collaborator

@ekpyron ekpyron Aug 12, 2021

Choose a reason for hiding this comment

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

Yeah, that's of course a bit weird... but if we make the -> x a proper shadowing error later on, I guess it's fine for now.

Copy link
Contributor Author

@chriseth chriseth Aug 12, 2021

Choose a reason for hiding this comment

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

By the way, this is not a new error.

@chriseth
Copy link
Contributor Author

chriseth commented Aug 12, 2021

Removed "fixes #11380 (maybe)" because I think it does not.

@chriseth chriseth merged commit ccdc11e into develop Aug 12, 2021
@chriseth chriseth deleted the shadowing_inlineasm_fix_yul_codegen branch August 12, 2021 12:38
@leonardoalt leonardoalt mentioned this pull request Aug 27, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ekpyron ekpyron ekpyron approved these changes

@hrkrshnn hrkrshnn Awaiting requested review from hrkrshnn

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[Sol->Yul] Unimplemented feature error

3 participants

@chriseth @ekpyron