Rollup of 3 pull requests - #161780
Closed
JonathanBrouwer wants to merge 6 commits into
Closed
Rollup of 3 pull requests#161780JonathanBrouwer wants to merge 6 commits into
JonathanBrouwer wants to merge 6 commits into
Conversation
…08 notes The `for<...>` prefixes in `cmp_fn_sig` are built from the region map returned by `name_all_regions`. That map also contained regions that are bound by an enclosing binder and merely escape through the binder being named, so nested binders in expected/found notes listed lifetimes they don't bind, printing invalid types such as `&mut for<'a> fn(for<'a> fn(&'a ()))` for `&mut for<'a> fn(fn(&'a ()))`. Key the folder's map by the region's binder offset and only return the regions actually bound by the binder being named. The offset in the key also fixes a latent collision between a bound and an escaping region sharing the same bound variable index. The printed text is unaffected: the `name` closure already skips escaping regions when writing to the printer, which is why diagnostic labels were already correct.
Replace `Allocator + Clone` with `AllocatorClone` in btree powered by ripgrep r? nia-e
…nting, r=jackh726
Don't list escaping bound regions in nested `for<...>` binders of E0308 notes
The expected/found notes of "one type is more general than the other" errors could print types that are not even valid syntax, repeating lifetimes from an outer binder inside nested `for<...>` lists:
```
= note: expected mutable reference `&mut for<'a> fn(for<'a> fn(&'a ()))`
found mutable reference `&mut fn(fn(&()))`
```
The expected type here is actually `&mut for<'a> fn(fn(&'a ()))`, the inner fn pointer binds nothing. In the example from rust-lang#111365 the note printed `for<'o> fn(for<'a, 'o> fn(&'a (), &'o ()))` even though the inner binder only binds `'a`.
The cause is in how `cmp_fn_sig` builds its `for<...>` prefixes. It calls `name_all_regions` and joins every region of the returned map into the list. But `RegionFolder` folded every region at or above the depth of the binder being printed (`db >= self.current_index`), so regions that are bound by an enclosing binder and merely escape through the current one also ended up in the map. The pretty printer's own text output already skipped those regions via a special case in the naming closure (which is why the labels of these diagnostics were correct, see rust-lang#102392), but the returned map kept them, and `cmp_fn_sig` recurses through nested fn pointers one binder at a time, so every nested `for<...>` list picked up all the outer lifetimes. That special case also rebound the escaping region to `self.current_index`, i.e. shifted its De Bruijn index down so that it looked like it was bound by the current binder.
This PR makes `RegionFolder` only fold regions bound by the binder being named (`db == self.current_index`). Escaping regions are left untouched: they keep their index, and they already carry a name because the enclosing binder named them when it was folded (`cmp_fn_sig` recurses on the folded value). This makes the rust-lang#102392 special case in the naming closure dead, so it is removed along with the two `DebruijnIndex` parameters it needed.
With this change, the two examples above print:
```
= note: expected mutable reference `&mut for<'a> fn(fn(&'a ()))`
found mutable reference `&mut fn(fn(&()))`
```
```
= note: expected fn pointer `for<'o> fn(for<'a> fn(&'a (), &'o ()))`
found fn pointer `fn(for<'a> fn(&'a (), &'a ()))`
```
The `found` line of `tests/ui/nll/relate_tys/placeholder-outlives-existential.rs` also loses its leaked binders and now matches the type written in the test's source.
Fixes rust-lang#134410
This also fixes the leaked outer lifetimes reported in rust-lang#111365, but I left that issue open since it additionally asks for renaming of shadowed lifetimes (`for<'a> fn(for<'a> ...)` for two distinct lifetimes both named `'a` in the source), which is a separate problem.
…nBrouwer Add codegen test for disjunction fed to unreachable_unchecked Closes rust-lang#115026
Member
Author
|
@bors r+ p=5 |
Contributor
Contributor
|
This pull request was unapproved due to being closed. |
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
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:
Allocator + ClonewithAllocatorClonein btree #161684 (ReplaceAllocator + ClonewithAllocatorClonein btree)for<...>binders of E0308 notes #159232 (Don't list escaping bound regions in nestedfor<...>binders of E0308 notes)r? @ghost
Create a similar rollup