-
Notifications
You must be signed in to change notification settings - Fork 5.9k
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
Make name clash with a builtin non-fatal in Yul parser #15712
Conversation
test/libsolidity/syntaxTests/inlineAssembly/clash_with_builtin_name.sol
Outdated
Show resolved
Hide resolved
let x: jump | ||
x := true: dup12 | ||
|
||
function f(y: jumpi) {} |
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.
Note that, in case of type names, the parser does not complain about the name clash with reserved identifiers even though it does for builtins. Does this mean that we were not checking this at all back when types were allowed?
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 just checked with this example by replacing x
by bool
, f
by bool
, and y
by bool
(and setting all the types to bool
as well) on an older compiler version. There is no warning and/or error being raised. At least on 0.8.20
, which is the version I randomly picked.
a3cbe60
to
1a2b1a4
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.
PR looks good to me, we could potentially look into the reserved identifier topic in a follow-up
1a2b1a4
to
4e63714
Compare
4e63714
to
edb6783
Compare
Currently Yul parser stops immediately when it notices that a function or variable being declared has a name matching a Yul builtin. I don't see any good reason for this to be a fatal error. In fact the same exact error is already non-fatal in inline assembly. Clashes with reserved identifiers are also non-fatal.
This is a minor thing but, aside from unnecessarily giving users less information than we can, it also makes writing some kinds of tests a bit more annoying (e.g. in #15700 it makes it impossible to have one test covering all Yul builtins).