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

Rollup of 10 pull requests #108052

Merged
merged 31 commits into from
Feb 14, 2023
Merged

Rollup of 10 pull requests #108052

merged 31 commits into from
Feb 14, 2023

Conversation

matthiaskrgr
Copy link
Member

Successful merges:

Failed merges:

r? @ghost
@rustbot modify labels: rollup

Create a similar rollup

SpanishPear and others added 30 commits January 22, 2023 17:05
--wip-- [skip ci]

get the generic text and put it int he suggestion, but suggestion not working on derive subdiagnostic

refactor away from derives and use span_suggestion() instead. Show's the correct(?) generic contents, but overwrites the fn name :(

x fmt

drop commented code and s/todo/fixme

get the correct diagnostic for functions, at least

x fmt

remove some debugs

remove format

remove debugs

remove useless change

remove useless change

remove legacy approach

correct lookahead + error message contains the ident name

fmt

refactor code

tests

add tests

remoev debug

remove comment
This commit makes intra-doc link tooltips consistent with generated
links in function signatures and item tables, with the format
`itemtype foo::bar::baz`. This way, you can tell if a link points at
a trait or a type (for example) by mousing over it.

See also fce944d
…66_fix, r=TaKO8Ki

 Suggest fix for misplaced generic params on fn item rust-lang#103366

fixes rust-lang#103366

This still has some work to go, but works for 2/3 of the initial base cases described in #1033366

simple fn:
```
error: expected identifier, found `<`
 --> shreys/test_1.rs:1:3
  |
1 | fn<T> id(x: T) -> T { x }
  |   ^ expected identifier
  |
help: help: place the generic parameter list after the function name:
  |
1 | fn id<T>(x: T) -> T { x }
  |    ~~~~

```

Complicated bounds
```
error: expected identifier, found `<`
 --> spanishpear/test_2.rs:1:3
  |
1 | fn<'a, B: 'a + std::ops::Add<Output = u32>> f(_x: B) { }
  |   ^ expected identifier
  |
help: help: place the generic parameter list after the function name:
  |
1 | fn f<'a, B: 'a + std::ops::Add<Output = u32>>(_x: B) { }
  |    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
```

Opening a draft PR for comments on approach, particularly I have the following questions:
 -  [x]  Is it okay to be using `err.span_suggestion` over struct derives? I struggled to get the initial implementation (particularly the correct suggestion message) on struct derives, although I think given what I've learned since starting, I could attempt re-doing it with that approach.
  -  [x] in the case where the snippet cannot be obtained from a span, is the `help` but no suggestion okay? I think yes (also, when does this case occur?)
  -  [x] are there any red flags for the generalisation of this work for relevant item kinds (i.e. `struct`, `enum`, `trait`, and `union`). My basic testing indicates it does work for those types except the help tip is currently hardcoded to `after the function name` - which should change dependent on the item.
  - [x] I am planning to not show the suggestion if there is already a `<` after the item identifier, (i.e. if there are already generics, as after a function name per the original issue). Any major objections?
  - [x] Is the style of error okay? I wasn't sure if there was a way to make it display nicer, or if thats handled by span_suggestion

These aren't blocking questions, and I will keep working on:
  - check if there is a `<` after the ident (and if so, not showing the suggestion)
  - generalize the help message
  - figuring out how to write/run/etc ui tests (including reading the docs for them)
  - logic cleanups
…_canonical_goal, r=lcnr

Check for overflow in evaluate_canonical_goal

r? `@lcnr`
…mpiler-errors

Avoid ICE when the generic_span is empty

Fixes rust-lang#107998
r? ```@TaKO8Ki```
"Basic usage" is redundant for there is just one example
…orkingjubilee

Shrink size of array benchmarks

Might've overdone it with the size of these benchmarks, as there's no need for them to be quite as large.

Fixes rust-lang#108011
add message to update Cargo.toml when x is changed

`@jyn514` Is this correct?

As mentioned in rust-lang#108021
…-tooltips, r=GuillaumeGomez

rustdoc: add more tooltips to intra-doc links

This commit makes intra-doc link tooltips consistent with generated links in function signatures and item tables, with the format `itemtype foo::bar::baz`. This way, you can tell if a link points at a trait or a type (for example) by mousing over it.

See also rust-lang#39697

Partially solves https://internals.rust-lang.org/t/rustdoc-suggestion-highlight-links-fn-s-mod-s-type-s-etc-appropriately-within-and-documentation/17931 (though the Internals thread asks for color-coding, while this PR adds a tooltip instead, it's accomplishing the same thing).

