Skip to content

Conversation

jjcnn
Copy link
Contributor

@jjcnn jjcnn commented Jan 31, 2025

Description

Fixes #6329 .

During insertion of impls into the trait map we used to forget to check for alias types, which meant that it was possible to have multiple implementations of a trait for the same type, one for the original type and one for the alias.

#6626 fixed this bug, but the error message was unhelpful in that it reported the error on the original type without mentioning the alias.

This PR improves the error message, and also improves the error message for aliased types when they contain multiple declarations of the same member.

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.

@jjcnn jjcnn self-assigned this Jan 31, 2025
@jjcnn jjcnn requested a review from a team as a code owner January 31, 2025 21:45
@jjcnn jjcnn added the blocked label Jan 31, 2025
@jjcnn jjcnn marked this pull request as draft January 31, 2025 22:15
@jjcnn jjcnn force-pushed the jjcnn/unalias_type_id_in_trait_map_unification branch from 610d237 to 7e268f7 Compare February 24, 2025 21:10
@jjcnn jjcnn added team:compiler Compiler Team and removed blocked labels Feb 24, 2025
Copy link

codspeed-hq bot commented Feb 24, 2025

CodSpeed Performance Report

Merging #6875 will not alter performance

Comparing jjcnn/unalias_type_id_in_trait_map_unification (05322d7) with master (8c0cc20)

Summary

✅ 22 untouched benchmarks

@jjcnn jjcnn temporarily deployed to fuel-sway-bot March 2, 2025 19:20 — with GitHub Actions Inactive
@jjcnn jjcnn marked this pull request as ready for review March 2, 2025 19:26
IGI-111
IGI-111 previously approved these changes Mar 3, 2025
@IGI-111 IGI-111 temporarily deployed to fuel-sway-bot March 3, 2025 12:47 — with GitHub Actions Inactive
Copy link
Contributor

@tritao tritao left a comment

Choose a reason for hiding this comment

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

Sorry to be the bad cop, just a few minor whitespace issues

@jjcnn jjcnn temporarily deployed to fuel-sway-bot March 3, 2025 22:19 — with GitHub Actions Inactive
@jjcnn jjcnn temporarily deployed to fuel-sway-bot March 5, 2025 15:19 — with GitHub Actions Inactive
@tritao tritao merged commit 8a3d8df into master Mar 6, 2025
42 checks passed
@tritao tritao deleted the jjcnn/unalias_type_id_in_trait_map_unification branch March 6, 2025 08:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

team:compiler Compiler Team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Insufficient trait duplication check

4 participants