Skip to content

Prevent duplicated impl on foreign types - #53801

Merged
bors merged 1 commit into
rust-lang:masterfrom
GuillaumeGomez:duplicated-foreign-type-impls
Sep 1, 2018
Merged

Prevent duplicated impl on foreign types#53801
bors merged 1 commit into
rust-lang:masterfrom
GuillaumeGomez:duplicated-foreign-type-impls

Conversation

@GuillaumeGomez

@GuillaumeGomezGuillaumeGomez commented Aug 29, 2018

Copy link
Copy Markdown
Member

@rust-highfiverust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Aug 29, 2018
@QuietMisdreavus

QuietMisdreavus commented Aug 29, 2018

Copy link
Copy Markdown
Contributor

Is there a way we can reproduce this so we can test it? The change is small enough that it looks fine (though i wonder if there's a deeper issue causing those duplicates to get added in the first place) but i'm hesitant to take it on its own.

@QuietMisdreavus

QuietMisdreavus commented Aug 29, 2018

Copy link
Copy Markdown
Contributor

(Closed by accident, oops...)

@GuillaumeGomez

GuillaumeGomez commented Aug 30, 2018

Copy link
Copy Markdown
MemberAuthor

The issue is actually "easy": the T represents multiple types, therefore, the current path duplicates check doesn't work since they're different T everytime.

Also for the test, the better one would be the UI one. I'm waiting for it to be merged before adding test for this fix. Do you want me to open an issue for it?

@QuietMisdreavus

QuietMisdreavus commented Aug 30, 2018

Copy link
Copy Markdown
Contributor

Isn't there a way we can use a @count rule in a regular rustdoc test that could track this? It seems like a perfect fit for it.

@GuillaumeGomez

GuillaumeGomez commented Aug 31, 2018

Copy link
Copy Markdown
MemberAuthor

It is, just thought it wasn't a great idea.

@QuietMisdreavus

QuietMisdreavus commented Aug 31, 2018

Copy link
Copy Markdown
Contributor

If you think you can get a better test in later on, then i guess it's fine.

@bors r+ rollup

@bors

bors commented Aug 31, 2018

Copy link
Copy Markdown
Collaborator

📌 Commit 3b98571 has been approved by QuietMisdreavus

@borsbors 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 Aug 31, 2018
kennytm added a commit to kennytm/rust that referenced this pull request Sep 1, 2018
@kennytm
Rollup merge ofrust-lang#53801- GuillaumeGomez:duplicated-foreign-t…
f370814
…ype-impls, r=QuietMisdreavus
Prevent duplicated impl on foreign types
Fixesrust-lang#53689.
r? @QuietMisdreavus
@kennytmkennytm mentioned this pull request Sep 1, 2018
Merged
bors added a commit that referenced this pull request Sep 1, 2018
@bors
Auto merge of#53884- kennytm:rollup, r=kennytm
839d99c
Rollup of 9 pull requests
Successful merges:
- #53076 (set cfg(rustdoc) when rustdoc is running on a crate)
- #53622 (cleanup: Add main functions to some UI tests)
- #53769 (Also link Clippy repo in the CONTRIBUTING.md file)
- #53774 (Add rust-gdbgui script.)
- #53781 (bench: libcore: fix build failure of any.rs benchmark (use "dyn Any"))
- #53782 (Make Arc cloning mechanics clearer in module docs)
- #53790 (Add regression test for issue #52060)
- #53801 (Prevent duplicated impl on foreign types)
- #53850 (Nuke the `const_to_allocation` query)
@bors
bors merged commit 3b98571 into rust-lang:masterSep 1, 2018
@GuillaumeGomez
GuillaumeGomez deleted the duplicated-foreign-type-impls branch September 1, 2018 22:26
@ollie27ollie27 mentioned this pull request Sep 9, 2018
Merged
kennytm added a commit to kennytm/rust that referenced this pull request Sep 12, 2018
@kennytm
Rollup merge ofrust-lang#54087- ollie27:rustdoc_blanket_impl, r=Qui…
40d9740
…etMisdreavus
rustdoc: Remove generated blanket impls from trait pages
rust-lang#53801 only deduped the generated blanket impls but they shouldn't be displayed at all because the original blanket impl is already in the "Implementors" section. This also removes the impls from the sidebar.
Fixesrust-lang#53689
r? @QuietMisdreavus
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

@QuietMisdreavusQuietMisdreavus

Labels

S-waiting-on-borsStatus: Waiting on bors to run and complete tests. Bors will change the label on completion.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@GuillaumeGomez@QuietMisdreavus@bors@rust-highfive