Allow builtins in yul identifier paths in antlr grammar. #12545
Conversation
ekpyron
force-pushed
the
yulGrammarFluke
branch
from
1c81eb7
to
8a6a3d6
Compare
ekpyron
force-pushed
the
yulGrammarFluke
branch
from
8a6a3d6
to
2d0f627
Compare
| @@ -564,7 +564,7 @@ yulFunctionDefinition: | |||
| * While only identifiers without dots can be declared within inline assembly, | |||
| * paths containing dots can refer to declarations outside the inline assembly block. | |||
| */ | |||
| yulPath: YulIdentifier (YulPeriod YulIdentifier)*; | |||
| yulPath: YulIdentifier (YulPeriod (YulIdentifier | YulEVMBuiltin))*; | |||
There was a problem hiding this comment.
Strictly speaking probably address.test would in theory be a valid yulPath as well, but I'd say this hack is good enough for all intents and purposes.
ekpyron
changed the title
|
If we want, we can think about whether we can do this "properly", i.e. have the solidity yul version just parse identifiers containing dots - but since this won't really happen too much in practice, since we reserved identifiers with dots for high-level-language specific constructions and we're unlikely to do more weird stuff like with |
test/libsolidity/syntaxTests/inlineAssembly/assignment_to_function_pointer.sol
Outdated
Show resolved
Hide resolved
…tion_pointer.sol Co-authored-by: Kamil Śliwak <kamil.sliwak@codepoets.it>
I wouldn't be aware of the grammar being used for that (or actually for anything apart from our docs and tests for that matter). |
|
Ok, good. Looking at the article that was posted last week (Securing Warp: A formal specification of the Yul IR), they just treat Yul identifiers as strings and don't have a "yul path" like we do in our grammar. |
|
Ah, that one you meant - that's a rather bold simplification of Yul anyways :-). |
Found with the Sanctuary run on Arbitrum: https://github.com/NomicFoundation/slang/actions/runs/8803528884 This fixes the last bug in Arbitrum data set: https://github.com/Xanewok/slang/actions/runs/8813993801 Implemented in ethereum/solidity#12016 (0.8.10), grammar updated in ethereum/solidity#12545. This matches the upstream grammar, however: - this is more relaxed and parses code that would be rejected by validation, i.e. `<built-in> := ...` - we do get a bit of noise, since most of the paths will never contain the built-in. I've also changed `YulidentifierPath` to `YulPath` to match upstream and since it's not only an identifier anymore. This also includes a patch level bump - while technically it's breaking the CST shape, I'd consider this a (hopefully last?) grammar bug fix that might justify the patch level.
Came up in #12470