Skip to content

[v2] feat(diagnostics): Check double override across definitions#1843

Merged
teofr merged 1 commit into
mainfrom
teofr/validate-multiple-override-specifier
Jun 9, 2026
Merged

[v2] feat(diagnostics): Check double override across definitions#1843
teofr merged 1 commit into
mainfrom
teofr/validate-multiple-override-specifier

Conversation

@teofr

@teofr teofr commented Jun 8, 2026

Copy link
Copy Markdown
Contributor

@teofr teofr requested a review from Copilot June 8, 2026 15:57
@changeset-bot

changeset-bot Bot commented Jun 8, 2026

Copy link
Copy Markdown

⚠️ No Changeset found

Latest commit: 5614c7e

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds a new v2 syntax diagnostic that rejects repeated override specifiers on a single definition, aligning Slang v2 diagnostics with solc behavior for this parse-time error class.

Changes:

  • Introduces MultipleOverrideSpecifiers diagnostic kind under syntax/*.
  • Updates the v2 CST→IR builder to extract the first override specifier and emit diagnostics for any subsequent ones (functions, fallback/receive functions, modifiers, state variables).
  • Adds diagnostics snapshot fixtures and generated test wiring to cover the new cases.

Reviewed changes

Copilot reviewed 9 out of 20 changed files in this pull request and generated no comments.

Show a summary per file
File Description
crates/solidity-v2/outputs/cargo/ir/src/ir/builder/mod.rs Emits MultipleOverrideSpecifiers when more than one override specifier appears in attributes.
crates/solidity-v2/outputs/cargo/common/src/diagnostics/kinds/syntax/multiple_override_specifiers.rs Defines the new syntax diagnostic (code, severity, message).
crates/solidity-v2/outputs/cargo/common/src/diagnostics/kinds/syntax/mod.rs Registers and re-exports the new diagnostic kind in the syntax diagnostics enum.
crates/solidity-v2/outputs/cargo/tests/src/diagnostics_output/snapshots.generated.rs Adds snapshot test entries for the new multiple_override_specifiers fixtures.
crates/solidity-v2/testing/snapshots/diagnostics_output/syntax/multiple_override_specifiers/** Adds new snapshot inputs and expected solc/slang outputs for functions, fallback/receive, modifiers, and state variables.
crates/solidity-v2/outputs/cargo/common/generated/public_api.txt Updates public API surface to include the new diagnostic kind.

@github-actions

github-actions Bot commented Jun 8, 2026

Copy link
Copy Markdown
Contributor

🐰 Bencher Report

Branchteofr/validate-multiple-override-specifier
Testbedci

⚠️ WARNING: Truncated view!

The full continuous benchmarking report exceeds the maximum length allowed on this platform.

⚠️ WARNING: No Threshold found!

Without a Threshold, no Alerts will ever be generated.

🚨 3 Alerts

🐰 View full continuous benchmarking report in Bencher

@teofr teofr marked this pull request as ready for review June 8, 2026 16:11
@teofr teofr requested review from a team as code owners June 8, 2026 16:11

@ggiraldez ggiraldez left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! 🚀

Reject functions, fallback/receive functions, modifiers, and state
variables that carry more than one `override` specifier, matching solc's
ParserError 1827/9125/9102. Closes the SDR[9] and SDR[12] validation
TODOs and the `override` part of SDR[1730] in the IR builder.
@teofr teofr force-pushed the teofr/validate-multiple-override-specifier branch from c99bda4 to 5614c7e Compare June 9, 2026 08:33
@teofr teofr enabled auto-merge June 9, 2026 08:33
@teofr teofr added this pull request to the merge queue Jun 9, 2026
Merged via the queue into main with commit 84c5a02 Jun 9, 2026
17 checks passed
@teofr teofr deleted the teofr/validate-multiple-override-specifier branch June 9, 2026 09:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants