-
Notifications
You must be signed in to change notification settings - Fork 13.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Tracking issue for overlapping_inherent_impls
compatibility lint
#36889
Comments
@aturon , could you write a description for this? |
#38967 could be related to this. |
@rfcbot fcp merge This warning has been out for a while. I propose we move it to deny-by-default. |
Team member @nikomatsakis has proposed to merge this. The next step is review by the rest of the tagged teams: No concerns currently listed. Once these reviewers reach consensus, this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up! See this document for info about what commands tagged team members can give me. |
There's a catch. |
All reactions
Sorry, something went wrong.
@petrochenkov heh =) Um, d'oh! Then I propose we kill it. I'll start again. :) |
All reactions
Sorry, something went wrong.
@rfcbot fcp cancel |
All reactions
Sorry, something went wrong.
@nikomatsakis proposal cancelled. |
All reactions
Sorry, something went wrong.
@rfcbot fcp merge This warning has been deny-by-default for a while. I propose we move it to a hard error. |
All reactions
Sorry, something went wrong.
Team member @nikomatsakis has proposed to merge this. The next step is review by the rest of the tagged teams: No concerns currently listed. Once these reviewers reach consensus, this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up! See this document for info about what commands tagged team members can give me. |
All reactions
Navigation Menu
🔔 This is now entering its final comment period, as per the review above. 🔔 |
All reactions
Sorry, something went wrong.
The final comment period is now complete. |
All reactions
Sorry, something went wrong.
cc @petrochenkov @rust-lang/compiler -- anyone want to make a quick stabilization PR that we can backport to 1.17 beta? |
All reactions
Sorry, something went wrong.
This seems like a good first bug. I wrote up some directions and posted them to forge for converting compatibility lints into hard errors. The descriptions probably haven't landed yet, but here is the PR: Marking a E-easy and E-mentor, the directions are in that PR. =) |
All reactions
Sorry, something went wrong.
Hello, I would like to work on this if no one is interested. |
All reactions
Sorry, something went wrong.
@topecongiro go for it:) |
All reactions
Sorry, something went wrong.
@topecongiro please feel free to ping me on irc ( |
All reactions
Sorry, something went wrong.
@nikomatsakis Thank you! |
All reactions
Sorry, something went wrong.
Make overlapping_inherent_impls lint a hard error Closes rust-lang#36889.
Make overlapping_inherent_impls lint a hard error Closes rust-lang#36889.
No branches or pull requests
UPDATE: We've decided to convert this warning into a hard error, but are still waiting for a PR to make that a reality. If you're interested in contributing, there are directions for writing such a PR available here.
This tracking issue pertains to a bug fix for #22889. The problem was that the compiler was accepting two inherent impls that both defined the same method, which creates an obvious ambiguity:
The most obvious fix here is to give one method a distinct name, though sometimes there are other possible rewrites (e.g., one could rewrite the impls to use a double dispatch pattern).
Current status
overlapping_inherent_impls
lint as warn-by-defaultoverlapping_inherent_impls
lint deny-by-defaultoverlapping_inherent_impls
lint a hard errorThe text was updated successfully, but these errors were encountered: