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

rustdoc: fix min_const_generics with ty::Param #76297

Merged
merged 1 commit into from
Sep 12, 2020
Merged

rustdoc: fix min_const_generics with ty::Param #76297

merged 1 commit into from
Sep 12, 2020

Conversation

lcnr
Copy link
Contributor

@lcnr lcnr commented Sep 3, 2020

fixes #75913

r? @varkor cc @jyn514

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Sep 3, 2020
@lcnr lcnr added A-const-generics Area: const generics (parameters and arguments) F-min_const_generics labels Sep 3, 2020
@jyn514 jyn514 added the T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. label Sep 3, 2020
jyn514
jyn514 reviewed Sep 3, 2020
View reviewed changes
src/librustdoc/clean/mod.rs
Comment on lines +1371 to +1372
// `const_eval_poly` tries to to first substitute generic parameters which
// results in an ICE while manually constructing the constant and using `eval`
Copy link
Member

Choose a reason for hiding this comment

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

Maybe this should also fix the ICE to return an error?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The underlying ICE in const_eval_poly can't really be fixed. It is a weird sideeffect of how min_const_generics works.

It might make sense to check for ConstKind::Param in const_eval_raw but I personally don't think that that would be a cleaner solution 🤔

@varkor
Copy link
Member

varkor commented Sep 7, 2020

@bors r+ rollup

@lcnr
Copy link
Contributor Author

lcnr commented Sep 8, 2020

Somehow @varkor 's comment wasn't registered by bors.

@bors r=varkor rollup

@bors
Copy link
Contributor

bors commented Sep 8, 2020

📌 Commit ccf1f58 has been approved by varkor

@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 Sep 8, 2020
@bors
Copy link
Contributor

bors commented Sep 8, 2020

⌛ Testing commit ccf1f58 with merge 67d6720668a1a2464364994e8c94791a49ee16db...

@bors
Copy link
Contributor

bors commented Sep 8, 2020

💔 Test failed - checks-actions

@bors bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Sep 8, 2020
@jyn514
Copy link
Member

jyn514 commented Sep 8, 2020

#76442

failures:

---- src/test\rustdoc-ui\failed-doctest-output.rs - SomeStruct (line 11) stdout ----
Test executable failed (exit code 101).

@jyn514 jyn514 mentioned this pull request Sep 8, 2020
Merged
@lcnr
Copy link
Contributor Author

lcnr commented Sep 12, 2020 •
edited
Loading

It looks like #76442 is fixed, so @bors retry

@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 Sep 12, 2020
@RalfJung RalfJung mentioned this pull request Sep 12, 2020
Merged
@RalfJung
Copy link
Member

Cc @oli-obk (seems like an interesting case where const_eval_poly did not behave as expected)

bors added a commit to rust-lang-ci/rust that referenced this pull request Sep 12, 2020
@bors
Auto merge of rust-lang#76637 - RalfJung:rollup-eaykf93, r=RalfJung
2d6cbd2 
Rollup of 7 pull requests

Successful merges:

 - rust-lang#76114 (Add saturating methods for `Duration`)
 - rust-lang#76297 (rustdoc: fix min_const_generics with ty::Param)
 - rust-lang#76484 (Add MaybeUninit::assume_init_drop.)
 - rust-lang#76530 (Eliminate mut reference UB in Drop impl for Rc<T>)
 - rust-lang#76583 (Update `std::os` module documentation.)
 - rust-lang#76599 (Finish off revisions for const generics UI tests.)
 - rust-lang#76615 (Add missing examples on binary core traits)

Failed merges:

r? `@ghost`
@bors bors merged commit 5d90d6e into rust-lang:master Sep 12, 2020
@rustbot rustbot added this to the 1.48.0 milestone Sep 12, 2020
@lcnr lcnr deleted the const-ty-alias branch September 12, 2020 15:19
@lcnr lcnr mentioned this pull request Nov 17, 2020
Closed
16 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jyn514 jyn514 jyn514 left review comments

Assignees

@varkor varkor

Labels
A-const-generics Area: const generics (parameters and arguments) S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
1.48.0
Development

Successfully merging this pull request may close these issues.

Error documenting code that uses min_const_generics
7 participants
@lcnr @varkor @bors @jyn514 @RalfJung @rust-highfive @rustbot