Skip to content

Conversation

@tritao
Copy link
Contributor

@tritao tritao commented Sep 13, 2025

This improves the trait coherence checks for impl self case, and polishes a bit of the wording on the existing diagnostics.

We now enforce package-level check in impl-self type checking, which means we now reject inherent impls for external nominal types (struct/enum).

There is still is a specialized whitelist for StorageKey so current code that uses this pattern keeps working.

Checklist

  • I have linked to any relevant issues.
  • I have commented my code, particularly in hard-to-understand areas.
  • I have updated the documentation where relevant (API docs, the reference, and the Sway book).
  • I have added tests that prove my fix is effective or that my feature works.
  • I have added (or requested a maintainer to add) the necessary Breaking* or New Feature labels where relevant.
  • I have done my best to ensure that my PR adheres to the Fuel Labs Code Review Standards.
  • I have requested a review from the relevant team or maintainers.

Note

Enforces package-level coherence by rejecting inherent impl blocks for external nominal types (with a whitelist for std::storage::StorageKey<_>), adds a dedicated diagnostic, and updates tests and orphan rule wording from module to package.

  • Semantic analysis:
    • Disallow inherent impl for external nominal types in type_check_impl_self (sway-core/src/semantic_analysis/ast_node/declaration/impl_trait.rs).
    • Temporary whitelist for std::storage::StorageKey<_>.
    • Emit new error CompileError::InherentImplForExternalType with spans.
  • Diagnostics (sway-error/src/error.rs):
    • Add InherentImplForExternalType variant and full diagnostic with hints/help.
    • Change orphan rule wording from "module" to "package" in messages and help.
  • Tests:
    • Update failing snapshots and checks to expect package-level wording and the new inherent-impl error.
    • Refactor should_pass/language/generic_transpose to use a new trait OptionTranspose instead of an inherent impl on Option<_>.
    • Adjust impl_self_overlap and orphan rule snapshots accordingly.

Written by Cursor Bugbot for commit 308d097. This will update automatically on new commits. Configure here.

@tritao tritao self-assigned this Sep 13, 2025
@tritao tritao added compiler General compiler. Should eventually become more specific as the issue is triaged compiler: frontend Everything to do with type checking, control flow analysis, and everything between parsing and IRgen labels Sep 13, 2025
@tritao tritao force-pushed the improve-trait-coherence-checks branch from 0d7b96a to 4c72c34 Compare September 13, 2025 14:58
@codspeed-hq
Copy link

codspeed-hq bot commented Sep 13, 2025

CodSpeed Performance Report

Merging #7385 will not alter performance

Comparing tritao:improve-trait-coherence-checks (770e69e) with master (4a44a36)

Summary

✅ 25 untouched

@tritao tritao force-pushed the improve-trait-coherence-checks branch from 4c72c34 to 70828a8 Compare September 16, 2025 09:18
@tritao tritao marked this pull request as ready for review September 16, 2025 10:01
@tritao tritao requested a review from a team as a code owner September 16, 2025 10:01
@tritao tritao marked this pull request as draft September 22, 2025 12:38
@tritao tritao force-pushed the improve-trait-coherence-checks branch from e4a6040 to e1285ed Compare October 12, 2025 18:56
@tritao tritao marked this pull request as ready for review October 12, 2025 19:19
IGI-111
IGI-111 previously approved these changes Oct 20, 2025
@tritao tritao force-pushed the improve-trait-coherence-checks branch from e1285ed to bc27b99 Compare October 20, 2025 15:18
@tritao tritao force-pushed the improve-trait-coherence-checks branch from bc27b99 to e22e7ec Compare October 20, 2025 15:22
@tritao tritao requested a review from IGI-111 October 20, 2025 15:51
@tritao
Copy link
Contributor Author

tritao commented Oct 20, 2025

Rebased due to conflicts.

@tritao tritao enabled auto-merge (squash) October 20, 2025 15:52
@tritao tritao requested a review from a team October 23, 2025 08:45
@tritao tritao removed the request for review from IGI-111 October 24, 2025 07:46
@tritao tritao requested review from a team and ironcev October 24, 2025 07:46
xunilrj
xunilrj previously approved these changes Nov 1, 2025
@cursor
Copy link

cursor bot commented Dec 15, 2025

PR Summary

Enforces package-level orphan rule for inherent impls, adds a new warning/diagnostics (with a std::StorageKey exception), and updates/extends tests accordingly.

  • Semantic analysis (impl self):
    • Disallow inherent impls for external struct/enum types; emit Warning::InherentImplForExternalType and abort type checking; whitelist std::storage::storage_key::StorageKey<_>.
  • Diagnostics:
    • Orphan-rule wording updated from "module" to "package" (IncoherentImplDueToOrphanRule).
    • Add Warning::InherentImplForExternalType with formatted diagnostic, hints, and future-hard-error help.
  • Tests:
    • Add failing case trait_coherence/impl_alias_external and adjust impl_self_overlap to compile with expected warnings.
    • Update orphan-rule snapshot messages to "package".
    • Add passing case trait_coherence/impl_alias_same_package.
    • Refactor language/generic_transpose to use a trait (OptionTranspose) instead of an inherent impl on Option<MyResult<..>>.

Written by Cursor Bugbot for commit f85841d. This will update automatically on new commits. Configure here.

@ironcev ironcev requested a review from xunilrj December 15, 2025 13:56
xunilrj
xunilrj previously approved these changes Dec 15, 2025
@tritao tritao force-pushed the improve-trait-coherence-checks branch from 07dd4f5 to 7b3d7ce Compare December 16, 2025 15:27
@tritao tritao force-pushed the improve-trait-coherence-checks branch from 7b3d7ce to be65496 Compare December 16, 2025 15:29
This improves the trait coherence checks for impl self case,
and polishes a bit of the wording on the existing diagnostics.

* Add CompileError::InherentImplForExternalType and diagnostics.
  Reason/issue/help now refer to "package" to match coherence scope

* Enforce package-level check in impl-self type checking
  Reject inherent impls for external nominal types (struct/enum)

Temporary whitelist: allow inherent impls on std::storage::StorageKey<_>

This is a workaround so current code that uses this pattern keeps
working.
@tritao tritao force-pushed the improve-trait-coherence-checks branch from be65496 to f85841d Compare December 16, 2025 15:45
@ironcev ironcev requested a review from xunilrj December 16, 2025 16:19
@tritao tritao merged commit b4fa78d into FuelLabs:master Dec 16, 2025
43 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

compiler: frontend Everything to do with type checking, control flow analysis, and everything between parsing and IRgen compiler General compiler. Should eventually become more specific as the issue is triaged

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants