Rollup of 9 pull requests #107738
Merged
Rollup of 9 pull requests #107738
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Especially when trying to diagnose runaway future sizes, it might be more intuitive to sort the variants according to the control flow (aka their yield points) rather than the size of the variants.
"Round half-way cases" is a little confusing (it's a 'garden path sentence' as it's not immediately clear whether round is an adjective or verb). Make this sentence longer and clearer.
…r-span-trait-impl, r=compiler-errors
Refine error spans for "The trait bound `T: Trait` is not satisfied" when passing literal structs/tuples
This PR adds a new heuristic which refines the error span reported for "`T: Trait` is not satisfied" errors, by "drilling down" into individual fields of structs/enums/tuples to point to the "problematic" value.
Here's a self-contained example of the difference in error span:
```rs
struct Burrito<Filling> {
filling: Filling,
}
impl <Filling: Delicious> Delicious for Burrito<Filling> {}
fn eat_delicious_food<Food: Delicious>(food: Food) {}
fn will_type_error() {
eat_delicious_food(Burrito { filling: Kale });
// ^~~~~~~~~~~~~~~~~~~~~~~~~ (before) The trait bound `Kale: Delicious` is not satisfied
// ^~~~ (after) The trait bound `Kale: Delicious` is not satisfied
}
```
(kale is fine, this is just a silly food-based example)
Before this PR, the error span is identified as the entire argument to the generic function `eat_delicious_food`. However, since only `Kale` is the "problematic" part, we can point at it specifically. In particular, the primary error message itself mentions the missing `Kale: Delicious` trait bound, so it's much clearer if this part is called out explicitly.
---
The _existing_ heuristic tries to label the right function argument in `point_at_arg_if_possible`. It goes something like this:
- Look at the broken base trait `Food: Delicious` and find which generics it mentions (in this case, only `Food`)
- Look at the parameter type definitions and find which of them mention `Filling` (in this case, only `food`)
- If there is exactly one relevant parameter, label the corresponding argument with the error span, instead of the entire call
This PR extends this heuristic by further refining the resulting expression span in the new `point_at_specific_expr_if_possible` function. For each `impl` in the (broken) chain, we apply the following strategy:
The strategy to determine this span involves connecting information about our generic `impl`
with information about our (struct) type and the (struct) literal expression:
- Find the `impl` (`impl <Filling: Delicious> Delicious for Burrito<Filling>`)
that links our obligation (`Kale: Delicious`) with the parent obligation (`Burrito<Kale>: Delicious`)
- Find the "original" predicate constraint in the impl (`Filling: Delicious`) which produced our obligation.
- Find all of the generics that are mentioned in the predicate (`Filling`).
- Examine the `Self` type in the `impl`, and see which of its type argument(s) mention any of those generics.
- Examing the definition for the `Self` type, and identify (for each of its variants) if there's a unique field
which uses those generic arguments.
- If there is a unique field mentioning the "blameable" arguments, use that field for the error span.
Before we do any of this logic, we recursively call `point_at_specific_expr_if_possible` on the parent
obligation. Hence we refine the `expr` "outwards-in" and bail at the first kind of expression/impl we don't recognize.
This function returns a `Result<&Expr, &Expr>` - either way, it returns the `Expr` whose span should be
reported as an error. If it is `Ok`, then it means it refined successfull. If it is `Err`, then it may be
only a partial success - but it cannot be refined even further.
---
I added a new test file which exercises this new behavior. A few existing tests were affected, since their error spans are now different. In one case, this leads to a different code suggestion for the autofix - although the new suggestion isn't _wrong_, it is different from what used to be.
This change doesn't create any new errors or remove any existing ones, it just adjusts the spans where they're presented.
---
Some considerations: right now, this check occurs in addition to some similar logic in `adjust_fulfillment_error_for_expr_obligation` function, which tidies up various kinds of error spans (not just trait-fulfillment error). It's possible that this new code would be better integrated into that function (or another one) - but I haven't looked into this yet.
Although this code only occurs when there's a type error, it's definitely not as efficient as possible. In particular, there are definitely some cases where it degrades to quadratic performance (e.g. for a trait `impl` with 100+ generic parameters or 100 levels deep nesting of generic types). I'm not sure if these are realistic enough to worry about optimizing yet.
There's also still a lot of repetition in some of the logic, where the behavior for different types (namely, `struct` vs `enum` variant) is _similar_ but not the same.
---
I think the biggest win here is better targeting for tuples; in particular, if you're using tuples + traits to express variadic-like functions, the compiler can't tell you which part of a tuple has the wrong type, since the span will cover the entire argument. This change allows the individual field in the tuple to be highlighted, as in this example:
```
// NEW
LL | want(Wrapper { value: (3, q) });
| ---- ^ the trait `T3` is not implemented for `Q`
// OLD
LL | want(Wrapper { value: (3, q) });
| ---- ^~~~~~~~~~~~~~~~~~~~~~~~~ the trait `T3` is not implemented for `Q`
```
Especially with large tuples, the existing error spans are not very effective at quickly narrowing down the source of the problem.
…imulacrum Add nicer output to PGO build timer This PR modifies the timer used in the PGO build script to contain nicer, hierarchical output of the individual build steps. It's not trivial to test locally, so I'll fire up a dist build right away. r? ``@Mark-Simulacrum``
…r-errors Sort Generator `print-type-sizes` according to their yield points Especially when trying to diagnose runaway future sizes, it might be more intuitive to sort the variants according to the control flow (aka their yield points) rather than the size of the variants.
Clarify wording on f64::round() and f32::round() "Round half-way cases" is a little confusing (it's a 'garden path sentence' as it's not immediately clear whether round is an adjective or verb). Make this sentence longer and clearer.
…acrum end entry paragraph with a period (.)
remove unused rustc_* imports
…c-into-struct, r=notriddle Turn MarkdownWithToc into a struct with named fields Extracted the commit from rust-lang#107640. r? `@notriddle`
…=cjgillot interpret: move discriminant reading and writing to separate file This is quite different from the otherwise fairly general read and write functions in place.rs and operand.rs, and also it's nice to have these two functions close together as they are basically inverses of each other.
…=dtolnay Add mailmap for commits made by xes@meta.com Saw my contributions split at https://thanks.rust-lang.org/rust/1.69.0/ I assume it's because I committed with my work and personal email?
rustbot
added
A-meta
rustbot
added
T-rustdoc
|
@bors r+ rollup=never p=9 |
bors
added
S-waiting-on-bors
|
☀️ Test successful - checks-actions |
|
📌 Perf builds for each rolled up PR: previous master: 7ff69b49df In the case of a perf regression, run the following command for each PR you suspect might be the cause: |
|
Finished benchmarking commit (e1eaa2d): comparison URL. Overall result: no relevant changes - no action needed@rustbot label: -perf-regression Instruction countThis benchmark run did not return any relevant results for this metric. Max RSS (memory usage)This benchmark run did not return any relevant results for this metric. 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.
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Successful merges:
T: Traitis not satisfied" when passing literal structs/tuples #106477 (Refine error spans for "The trait boundT: Traitis not satisfied" when passing literal structs/tuples)print-type-sizesaccording to their yield points #107692 (Sort Generatorprint-type-sizesaccording to their yield points)Failed merges:
r? @ghost
@rustbot modify labels: rollup
Create a similar rollup