-
Notifications
You must be signed in to change notification settings - Fork 12.7k
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
rustdoc: re-elide cross-crate default trait-object lifetime bounds #107637
rustdoc: re-elide cross-crate default trait-object lifetime bounds #107637
Conversation
e630fc9
to
c62daa1
Compare
The changes look good to me, thanks for working on this! I think it'd be nice to have someone from the compiler team to also take a look at this to confirm that it's the right solution and also another person from rustdoc to ensure I didn't miss something. |
@bors try @rust-timer queue |
This comment has been minimized.
This comment has been minimized.
⌛ Trying commit c62daa166c21ffb8c12d6df6f1cab6edcb310ed3 with merge c0b38d8c099b44a2e66768efae13b34971552f9d... |
💔 Test failed - checks-actions |
Spurious network failure as far as I can tell 😞, could you please retry? |
@bors try @rust-timer queue |
This comment has been minimized.
This comment has been minimized.
⌛ Trying commit c62daa166c21ffb8c12d6df6f1cab6edcb310ed3 with merge d784b9581340dda4299f1b68760e703b72c91231... |
☀️ Try build successful - checks-actions |
This comment has been minimized.
This comment has been minimized.
Finished benchmarking commit (d784b9581340dda4299f1b68760e703b72c91231): comparison URL. Overall result: ❌✅ regressions and improvements - ACTION NEEDEDBenchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. While you can manually mark this PR as fit for rollup, we strongly recommend not doing so since this PR may lead to changes in compiler perf. Next Steps: If you can justify the regressions found in this try perf run, please indicate this with @bors rollup=never Instruction countThis is a highly reliable metric that was used to determine the overall result at the top of this comment.
Max RSS (memory usage)ResultsThis is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.
CyclesResultsThis is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.
|
This comment has been minimized.
This comment has been minimized.
r? compiler |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This all looks okay to me as well.
Then let's go! Thanks to everyone involved! @bors r=notriddle,cgillot,GuillaumeGomez |
☀️ Test successful - checks-actions |
Finished benchmarking commit (7820972): comparison URL. Overall result: ❌ regressions - ACTION NEEDEDNext Steps: If you can justify the regressions found in this perf run, please indicate this with @rustbot label: +perf-regression Instruction countThis is a highly reliable metric that was used to determine the overall result at the top of this comment.
Max RSS (memory usage)ResultsThis is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.
CyclesThis benchmark run did not return any relevant results for this metric. Binary sizeThis benchmark run did not return any relevant results for this metric. Bootstrap: 650.406s -> 649.872s (-0.08%) |
Right, that was to be expected (doing more work for every single trait object type) and we've already seen this in the 2nd of 3 perf runs. Let me know if this is acceptable or if I should try to make this faster (fast path or cache maybe?). I'm already delaying as much work as possible (hence |
I think it’s fine, anyway. |
Ok, then |
…-args, r=<try> rustdoc: elide cross-crate default generic arguments Early draft. Requesting perf run. Thx :) CC `@GuillaumeGomez` Elide cross-crate generic arguments if they coincide with their default. TL;DR: Most notably, no more `Box<…, Global>` in `std`'s docs, just `Box<…>` from now on. Fixes rust-lang#80379. Also helps with rust-lang#44306. Follow-up to rust-lang#103885,rust-lang#107637. `@rustbot` label T-rustdoc A-cross-crate-reexports S-experimental r? `@ghost`
Heads up: This PR seems to be based around the reference's section on default trait object lifetimes, but that section of the reference is inaccurate in many ways. I imagine the main cases for Rustdoc would be
Unfortunately the exception to trait bounds overriding types bound is "in function signatures, sometimes" (when no bound parameters are late bound or I'm not sure what the best way to actually test Rustdoc on this matter is, but would be willing to give it a shot if there's some guidance on how to test Rustdoc somewhere. |
I've been meaning to reply to this comment of yours for a while now but I've since forgotten about it. I'm not an expert on region checking and I haven't carefully read the implementation of object lifetime defaults yet but I'm gonna check each snippet you posted over at rust-lang/reference. Some things definitely look fishy |
Also note that most of the things you listed here and over at rust-lang/reference shouldn't be relevant to rustdoc since they only affect types found in bodies (function, closure, constant) which rustdoc doesn't (really) render. |
…-args, r=<try> rustdoc: elide cross-crate default generic arguments Elide cross-crate generic arguments if they coincide with their default. TL;DR: Most notably, no more `Box<…, Global>` in `std`'s docs, just `Box<…>` from now on. Fixes rust-lang#80379. Also helps with rust-lang#44306. Follow-up to rust-lang#103885, rust-lang#107637. r? `@ghost`
…-args, r=<try> rustdoc: elide cross-crate default generic arguments Elide cross-crate generic arguments if they coincide with their default. TL;DR: Most notably, no more `Box<…, Global>` in `std`'s docs, just `Box<…>` from now on. Fixes rust-lang#80379. Also helps with rust-lang#44306. Follow-up to rust-lang#103885, rust-lang#107637. r? `@ghost`
…en-args, r=GuillaumeGomez rustdoc: elide cross-crate default generic arguments Elide cross-crate generic arguments if they coincide with their default. TL;DR: Most notably, no more `Box<…, Global>` in `std`'s docs, just `Box<…>` from now on. Fixes rust-lang#80379. Also helps with rust-lang#44306. Follow-up to rust-lang#103885, rust-lang#107637. r? `@ghost`
…en-args, r=GuillaumeGomez rustdoc: elide cross-crate default generic arguments Elide cross-crate generic arguments if they coincide with their default. TL;DR: Most notably, no more `Box<…, Global>` in `std`'s docs, just `Box<…>` from now on. Fixes rust-lang#80379. Also helps with rust-lang#44306. Follow-up to rust-lang#103885, rust-lang#107637. r? ``@ghost``
Rollup merge of rust-lang#112463 - fmease:rustdoc-elide-x-crate-def-gen-args, r=GuillaumeGomez rustdoc: elide cross-crate default generic arguments Elide cross-crate generic arguments if they coincide with their default. TL;DR: Most notably, no more `Box<…, Global>` in `std`'s docs, just `Box<…>` from now on. Fixes rust-lang#80379. Also helps with rust-lang#44306. Follow-up to rust-lang#103885, rust-lang#107637. r? ``@ghost``
Hide trait-object lifetime bounds (re-exported from an external crate) if they coincide with their default.
Partially addresses #44306. Follow-up to #103885. Zulip discussion.
Most notably, if
std
exported something fromcore
containing a type likeBox<dyn Fn()>
, then it would now be rendered asBox<dyn Fn(), Global>
instead ofBox<dyn Fn() + 'static, Global>
(hiding+ 'static
as it is the default in this case). ShowingGlobal
here is a separate issue, #80379, which is on my agenda.Note that I am not really fond of the fact that I had to add a parameter to such a widely used function (30+ call sites) to address such a niche bug.
CC @GuillaumeGomez
Requesting a review from a compiler contributor or team member as recommended on Zulip.
r? compiler
@rustbot label T-compiler T-rustdoc A-cross-crate-reexports