Skip to content

rustdoc: Fix duplicated impls with generics#45620

Merged
bors merged 1 commit intorust-lang:masterfrom
ollie27:rustdoc_impl_generic_dupe
Nov 7, 2017
Merged

rustdoc: Fix duplicated impls with generics#45620
bors merged 1 commit intorust-lang:masterfrom
ollie27:rustdoc_impl_generic_dupe

Conversation

@ollie27
Copy link
Copy Markdown
Contributor

@ollie27 ollie27 commented Oct 29, 2017

The same type can appear multiple times in impls so we need to use a set
to avoid adding it multiple times.

Fixes: #45584

@ollie27
rustdoc: Fix duplicated impls with generics
676b4bb 
The same type can appear multiple times in impls so we need to use a set
to avoid adding it multiple times.
@rust-highfive
Copy link
Copy Markdown
Contributor

rust-highfive commented Oct 29, 2017

r? @frewsxcv

(rust_highfive has picked a reviewer for you, use r? to override)

@ollie27 ollie27 mentioned this pull request Oct 29, 2017
Closed
@frewsxcv
Copy link
Copy Markdown
Contributor

frewsxcv commented Oct 29, 2017

reassigning to someone from dev-tools team

@GuillaumeGomez
Copy link
Copy Markdown
Member

GuillaumeGomez commented Oct 30, 2017 •
edited
Loading

I think that the order won't be kept with only the set. You need both a Vec and a Set.

@kennytm kennytm added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Oct 30, 2017
@frewsxcv
Copy link
Copy Markdown
Contributor

frewsxcv commented Oct 30, 2017

I'm not sure about a FxHashSet but order is preserved with a BTreeSet

@QuietMisdreavus
Copy link
Copy Markdown
Contributor

QuietMisdreavus commented Oct 30, 2017

Right, but is that the order of the DefIds or the order of the pushes? Anyway, it shouldn't matter because the Set in question isn't the global set but a local list of items to add a single impl to. The order in which you choose lists to push to shouldn't matter if there's only one item you're pushing at a time.

@ollie27
Copy link
Copy Markdown
Contributor Author

ollie27 commented Oct 31, 2017

I think that the order won't be kept with only the set.

The order of what? This is just gathering DefIds to use as keys to insert into a HashMap and it doesn't matter what order we insert into the HashMap. FxHashSet is deterministic if that's what you're worried about.

@kennytm kennytm added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Nov 1, 2017
@carols10cents
Copy link
Copy Markdown
Member

carols10cents commented Nov 6, 2017

review ping @QuietMisdreavus or @GuillaumeGomez!

@QuietMisdreavus
Copy link
Copy Markdown
Contributor

QuietMisdreavus commented Nov 6, 2017

@bors r+

Sorry for letting this get stale!

@bors
Copy link
Copy Markdown
Collaborator

bors commented Nov 6, 2017

📌 Commit 676b4bb has been approved by QuietMisdreavus

@kennytm kennytm 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 Nov 7, 2017
@kennytm kennytm mentioned this pull request Nov 7, 2017
Closed
kennytm added a commit to kennytm/rust that referenced this pull request Nov 7, 2017
@kennytm
Rollup merge of rust-lang#45620 - ollie27:rustdoc_impl_generic_dupe, …
e957d52 
…r=QuietMisdreavus

rustdoc: Fix duplicated impls with generics

The same type can appear multiple times in impls so we need to use a set
to avoid adding it multiple times.

Fixes: rust-lang#45584
@bors
Copy link
Copy Markdown
Collaborator

bors commented Nov 7, 2017

⌛ Testing commit 676b4bb with merge 3e7f501...

bors added a commit that referenced this pull request Nov 7, 2017
@bors
Auto merge of #45620 - ollie27:rustdoc_impl_generic_dupe, r=QuietMisd…
3e7f501 
…reavus

rustdoc: Fix duplicated impls with generics

The same type can appear multiple times in impls so we need to use a set
to avoid adding it multiple times.

Fixes: #45584
@bors
Copy link
Copy Markdown
Collaborator

bors commented Nov 7, 2017

☀️ Test successful - status-appveyor, status-travis
Approved by: QuietMisdreavus
Pushing 3e7f501 to master...

@bors bors merged commit 676b4bb into rust-lang:master Nov 7, 2017
@ollie27 ollie27 deleted the rustdoc_impl_generic_dupe branch November 7, 2017 15:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

@GuillaumeGomez GuillaumeGomez

@QuietMisdreavus QuietMisdreavus

Labels

S-waiting-on-bors Status: 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.

8 participants

@ollie27 @rust-highfive @frewsxcv @GuillaumeGomez @QuietMisdreavus @carols10cents @bors @kennytm