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.

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

Support impl Trait in inlined documentation #61613

Merged
merged 6 commits into from
Aug 25, 2019

Conversation

sinkuu
Copy link
Contributor

@sinkuu sinkuu commented Jun 7, 2019

impl Trait in argument position was not properly rendered when inlined from other crates. (a live example on docs.rs)

old

new

@rust-highfive
Copy link
Collaborator

The job x86_64-gnu-llvm-6.0 of your PR failed on Travis (raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

Click to expand the log.
travis_time:end:0efa6ae3:start=1559894987881585530,finish=1559894988615283313,duration=733697783
$ git checkout -qf FETCH_HEAD
travis_fold:end:git.checkout

Encrypted environment variables have been removed for security reasons.
See https://docs.travis-ci.com/user/pull-requests/#pull-requests-and-security-restrictions
$ export SCCACHE_BUCKET=rust-lang-ci-sccache2
$ export SCCACHE_REGION=us-west-1
$ export GCP_CACHE_BUCKET=rust-lang-ci-cache
$ export AWS_ACCESS_KEY_ID=AKIA46X5W6CZEJZ6XT55
---

[00:04:17] travis_fold:start:tidy
travis_time:start:tidy
tidy check
[00:04:18] tidy error: /checkout/src/librustdoc/clean/mod.rs:1702: line longer than 100 chars
[00:04:22] some tidy checks failed
[00:04:22] 
[00:04:22] 
[00:04:22] command did not execute successfully: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-tools-bin/tidy" "/checkout/src" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0/bin/cargo" "--no-vendor" "--quiet"
[00:04:22] 
[00:04:22] 
[00:04:22] failed to run: /checkout/obj/build/bootstrap/debug/bootstrap test src/tools/tidy
[00:04:22] Build completed unsuccessfully in 0:01:11
---
travis_time:end:01252620:start=1559895263298385339,finish=1559895263303144686,duration=4759347
travis_fold:end:after_failure.3
travis_fold:start:after_failure.4
travis_time:start:0af192c8
$ ln -s . checkout && for CORE in obj/cores/core.*; do EXE=$(echo $CORE | sed 's|obj/cores/core\.[0-9]*\.!checkout!\(.*\)|\1|;y|!|/|'); if [ -f "$EXE" ]; then printf travis_fold":start:crashlog\n\033[31;1m%s\033[0m\n" "$CORE"; gdb --batch -q -c "$CORE" "$EXE" -iex 'set auto-load off' -iex 'dir src/' -iex 'set sysroot .' -ex bt -ex q; echo travis_fold":"end:crashlog; fi; done || true
travis_fold:end:after_failure.4
travis_fold:start:after_failure.5
travis_time:start:10f347cc
travis_time:start:10f347cc
$ cat ./obj/build/x86_64-unknown-linux-gnu/native/asan/build/lib/asan/clang_rt.asan-dynamic-i386.vers || true
cat: ./obj/build/x86_64-unknown-linux-gnu/native/asan/build/lib/asan/clang_rt.asan-dynamic-i386.vers: No such file or directory
travis_fold:end:after_failure.5
travis_fold:start:after_failure.6
travis_time:start:0019eb18
$ dmesg | grep -i kill

I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact @TimNN. (Feature Requests)

@sinkuu sinkuu force-pushed the impl_trait_inline branch from 7844ce4 to 6129d61 Compare June 7, 2019 08:35
@jonas-schievink jonas-schievink added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jun 10, 2019
@bors
Copy link
Contributor

bors commented Jun 21, 2019

☔ The latest upstream changes (presumably #60293) made this pull request unmergeable. Please resolve the merge conflicts.

@sinkuu sinkuu force-pushed the impl_trait_inline branch from 6129d61 to ecda335 Compare June 21, 2019 10:32
Copy link
Member

@ollie27 ollie27 left a comment

Choose a reason for hiding this comment

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

Sorry for the late review.

Even with the (i as u32) -> param.index change I suggested there is still a problem with things like pub fn h(_x: impl Iterator<Item = u8>) {}, they render as:
image
The issue is that rustdoc uses simplify::where_clauses to convert the equality predicates into type bindings so that needs to be run before moving where predicates that are actually part of impl traits into cx.impl_trait_bounds. We could leave fixing this to a follow up PR though.

src/librustdoc/clean/mod.rs Outdated Show resolved Hide resolved
src/test/rustdoc/inline_cross/impl_trait.rs Outdated Show resolved Hide resolved
src/test/rustdoc/inline_cross/impl_trait.rs Outdated Show resolved Hide resolved
@ollie27 ollie27 added 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 Jul 6, 2019
@ollie27
Copy link
Member

ollie27 commented Jul 8, 2019

Looks good, thanks!

@bors r+

@bors
Copy link
Contributor

bors commented Jul 8, 2019

📌 Commit ea3e804 has been approved by ollie27

@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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jul 8, 2019
Centril added a commit to Centril/rust that referenced this pull request Jul 8, 2019
bors added a commit that referenced this pull request Jul 9, 2019
Rollup of 4 pull requests

Successful merges:

 - #61613 (Support `impl Trait` in inlined documentation)
 - #62090 (typeck: merge opaque type inference logic)
 - #62403 (Replace SliceConcatExt trait with inherent methods and SliceConcat helper trait)
 - #62494 (Remove unused dependencies)

Failed merges:

r? @ghost
@bors
Copy link
Contributor

bors commented Jul 9, 2019

⌛ Testing commit ea3e804 with merge 0ab453ada4ea66b0902b62d5db1648c0c9e0c69e...

@Centril
Copy link
Contributor

Centril commented Jul 9, 2019

Failed in #62509 (comment), @bors r-

@bors bors added 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-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Jul 9, 2019
@Centril
Copy link
Contributor

Centril commented Jul 9, 2019

@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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jul 9, 2019
@sinkuu sinkuu force-pushed the impl_trait_inline branch from ea3e804 to 2cf5a9e Compare July 9, 2019 08:02
@sinkuu
Copy link
Contributor Author

sinkuu commented Jul 9, 2019

I guess test failure on one platform was caused by indeterminism of HashMap iteration, so I changed it to use BTreeMap (param-index order should be ok).
Also a drive-by fix for ICE with impl Trait in type bounds.

@sinkuu
Copy link
Contributor Author

sinkuu commented Jul 16, 2019

@ollie27 Could you re-approve?

@bors
Copy link
Contributor

bors commented Aug 11, 2019

☔ The latest upstream changes (presumably #63471) made this pull request unmergeable. Please resolve the merge conflicts.

@bors bors added 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-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Aug 11, 2019
@JohnCSimon
Copy link
Member

Ping from triage
@sinkuu - can you please fix the merge conflict so that @ollie27 can re-review?
Thank you.

@sinkuu sinkuu force-pushed the impl_trait_inline branch from 2cf5a9e to 1fe6160 Compare August 19, 2019 11:10
@sinkuu
Copy link
Contributor Author

sinkuu commented Aug 19, 2019

Rebased.

@JohnCSimon JohnCSimon 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 Aug 19, 2019
@ollie27
Copy link
Member

ollie27 commented Aug 25, 2019

Let's give this another go.

@bors r+

@bors
Copy link
Contributor

bors commented Aug 25, 2019

📌 Commit 1fe6160 has been approved by ollie27

@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 Aug 25, 2019
@bors
Copy link
Contributor

bors commented Aug 25, 2019

⌛ Testing commit 1fe6160 with merge 521d784...

bors added a commit that referenced this pull request Aug 25, 2019
@bors
Copy link
Contributor

bors commented Aug 25, 2019

☀️ Test successful - checks-azure
Approved by: ollie27
Pushing 521d784 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Aug 25, 2019
@bors bors merged commit 1fe6160 into rust-lang:master Aug 25, 2019
@bors bors mentioned this pull request Aug 25, 2019
@sinkuu sinkuu deleted the impl_trait_inline branch August 25, 2019 23:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merged-by-bors This PR was explicitly merged by bors. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants