Enforce that gas expectations match enabled pipelines#16572
Open
tavian-dev wants to merge 1 commit intoargotorg:developfrom
Open
Enforce that gas expectations match enabled pipelines#16572tavian-dev wants to merge 1 commit intoargotorg:developfrom
tavian-dev wants to merge 1 commit intoargotorg:developfrom
Conversation
Validates in the SemanticTest constructor that test files do not carry gas expectations for pipelines that are not enabled by their settings. For example, a test with `compileViaYul: true` should not have `gas legacy:` or `gas legacyOptimized:` expectations, as these would never be validated against actual compilation output. This addresses the issue noted in PR argotorg#16369 where some tests retained stale legacy gas expectations after being switched to Yul-only. Changes: - Add validation loop after parseExpectations() that checks all gas expectation keys against the test's pipeline configuration - Remove stale gas expectations from 5 test files: - 3 functionCall tests with unused `gas legacy:` lines - 1 immutable test with unused `gas legacy:` and `gas legacy code:` - 1 array copying test with unused `gas legacyOptimized:` line Fixes argotorg#16461
|
Thank you for your contribution to the Solidity compiler! A team member will follow up shortly. If you haven't read our contributing guidelines and our review checklist before, please do it now, this makes the reviewing process and accepting your contribution smoother. If you have any questions or need our help, feel free to post them in the PR or talk to us directly on the #solidity-dev channel on Matrix. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Addresses #16461.
Adds validation in the
SemanticTestconstructor to ensure that test files do not carry gas expectations for pipelines that are not enabled by their settings.The validation checks:
compileViaYul: truemust not havegas legacy:orgas legacyOptimized:expectationscompileViaYul: falsemust not havegas ir:orgas irOptimized:expectationscompileViaSSACFG: truemust not havegas ssaCFG:orgas ssaCFGOptimized:expectationsIf a stale gas expectation is found, a clear
runtime_erroris thrown identifying the offending key and the current pipeline settings.Also removes stale gas expectations from 5 test files:
semanticTests/functionCall/return_size_shorter_than_expected.sol— removedgas legacy: 131966semanticTests/functionCall/return_size_shorter_than_expected_evm_version_after_homestead.sol— removedgas legacy: 131966semanticTests/functionCall/return_size_bigger_than_expected.sol— removedgas legacy: 131966semanticTests/immutable/immutable_tag_too_large_bug.sol— removedgas legacy: 83499andgas legacy code: 408800semanticTests/array/copying/nested_dynamic_array_element_calldata_to_storage.sol— removedgas legacyOptimized: 120228Testing
soltestand ran the full semantic test suite (1636 tests) — all passgas legacy:line on acompileViaYul: truetest — validation correctly rejects it with a clear error messagescripts/check_style.sh) passes