Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Remove duplicate notes from error on inter-crate ambiguous impl of traits #99095

Merged
merged 1 commit into from
Jul 11, 2022
Merged

Remove duplicate notes from error on inter-crate ambiguous impl of traits #99095

merged 1 commit into from
Jul 11, 2022

Conversation

rhysd
Copy link
Contributor

@rhysd rhysd commented Jul 9, 2022

Fixes #99092

@rustbot rustbot added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Jul 9, 2022
@rust-highfive
Copy link
Collaborator

r? @davidtwco

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jul 9, 2022
@rhysd rhysd changed the title Remove duplicate notes from error on on inter-crate ambiguous impl of traits Remove duplicate notes from error on inter-crate ambiguous impl of traits Jul 9, 2022
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@compiler-errors
Copy link
Member

The change looks good! Can you move the UI test somewhere different than src/test/ui/issues? There is probably a better directory for it to live.

r? @compiler-errors

@rhysd
Copy link
Contributor Author

rhysd commented Jul 10, 2022

@compiler-errors Thank you for taking a look. I moved test to src/test/ui/inter-crate-ambiguity-causes-notes.{rs,stderr}.

@rust-log-analyzer

This comment has been minimized.

Sign in to view
@rhysd
Copy link
Contributor Author

rhysd commented Jul 10, 2022

@compiler-errors CI passed and I confirmed all UI tests passed on my local machine. Would you review this patch again?

@compiler-errors
Copy link
Member

This looks good to me. It's typically better to add new unit tests to subdirectories of src/test/ui instead of the top directory itself, so can you move that test you added to src/test/ui/coherence? Then I can approve. Thanks again.

@rhysd
Copy link
Contributor Author

rhysd commented Jul 10, 2022

Thanks for your advice. I moved the test case again.

@compiler-errors
Copy link
Member

Thanks!

@bors r+

@bors
Copy link
Contributor

bors commented Jul 10, 2022

📌 Commit d5aed20 has been approved by compiler-errors

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 10, 2022
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Jul 10, 2022
@matthiaskrgr
Rollup merge of rust-lang#99095 - rhysd:issue-99092, r=compiler-errors
e5b91c8 
Remove duplicate notes from error on inter-crate ambiguous impl of traits

Fixes rust-lang#99092
@matthiaskrgr matthiaskrgr mentioned this pull request Jul 10, 2022
Closed
@matthiaskrgr matthiaskrgr mentioned this pull request Jul 10, 2022
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Jul 10, 2022
@bors
Auto merge of rust-lang#99133 - matthiaskrgr:rollup-eignphd, r=matthi…
e1b348f 
…askrgr

Rollup of 5 pull requests

Successful merges:

 - rust-lang#98713 (promote placeholder bounds to 'static obligations)
 - rust-lang#99094 (Remove extra space in AtomicPtr::new docs)
 - rust-lang#99095 (Remove duplicate notes from error on inter-crate ambiguous impl of traits)
 - rust-lang#99114 (Group .test-arrow CSS rules and fix rgb/rgba property)
 - rust-lang#99128 (Fix `download-ci-llvm` NixOS patching for binaries)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 16d0d0b into rust-lang:master Jul 11, 2022
@rustbot rustbot added this to the 1.64.0 milestone Jul 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@compiler-errors compiler-errors

Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
1.64.0
Development

Successfully merging this pull request may close these issues.

Duplicate 'upstream crates may add a new impl of trait' notes on inter-crate ambiguous impl of traits
7 participants
@rhysd @rust-highfive @rust-log-analyzer @compiler-errors @bors @davidtwco @rustbot