-
Notifications
You must be signed in to change notification settings - Fork 6.1k
Do not use named function labels if function names are not unique. #12139
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.
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
Conversation
8ae7765
to
f0c9a0d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, just a stray space
@@ -0,0 +1 @@ | |||
--strict-assembly |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think the annotations add much in this particular test. Might actually be more readable without them. Maybe it would be better to disable them?
--strict-assembly | |
--strict-assembly --debug-info none |
Does this PR fix also a similar case I found in inline assembly #12090 (comment)? EDIT: Reported as a separate issue (#12158). |
@@ -0,0 +1 @@ | |||
--experimental-via-ir --combined-json function-debug-runtime |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Output would be more readable if it was not all on a single line:
--experimental-via-ir --combined-json function-debug-runtime | |
--experimental-via-ir --combined-json function-debug-runtime --pretty-json --json-indent 4 |
contract C { | ||
function f() public pure returns (uint r) { | ||
assembly { function f() -> x { x := 1 } r := f() } | ||
} | ||
function g() public pure returns (uint r) { | ||
assembly { function f() -> x { x := 2 } r := f() } | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
contract C { | |
function f() public pure returns (uint r) { | |
assembly { function f() -> x { x := 1 } r := f() } | |
} | |
function g() public pure returns (uint r) { | |
assembly { function f() -> x { x := 2 } r := f() } | |
} | |
} | |
contract C { | |
function f() public pure returns (uint r) { | |
assembly { function f() -> x { x := 1 } r := f() } | |
} | |
function g() public pure returns (uint r) { | |
assembly { function f() -> x { x := 2 } r := f() } | |
} | |
} |
Also in function_name_clash.sol
below.
contract C { | ||
function f() public pure returns (uint r) { | ||
assembly { function f() -> x { x := 1 } r := f() } | ||
} | ||
function g() public pure returns (uint r) { | ||
assembly { function f() -> x { x := 2 } r := f() } | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Incorporated #12149 into the test. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are still some open comments that could be applied to improve the command-line test output but overall the fix seems correct and does the job even in the current form so I'm approving.
346a850
to
9f48b74
Compare
Rebased, simplified commandline tests and updated the "fixes". |
Fixes #12090
Fixes #12149
Fixes #12158