Before:

<img width="950" alt="image" src="https://user-images.githubusercontent.com/1593513/218653059-911cea01-7231-438a-ad98-be98ab73783f.png">

After:

<img width="432" alt="image" src="https://user-images.githubusercontent.com/1593513/218653201-34ca3aa7-18f1-4cb1-be68-a1411bbe797e.png">
s/eval_usize/eval_target_usize/ for clarity

r? `@nnethercote`

as discussed in https://rust-lang.zulipchat.com/#narrow/stream/131828-t-compiler/topic/.60Const.60.20and.20.60usize.60.2F.60u64.60 it is unclear what `usize` means and why we use a `u64` for something talking about `usize`. This renaming should make it clear that we're talking about `usize`s on the target platform, irrespective of the compiler host platform.
…ess, r=Mark-Simulacrum

Avoid using a dead email address as the main email address

This caused highfive to welcome me as a new contributor on every PR, because it couldn't find any commits of mine.
@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 Feb 14, 2023
@bors
Copy link
Contributor

bors commented Feb 14, 2023

⌛ Testing commit ea679fb with merge 6e01157...

@bors
Copy link
Contributor

bors commented Feb 14, 2023

☀️ Test successful - checks-actions
Approved by: matthiaskrgr
Pushing 6e01157 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Feb 14, 2023
@bors bors merged commit 6e01157 into rust-lang:master Feb 14, 2023
@rustbot rustbot added this to the 1.69.0 milestone Feb 14, 2023
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (6e01157): comparison URL.

Overall result: ❌ regressions - ACTION NEEDED

Next Steps: If you can justify the regressions found in this perf run, please indicate this with @rustbot label: +perf-regression-triaged along with sufficient written justification. If you cannot justify the regressions please open an issue or create a new PR that fixes the regressions, add a comment linking to the newly created issue or PR, and then add the perf-regression-triaged label to this PR.

@rustbot label: +perf-regression
cc @rust-lang/wg-compiler-performance

Instruction count

This is a highly reliable metric that was used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
9.5% [0.3%, 80.7%] 10
Regressions ❌
(secondary)
2.7% [2.7%, 2.7%] 1
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) 9.5% [0.3%, 80.7%] 10

Max RSS (memory usage)

Results

This 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.

mean range count
Regressions ❌
(primary)
2.0% [2.0%, 2.0%] 1
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) 2.0% [2.0%, 2.0%] 1

Cycles

Results

This 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.

mean range count
Regressions ❌
(primary)
22.9% [2.2%, 63.7%] 3
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) 22.9% [2.2%, 63.7%] 3

@rustbot rustbot added the perf-regression Performance regression. label Feb 15, 2023
@Mark-Simulacrum Mark-Simulacrum added the perf-regression-triaged The performance regression has been triaged. label Feb 21, 2023
@Mark-Simulacrum
Copy link
Member

Marking as triaged, likely caused by #108025. #108098 is a start towards reducing the regression here.

notriddle added a commit to notriddle/rust that referenced this pull request Feb 23, 2023
bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 28, 2023
…alloc, r=GuillaumeGomez

rustdoc: reduce allocations when generating tooltips

An attempt to reduce the perf regression in
rust-lang#108052 (comment)
saethlin pushed a commit to saethlin/miri that referenced this pull request Mar 5, 2023
…GuillaumeGomez

rustdoc: reduce allocations when generating tooltips

An attempt to reduce the perf regression in
rust-lang/rust#108052 (comment)
@matthiaskrgr matthiaskrgr deleted the rollup-p6r6rnl branch March 16, 2024 18:19
RalfJung pushed a commit to RalfJung/rust-analyzer that referenced this pull request Apr 20, 2024
…GuillaumeGomez

rustdoc: reduce allocations when generating tooltips

An attempt to reduce the perf regression in
rust-lang/rust#108052 (comment)
RalfJung pushed a commit to RalfJung/rust-analyzer that referenced this pull request Apr 27, 2024
…GuillaumeGomez

rustdoc: reduce allocations when generating tooltips

An attempt to reduce the perf regression in
rust-lang/rust#108052 (comment)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-meta Area: Issues about the rust-lang/rust repository. merged-by-bors This PR was explicitly merged by bors. perf-regression Performance regression. perf-regression-triaged The performance regression has been triaged. rollup A PR which is a rollup S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-libs Relevant to the library team, which will review and decide on the PR/issue. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. WG-trait-system-refactor The Rustc Trait System Refactor Initiative
Projects
None yet
Development

Successfully merging this pull request may close these issues.