Disallow shadowing for all identifiers in inline assembly #11735
Conversation
chriseth
force-pushed
the
shadowing_inlineasm
branch
2 times, most recently
from
85448c8 to
a88a11f
Compare
.../libsolidity/semanticTests/inlineAssembly/inline_assembly_storage_access_inside_function.sol
Show resolved
Hide resolved
|
Can you add a test with variable in inline assembly that shadows local variable? |
that already exists: test/libsolidity/syntaxTests/inlineAssembly/shadowing/local_variable.sol |
|
Would like to get some more thumbsup since this can break existing contracts (see the tests). |
| uint x; | ||
| function f() public pure { | ||
| assembly { | ||
| function g(f) -> x {} |
There was a problem hiding this comment.
Technically, this wouldn't trigger the bug from the issue. But it's fine as long as such examples don't reach codegen.
| { | ||
| SecondarySourceLocation ssl; | ||
| for (auto const* decl: declarations) | ||
| ssl.append("The shadowed declaration is here:", decl->location()); |
There was a problem hiding this comment.
For
contract C {
function f() external {
assembly {
function g(gasleft) { gasleft := 2 }
}
}
}The decl->location would be empty.
Should we do something about global functions?
|
Ok, true, let me try to fix it in yul codegen in a different way. |
chriseth
force-pushed
the
shadowing_inlineasm
branch
from
a88a11f to
5948fed
Compare
chriseth
changed the title
chriseth
force-pushed
the
shadowing_inlineasm
branch
3 times, most recently
from
8a2b7bf to
5072e6c
Compare
ekpyron
left a comment
ekpyron
left a comment
There was a problem hiding this comment.
Missing a test case for function arguments colliding with local variables (it's implicitly covered, but might be nice to have explicitly), maybe some other cases, too.
But generally, looks good!
chriseth
force-pushed
the
shadowing_inlineasm
branch
from
5072e6c to
aeb2438
Compare
Manual Resolved Conflicts: Changelog.md * Updated changelog test/externalTests/ens.sh * Merged fixes for upstream from both develop and breaking test/libsolidity/semanticTests/inlineAssembly/external_identifier_access_shadowing.sol * Removed in #11735 (breaking) test/libsolidity/semanticTests/inlineAssembly/function_name_clash.sol * Removed in #12209 (breaking) test/libsolidity/semanticTests/storage/mappings_array2d_pop_delete.sol * Removed in #11843 (breaking) test/libsolidity/semanticTests/storage/mappings_array_pop_delete.sol * Removed in #11843 (breaking) test/libsolidity/syntaxTests/inlineAssembly/basefee_berlin_function.sol * Used version of file from #11842 (breaking)
Uh oh!
There was an error while loading. Please reload this page.