Don't repeat lifetime names from outer binder in print#102514
Don't repeat lifetime names from outer binder in print#102514bors merged 4 commits intorust-lang:masterfrom
Conversation
rustbot
added
the
T-compiler
rust-highfive
added
the
S-waiting-on-review
b-naber
force-pushed
the
binder-print-fixes
branch
from
f6486a6 to
1042b65
Compare
b-naber
force-pushed
the
binder-print-fixes
branch
from
1042b65 to
a8f17e3
Compare
| *self | ||
| .region_map | ||
| .entry(br) | ||
| .or_insert_with(|| name(None, self.current_index, br)) |
There was a problem hiding this comment.
is that the issue why we're not printing the binder in the src/test/ui/where-clauses/higher-ranked-fn-type.rs test?
We have both a late-bound region with name 'b and index 0 and a placeholder with name 'b and index 0.
I am not too confident in this code and ideally we stop printing placeholders as if they were bound regions.
@jackh726 do you have an idea how this should work or any expectations for how difficult it is to remove the placeholder hack all-together
There was a problem hiding this comment.
Seems like the reason this isn't printed is that we skip the binder in the error reporting code?!
There was a problem hiding this comment.
yeah, we shouldn't do that. Either map_bound or replace with placeholders. Skipping binders is incorrect for errors as well i think
| let name = &mut self.name; | ||
| let region = match *r { | ||
| ty::ReLateBound(_, br) => *self.region_map.entry(br).or_insert_with(|| name(br)), | ||
| ty::ReLateBound(db, br) => { |
There was a problem hiding this comment.
that's weird, why is this not
| ty::ReLateBound(db, br) => { | |
| ty::ReLateBound(db, br) if db == self.current_index => { |
we shouldn't replace late-bound regions of inner binders.
There was a problem hiding this comment.
(ah, that's whatyou're currently doing inside of name. i think it's better to do this here instead)
|
Its actually crazy because I half-fixed this completely independently locally, when working on something else. I don't think we should stop printing placeholders as bound vars. They're sort of an "internal detail" of how we solve higher-ranked bounds. What instead would you want to show the user? The lifetime name must come from somewhere. |
There was a problem hiding this comment.
With the proposed change we don't create names for all anonymous late-bound vars.
There was a problem hiding this comment.
Yeah, this is what I did locally. Kind of weird. I guess it makes more sense to do what you had before.
b-naber
force-pushed
the
binder-print-fixes
branch
from
d262260 to
e83dcf4
Compare
Haven't thought too deeply about this, but: Ideally we only show placeholders to users after already introducing the binder somewhere. E.g. if they are part of the trace of a selection error, we should start with the higher ranked obligation which uses |
b-naber
force-pushed
the
binder-print-fixes
branch
from
0972678 to
048e637
Compare
|
I think this is a good start. @bors r+ |
bors
added
S-waiting-on-bors
…h726 Don't repeat lifetime names from outer binder in print Fixes rust-lang#102392 Fixes rust-lang#102414 r? `@lcnr`
…h726 Don't repeat lifetime names from outer binder in print Fixes rust-lang#102392 Fixes rust-lang#102414 r? ``@lcnr``
Rollup of 8 pull requests Successful merges: - rust-lang#99818 (don't ICE when normalizing closure input tys) - rust-lang#102514 (Don't repeat lifetime names from outer binder in print) - rust-lang#102661 (rustdoc: Document effect of fundamental types) - rust-lang#102782 (Add regression test for rust-lang#102124) - rust-lang#102790 (Fix llvm-tblgen for cross compiling) - rust-lang#102807 (Document `rust-docs-json` component) - rust-lang#102812 (Remove empty core::lazy and std::lazy) - rust-lang#102818 (Clean up rustdoc highlight.rs imports a bit) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup
7 participants
Fixes #102392
Fixes #102414
r? @lcnr