add EVM targets to grammar#1823
Conversation
|
ggiraldez
left a comment
ggiraldez
left a comment
There was a problem hiding this comment.
Left a couple of questions (just a one in two places actually), but otherwise looks great!
| Self::From { from } => (*from as u32) <= (other as u32), | ||
| Self::Till { till } => (other as u32) < (*till as u32), | ||
| Self::Range { from, till } => { | ||
| (*from as u32) <= (other as u32) && (other as u32) < (*till as u32) | ||
| } |
There was a problem hiding this comment.
Q: why are you casting to u32 here? Since the enum derives Ord and PartialOrd the comparisons could be done without casting. Is there any advantage to it?
There was a problem hiding this comment.
This was because the built-in derive doesn't generate const implementations. This is probably an overkill, so I simplified/removed the const.
| Self::From { from } => (*from as u32) <= (other as u32), | ||
| Self::Till { till } => (other as u32) < (*till as u32), | ||
| Self::Range { from, till } => { | ||
| (*from as u32) <= (other as u32) && (other as u32) < (*till as u32) | ||
| } |
There was a problem hiding this comment.
Same question as before: why cast the values for comparison?
There was a problem hiding this comment.
This was because the built-in derive doesn't generate const implementations. This is probably an overkill, so I simplified/removed the const.
OmarTawfik
force-pushed
the
OmarTawfik/stacks/versions/3/fix-function-builtin
branch
from
808a8c3 to
425d32d
Compare
OmarTawfik
force-pushed
the
OmarTawfik/stacks/versions/4/add-evm-hardforks
branch
from
5600951 to
c692190
Compare
|
Bencher Report| Branch | OmarTawfik/stacks/versions/4/add-evm-hardforks |
| Testbed | ci |
⚠️ WARNING: Truncated view!The full continuous benchmarking report exceeds the maximum length allowed on this platform.
🐰 View full continuous benchmarking report in Bencher
⚠️ WARNING: No Threshold found!Without a Threshold, no Alerts will ever be generated.
OmarTawfik
force-pushed
the
OmarTawfik/stacks/versions/4/add-evm-hardforks
branch
from
c692190 to
19e2b35
Compare
OmarTawfik
force-pushed
the
OmarTawfik/stacks/versions/4/add-evm-hardforks
branch
from
19e2b35 to
53dea41
Compare
2 participants
This PR adds
EvmTargetas a data type to our grammar, and adds a newevm_enabledfield to all built-ins, along with a generated strongly-typed enum, and aEvmTargetSpecifierhelper, matching what we have forLanguageVersion.A built-in's availability is gated on two axes:
enabled = {version}) - the version at which the compiler started accepting the name. Example:abi.encodeCallis rejected bysolcbefore0.8.11regardless of the EVM target.evm_enabled = {target}) - the target at which the opcode/feature a built-in lowers to became available. Example:blockhash's siblingblock.blobbasefeelowers toBLOBBASEFEE, which only exists from Cancun, regardless of the frontend version.The two are independent: a built-in may carry one attribute, the other, or both, and when both are present both must hold. Empirically,
blobhashrequires solc>=0.8.24(frontend) and Cancun (opcode): solc-0.8.23 @ cancun and solc-0.8.24 @ shanghai both reject it.Note:
evm_enabledis metadata-only for now. A later PR will add the relevant diagnostics, and update the binder to use the new metadata.