Strict region folders #110312

nnethercote · 2023-04-14T06:40:47Z

Some region folders expect certain region variants to never occur. Other region folders handle all region variants. I was wondering if we could be stricter. So I tried changing every region folder to not accept any variants, and then gradually allowed variants until the full test suite passed.

The results are less enlightening than I'd hoped. I was expecting to see some common patterns but there aren't any, there is no consistency at all. I'm ambivalent about whether this should be merged, but I figure it's worth showing others. If we do merge it, we should probably do a crater run first to flush out any edge cases not covered by the test suite.

r? @oli-obk

rustbot · 2023-04-14T06:40:56Z

Some changes occurred to the core trait solver

cc @rust-lang/initiative-trait-system-refactor

compiler-errors · 2023-04-14T06:56:46Z

If we do merge it, we should probably do a crater run first to flush out any edge cases not covered by the test suite.

This is insufficient, IMO. I assume most of these bug cases are going to be triggered not in good-path code, but on subtle combinations of bad-path code.

Anyways, I'll take a look at this tomorrow and try to find ways of triggering some of these bugs, or at least I can leave inline comments for the ones that confidently can remain stronger assertions.

bors · 2023-04-14T11:25:02Z

☔ The latest upstream changes (presumably #110311) made this pull request unmergeable. Please resolve the merge conflicts.

jackh726 · 2023-04-14T14:43:48Z

If we do merge it, we should probably do a crater run first to flush out any edge cases not covered by the test suite.

I argue that we should do a crater run regardless on whether we merge or not. Imo, it's always good to find where the test suite is lacking coverage. One way to do that is to, as in this PR, place constraints on the compiler that pass the test suite and test on crater.

This would help two fold: one, it would directly help expand test coverage; two, it help to document the patterns that extra uses support.

jackh726 · 2023-04-14T14:49:37Z

This is insufficient, IMO. I assume most of these bug cases are going to be triggered not in good-path code, but on subtle combinations of bad-path code.

What exactly is the argument here? One argument should be that we gracefully handle "invalid" state, with the expectation that we catch it later. But a counter argument is that we should detect that state early (even if we just delay the bug).

compiler-errors · 2023-04-14T14:59:28Z

@jackh726:

What exactly is the argument here?

The argument is that I don't think that we should be unnecessarily introducing ICEs into the compiler, even on failed compilation sessions, so I don't think we should land this without more testing than just crater. I just don't want to see "Let's land this now and see what ICEs people file later". For example, a UI-test-guided approach is insufficient because this isn't exercising, e.g., every combination of parser recovery that ends up causing us to generate ReError and other region errors.

End users are frustrated when their already messed up code ends up ICEing due to far-too-strict assertions just like this PR is introducing, especially since the PR description above mentions that the changes here aren't even particularly compelling because there aren't general patterns in the choices of lifetimes that each fold method is supposed to support.

compiler-errors · 2023-04-14T15:02:34Z

One argument should be that we gracefully handle "invalid" state, with the expectation that we catch it later. But a counter argument is that we should detect that state early (even if we just delay the bug).

A handful of these folders are happening in diagnostics code anyways, so it's not worthwhile delaying an additional bug when we're already constructing a diagnostic.

Also, ReError should never bug IMO. It already carries an ErrorGuaranteed, and since the compilation session is doomed, it should be handled everywhere. We've already had more than a few ICEs filed by @matthiaskrgr since ReError was introduced that have been due to matches that were bug!()ing on ReError.

rust-log-analyzer · 2023-04-14T17:34:02Z

The job x86_64-gnu-tools failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)


---- compile_test stdout ----
diff of stderr:

 error: the following explicit lifetimes could be elided: 'a, 'b
   --> $DIR/needless_lifetimes.rs:18:1
    |
 LL | fn distinct_lifetimes<'a, 'b>(_x: &'a u8, _y: &'b u8, _z: u8) {}
    |
    |
    = note: `-D clippy::needless-lifetimes` implied by `-D warnings`
 help: elide the lifetimes
    |
 LL - fn distinct_lifetimes<'a, 'b>(_x: &'a u8, _y: &'b u8, _z: u8) {}
 LL + fn distinct_lifetimes(_x: &u8, _y: &u8, _z: u8) {}
 
 
 error: the following explicit lifetimes could be elided: 'a, 'b
   --> $DIR/needless_lifetimes.rs:20:1
    |
    |
 LL | fn distinct_and_static<'a, 'b>(_x: &'a u8, _y: &'b u8, _z: &'static u8) {}
    |
 help: elide the lifetimes
    |
    |
 LL - fn distinct_and_static<'a, 'b>(_x: &'a u8, _y: &'b u8, _z: &'static u8) {}
 LL + fn distinct_and_static(_x: &u8, _y: &u8, _z: &'static u8) {}
 
 
 error: the following explicit lifetimes could be elided: 'a
   --> $DIR/needless_lifetimes.rs:30:1
    |
 LL | fn in_and_out<'a>(x: &'a u8, _y: u8) -> &'a u8 {
    |
 help: elide the lifetimes
    |
    |
 LL - fn in_and_out<'a>(x: &'a u8, _y: u8) -> &'a u8 {
 LL + fn in_and_out(x: &u8, _y: u8) -> &u8 {
 
 
 error: the following explicit lifetimes could be elided: 'b
   --> $DIR/needless_lifetimes.rs:42:1
    |
 LL | fn multiple_in_and_out_2a<'a, 'b>(x: &'a u8, _y: &'b u8) -> &'a u8 {
    |
 help: elide the lifetimes
    |
    |
 LL - fn multiple_in_and_out_2a<'a, 'b>(x: &'a u8, _y: &'b u8) -> &'a u8 {
 LL + fn multiple_in_and_out_2a<'a>(x: &'a u8, _y: &u8) -> &'a u8 {
 
 
 error: the following explicit lifetimes could be elided: 'a
   --> $DIR/needless_lifetimes.rs:49:1
    |
 LL | fn multiple_in_and_out_2b<'a, 'b>(_x: &'a u8, y: &'b u8) -> &'b u8 {
    |
 help: elide the lifetimes
    |
    |
 LL - fn multiple_in_and_out_2b<'a, 'b>(_x: &'a u8, y: &'b u8) -> &'b u8 {
 LL + fn multiple_in_and_out_2b<'b>(_x: &u8, y: &'b u8) -> &'b u8 {
 
 
 error: the following explicit lifetimes could be elided: 'b
   --> $DIR/needless_lifetimes.rs:66:1
    |
 LL | fn deep_reference_1a<'a, 'b>(x: &'a u8, _y: &'b u8) -> Result<&'a u8, ()> {
    |
 help: elide the lifetimes
    |
    |
 LL - fn deep_reference_1a<'a, 'b>(x: &'a u8, _y: &'b u8) -> Result<&'a u8, ()> {
 LL + fn deep_reference_1a<'a>(x: &'a u8, _y: &u8) -> Result<&'a u8, ()> {
 
 
 error: the following explicit lifetimes could be elided: 'a
   --> $DIR/needless_lifetimes.rs:73:1
    |
 LL | fn deep_reference_1b<'a, 'b>(_x: &'a u8, y: &'b u8) -> Result<&'b u8, ()> {
    |
 help: elide the lifetimes
    |
    |
 LL - fn deep_reference_1b<'a, 'b>(_x: &'a u8, y: &'b u8) -> Result<&'b u8, ()> {
 LL + fn deep_reference_1b<'b>(_x: &u8, y: &'b u8) -> Result<&'b u8, ()> {
 
 
 error: the following explicit lifetimes could be elided: 'a
   --> $DIR/needless_lifetimes.rs:82:1
    |
 LL | fn deep_reference_3<'a>(x: &'a u8, _y: u8) -> Result<&'a u8, ()> {
    |
 help: elide the lifetimes
    |
    |
 LL - fn deep_reference_3<'a>(x: &'a u8, _y: u8) -> Result<&'a u8, ()> {
 LL + fn deep_reference_3(x: &u8, _y: u8) -> Result<&u8, ()> {
 
 
 error: the following explicit lifetimes could be elided: 'a
   --> $DIR/needless_lifetimes.rs:87:1
    |
 LL | fn where_clause_without_lt<'a, T>(x: &'a u8, _y: u8) -> Result<&'a u8, ()>
    |
 help: elide the lifetimes
    |
    |
 LL - fn where_clause_without_lt<'a, T>(x: &'a u8, _y: u8) -> Result<&'a u8, ()>
 LL + fn where_clause_without_lt<T>(x: &u8, _y: u8) -> Result<&u8, ()>
 
 
 error: the following explicit lifetimes could be elided: 'a, 'b
   --> $DIR/needless_lifetimes.rs:99:1
    |
 LL | fn lifetime_param_2<'a, 'b>(_x: Ref<'a>, _y: &'b u8) {}
    |
 help: elide the lifetimes
    |
    |
 LL - fn lifetime_param_2<'a, 'b>(_x: Ref<'a>, _y: &'b u8) {}
 LL + fn lifetime_param_2(_x: Ref<'_>, _y: &u8) {}
 
 
 error: the following explicit lifetimes could be elided: 'a
   --> $DIR/needless_lifetimes.rs:123:1
    |
 LL | fn fn_bound_2<'a, F, I>(_m: Lt<'a, I>, _f: F) -> Lt<'a, I>
    |
 help: elide the lifetimes
    |
    |
 LL - fn fn_bound_2<'a, F, I>(_m: Lt<'a, I>, _f: F) -> Lt<'a, I>
 LL + fn fn_bound_2<F, I>(_m: Lt<'_, I>, _f: F) -> Lt<'_, I>
 
 
 error: the following explicit lifetimes could be elided: 's
   --> $DIR/needless_lifetimes.rs:153:5
    |
 LL |     fn self_and_out<'s>(&'s self) -> &'s u8 {
    |
 help: elide the lifetimes
    |
    |
 LL -     fn self_and_out<'s>(&'s self) -> &'s u8 {
 LL +     fn self_and_out(&self) -> &u8 {
 
 
 error: the following explicit lifetimes could be elided: 't
   --> $DIR/needless_lifetimes.rs:160:5
    |
 LL |     fn self_and_in_out_1<'s, 't>(&'s self, _x: &'t u8) -> &'s u8 {
    |
 help: elide the lifetimes
    |
    |
 LL -     fn self_and_in_out_1<'s, 't>(&'s self, _x: &'t u8) -> &'s u8 {
 LL +     fn self_and_in_out_1<'s>(&'s self, _x: &u8) -> &'s u8 {
 
 
 error: the following explicit lifetimes could be elided: 's
   --> $DIR/needless_lifetimes.rs:167:5
    |
 LL |     fn self_and_in_out_2<'s, 't>(&'s self, x: &'t u8) -> &'t u8 {
    |
 help: elide the lifetimes
    |
    |
 LL -     fn self_and_in_out_2<'s, 't>(&'s self, x: &'t u8) -> &'t u8 {
 LL +     fn self_and_in_out_2<'t>(&self, x: &'t u8) -> &'t u8 {
 
 
 error: the following explicit lifetimes could be elided: 's, 't
   --> $DIR/needless_lifetimes.rs:171:5
    |
 LL |     fn distinct_self_and_in<'s, 't>(&'s self, _x: &'t u8) {}
    |
 help: elide the lifetimes
    |
    |
 LL -     fn distinct_self_and_in<'s, 't>(&'s self, _x: &'t u8) {}
 LL +     fn distinct_self_and_in(&self, _x: &u8) {}
 
 
 error: the following explicit lifetimes could be elided: 'a
   --> $DIR/needless_lifetimes.rs:190:1
    |
 LL | fn struct_with_lt<'a>(_foo: Foo<'a>) -> &'a str {
    |
 help: elide the lifetimes
    |
    |
 LL - fn struct_with_lt<'a>(_foo: Foo<'a>) -> &'a str {
 LL + fn struct_with_lt(_foo: Foo<'_>) -> &str {
 
 
 error: the following explicit lifetimes could be elided: 'b
   --> $DIR/needless_lifetimes.rs:208:1
    |
 LL | fn struct_with_lt4a<'a, 'b>(_foo: &'a Foo<'b>) -> &'a str {
    |
 help: elide the lifetimes
    |
    |
 LL - fn struct_with_lt4a<'a, 'b>(_foo: &'a Foo<'b>) -> &'a str {
 LL + fn struct_with_lt4a<'a>(_foo: &'a Foo<'_>) -> &'a str {
 
 
 error: the following explicit lifetimes could be elided: 'a
   --> $DIR/needless_lifetimes.rs:216:1
    |
 LL | fn struct_with_lt4b<'a, 'b>(_foo: &'a Foo<'b>) -> &'b str {
    |
 help: elide the lifetimes
    |
    |
 LL - fn struct_with_lt4b<'a, 'b>(_foo: &'a Foo<'b>) -> &'b str {
 LL + fn struct_with_lt4b<'b>(_foo: &Foo<'b>) -> &'b str {
 
 
 error: the following explicit lifetimes could be elided: 'a
   --> $DIR/needless_lifetimes.rs:231:1
    |
 LL | fn trait_obj_elided2<'a>(_arg: &'a dyn Drop) -> &'a str {
    |
 help: elide the lifetimes
    |
    |
 LL - fn trait_obj_elided2<'a>(_arg: &'a dyn Drop) -> &'a str {
 LL + fn trait_obj_elided2(_arg: &dyn Drop) -> &str {
 
 
 error: the following explicit lifetimes could be elided: 'a
   --> $DIR/needless_lifetimes.rs:237:1
    |
 LL | fn alias_with_lt<'a>(_foo: FooAlias<'a>) -> &'a str {
    |
 help: elide the lifetimes
    |
    |
 LL - fn alias_with_lt<'a>(_foo: FooAlias<'a>) -> &'a str {
 LL + fn alias_with_lt(_foo: FooAlias<'_>) -> &str {
 
 
 error: the following explicit lifetimes could be elided: 'b
   --> $DIR/needless_lifetimes.rs:255:1
    |
 LL | fn alias_with_lt4a<'a, 'b>(_foo: &'a FooAlias<'b>) -> &'a str {
    |
 help: elide the lifetimes
    |
    |
 LL - fn alias_with_lt4a<'a, 'b>(_foo: &'a FooAlias<'b>) -> &'a str {
 LL + fn alias_with_lt4a<'a>(_foo: &'a FooAlias<'_>) -> &'a str {
 
 
 error: the following explicit lifetimes could be elided: 'a
   --> $DIR/needless_lifetimes.rs:263:1
    |
 LL | fn alias_with_lt4b<'a, 'b>(_foo: &'a FooAlias<'b>) -> &'b str {
    |
 help: elide the lifetimes
    |
    |
 LL - fn alias_with_lt4b<'a, 'b>(_foo: &'a FooAlias<'b>) -> &'b str {
 LL + fn alias_with_lt4b<'b>(_foo: &FooAlias<'b>) -> &'b str {
 
 
 error: the following explicit lifetimes could be elided: 'a
   --> $DIR/needless_lifetimes.rs:267:1
    |
 LL | fn named_input_elided_output<'a>(_arg: &'a str) -> &str {
    |
 help: elide the lifetimes
    |
    |
 LL - fn named_input_elided_output<'a>(_arg: &'a str) -> &str {
 LL + fn named_input_elided_output(_arg: &str) -> &str {
 
 
 error: the following explicit lifetimes could be elided: 'a
   --> $DIR/needless_lifetimes.rs:275:1
    |
 LL | fn trait_bound_ok<'a, T: WithLifetime<'static>>(_: &'a u8, _: T) {
    |
 help: elide the lifetimes
    |
    |
 LL - fn trait_bound_ok<'a, T: WithLifetime<'static>>(_: &'a u8, _: T) {
 LL + fn trait_bound_ok<T: WithLifetime<'static>>(_: &u8, _: T) {
 
 
 error: the following explicit lifetimes could be elided: 'a
   --> $DIR/needless_lifetimes.rs:311:1
    |
 LL | fn out_return_type_lts<'a>(e: &'a str) -> Cow<'a> {
    |
 help: elide the lifetimes
    |
    |
 LL - fn out_return_type_lts<'a>(e: &'a str) -> Cow<'a> {
 LL + fn out_return_type_lts(e: &str) -> Cow<'_> {
 
 
 error: the following explicit lifetimes could be elided: 'a
   --> $DIR/needless_lifetimes.rs:318:9
    |
 LL |         fn needless_lt<'a>(x: &'a u8) {}
    |
 help: elide the lifetimes
    |
    |
 LL -         fn needless_lt<'a>(x: &'a u8) {}
 LL +         fn needless_lt(x: &u8) {}
 
 
 error: the following explicit lifetimes could be elided: 'a
   --> $DIR/needless_lifetimes.rs:322:9
    |
 LL |         fn needless_lt<'a>(_x: &'a u8) {}
    |
 help: elide the lifetimes
    |
    |
 LL -         fn needless_lt<'a>(_x: &'a u8) {}
 LL +         fn needless_lt(_x: &u8) {}
 
 
 error: the following explicit lifetimes could be elided: 'a
   --> $DIR/needless_lifetimes.rs:335:9
    |
 LL |         fn baz<'a>(&'a self) -> impl Foo + 'a {
    |
 help: elide the lifetimes
    |
    |
 LL -         fn baz<'a>(&'a self) -> impl Foo + 'a {
 LL +         fn baz(&self) -> impl Foo + '_ {
 
 
 error: the following explicit lifetimes could be elided: 'a
   --> $DIR/needless_lifetimes.rs:367:5
    |
 LL |     fn impl_trait_elidable_nested_anonymous_lifetimes<'a>(i: &'a i32, f: impl Fn(&i32) -> &i32) -> &'a i32 {
    |
 help: elide the lifetimes
    |
    |
 LL -     fn impl_trait_elidable_nested_anonymous_lifetimes<'a>(i: &'a i32, f: impl Fn(&i32) -> &i32) -> &'a i32 {
 LL +     fn impl_trait_elidable_nested_anonymous_lifetimes(i: &i32, f: impl Fn(&i32) -> &i32) -> &i32 {
 
 
 error: the following explicit lifetimes could be elided: 'a
   --> $DIR/needless_lifetimes.rs:376:5
    |
 LL |     fn generics_elidable<'a, T: Fn(&i32) -> &i32>(i: &'a i32, f: T) -> &'a i32 {
    |
 help: elide the lifetimes
    |
    |
 LL -     fn generics_elidable<'a, T: Fn(&i32) -> &i32>(i: &'a i32, f: T) -> &'a i32 {
 LL +     fn generics_elidable<T: Fn(&i32) -> &i32>(i: &i32, f: T) -> &i32 {
 
 
 error: the following explicit lifetimes could be elided: 'a
   --> $DIR/needless_lifetimes.rs:388:5
    |
 LL |     fn where_clause_elidadable<'a, T>(i: &'a i32, f: T) -> &'a i32
    |
 help: elide the lifetimes
    |
    |
 LL -     fn where_clause_elidadable<'a, T>(i: &'a i32, f: T) -> &'a i32
 LL +     fn where_clause_elidadable<T>(i: &i32, f: T) -> &i32
 
 
 error: the following explicit lifetimes could be elided: 'a
   --> $DIR/needless_lifetimes.rs:403:5
    |
 LL |     fn pointer_fn_elidable<'a>(i: &'a i32, f: fn(&i32) -> &i32) -> &'a i32 {
    |
 help: elide the lifetimes
    |
    |
 LL -     fn pointer_fn_elidable<'a>(i: &'a i32, f: fn(&i32) -> &i32) -> &'a i32 {
 LL +     fn pointer_fn_elidable(i: &i32, f: fn(&i32) -> &i32) -> &i32 {
 
 
 error: the following explicit lifetimes could be elided: 'a
   --> $DIR/needless_lifetimes.rs:416:5
    |
 LL |     fn nested_fn_pointer_3<'a>(_: &'a i32) -> fn(fn(&i32) -> &i32) -> i32 {
    |
 help: elide the lifetimes
    |
    |
 LL -     fn nested_fn_pointer_3<'a>(_: &'a i32) -> fn(fn(&i32) -> &i32) -> i32 {
 LL +     fn nested_fn_pointer_3(_: &i32) -> fn(fn(&i32) -> &i32) -> i32 {
 
 
 error: the following explicit lifetimes could be elided: 'a
   --> $DIR/needless_lifetimes.rs:419:5
    |
 LL |     fn nested_fn_pointer_4<'a>(_: &'a i32) -> impl Fn(fn(&i32)) {
    |
 help: elide the lifetimes
    |
    |
 LL -     fn nested_fn_pointer_4<'a>(_: &'a i32) -> impl Fn(fn(&i32)) {
 LL +     fn nested_fn_pointer_4(_: &i32) -> impl Fn(fn(&i32)) {
 
 
 error: the following explicit lifetimes could be elided: 'a
   --> $DIR/needless_lifetimes.rs:441:9
    |
 LL |         fn implicit<'a>(&'a self) -> &'a () {
    |
 help: elide the lifetimes
    |
    |
 LL -         fn implicit<'a>(&'a self) -> &'a () {
 LL +         fn implicit(&self) -> &() {
 
 
 error: the following explicit lifetimes could be elided: 'a
   --> $DIR/needless_lifetimes.rs:444:9
    |
 LL |         fn implicit_mut<'a>(&'a mut self) -> &'a () {
    |
 help: elide the lifetimes
    |
    |
 LL -         fn implicit_mut<'a>(&'a mut self) -> &'a () {
 LL +         fn implicit_mut(&mut self) -> &() {
 
 
 error: the following explicit lifetimes could be elided: 'a
   --> $DIR/needless_lifetimes.rs:455:9
    |
 LL |         fn lifetime_elsewhere<'a>(self: Box<Self>, here: &'a ()) -> &'a () {
    |
 help: elide the lifetimes
    |
    |
 LL -         fn lifetime_elsewhere<'a>(self: Box<Self>, here: &'a ()) -> &'a () {
 LL +         fn lifetime_elsewhere(self: Box<Self>, here: &()) -> &() {
 
 
 error: the following explicit lifetimes could be elided: 'a
   --> $DIR/needless_lifetimes.rs:461:9
    |
 LL |         fn implicit<'a>(&'a self) -> &'a ();
    |
 help: elide the lifetimes
    |
    |
 LL -         fn implicit<'a>(&'a self) -> &'a ();
 LL +         fn implicit(&self) -> &();
 
 
 error: the following explicit lifetimes could be elided: 'a
   --> $DIR/needless_lifetimes.rs:462:9
    |
 LL |         fn implicit_provided<'a>(&'a self) -> &'a () {
    |
 help: elide the lifetimes
    |
    |
 LL -         fn implicit_provided<'a>(&'a self) -> &'a () {
 LL +         fn implicit_provided(&self) -> &() {
 
 
 error: the following explicit lifetimes could be elided: 'a
   --> $DIR/needless_lifetimes.rs:471:9
    |
 LL |         fn lifetime_elsewhere<'a>(self: Box<Self>, here: &'a ()) -> &'a ();
    |
 help: elide the lifetimes
    |
    |
 LL -         fn lifetime_elsewhere<'a>(self: Box<Self>, here: &'a ()) -> &'a ();
 LL +         fn lifetime_elsewhere(self: Box<Self>, here: &()) -> &();
 
 
 error: the following explicit lifetimes could be elided: 'a
   --> $DIR/needless_lifetimes.rs:472:9
    |
 LL |         fn lifetime_elsewhere_provided<'a>(self: Box<Self>, here: &'a ()) -> &'a () {
    |
 help: elide the lifetimes
    |
    |
 LL -         fn lifetime_elsewhere_provided<'a>(self: Box<Self>, here: &'a ()) -> &'a () {
 LL +         fn lifetime_elsewhere_provided(self: Box<Self>, here: &()) -> &() {
 
 
 error: the following explicit lifetimes could be elided: 'a
   --> $DIR/needless_lifetimes.rs:481:5

compiler-errors

Left a bunch of examples of ICEs due to the lack of handling of ReError. Left some thoughts about some of the other assertions.

I didn't touch borrowck, maybe someone like @aliemjay can leave thoughts on those.

compiler-errors · 2023-04-14T18:23:44Z

src/librustdoc/clean/auto_trait.rs

-        .unwrap_or_else(|| r.super_fold_with(self))
+        match *r {
+            ty::ReVar(vid) => self.vid_to_region.get(&vid).cloned().unwrap_or(r),
+            ty::ReEarlyBound(_) | ty::ReStatic | ty::ReLateBound(..) => r,


This is probably fine. If I were being defensive, I'd also include ReError here, but whatever.

Some comment to the effect of "these are the regions that can be seen in AST" as justification.

compiler-errors · 2023-04-14T18:26:47Z

compiler/rustc_ty_utils/src/ty.rs

-                        );
+                match re.kind() {
+                    ty::ReEarlyBound(_) => re,
+                    ty::ReLateBound(index, bv) => {


This ICEs on ReEarlyBound:

#![feature(return_position_impl_trait_in_trait)] use std::ops::Deref; trait Foo { fn test<'a: 'a>() -> impl Deref<Target = impl Sized> { &() } } fn main() {}

Actually, that error is coming from somewhere else in borrowck.

compiler/rustc_traits/src/chalk/lowering.rs

compiler-errors · 2023-04-14T18:34:00Z

compiler/rustc_traits/src/chalk/lowering.rs

@@ -1141,8 +1142,9 @@ impl<'tcx> TypeFolder<TyCtxt<'tcx>> for ParamsSubstitutor<'tcx> {
                    self.tcx.mk_re_late_bound(self.binder_index, br)
                }
            },
-
-            _ => r.super_fold_with(self),
+            // Other region kinds might need the same treatment here.


Canonicalization already canonicalizes all free regions for chalk. We probably shouldn't even have this folder at all.

Unrelated, but this ICEs on nightly already, lol:

// compile-flags: -Ztrait-solver=chalk trait Trait {} impl Trait for &() {} fn trait_pred() where &'missing (): Trait {} fn main() { trait_pred(); }

compiler-errors · 2023-04-14T18:39:09Z

compiler/rustc_traits/src/chalk/lowering.rs

@@ -1045,7 +1045,8 @@ impl<'a, 'tcx> TypeFolder<TyCtxt<'tcx>> for NamedBoundVarSubstitutor<'a, 'tcx> {
                ty::BrEnv => unimplemented!(),
                ty::BrAnon(..) => {}
            },
-            _ => (),
+            ty::ReLateBound(..) | ty::ReStatic => (),
+            r => bug!("unexpected region: {r:?}"),


This probably ICEs on ReError if we do something like:

fn def<'a, T>(_: T) where T: Fn() {} fn foo() where &'missing i32: Copy {} fn main() { def(foo); }

But that's obscured by other canonicalizer+ReError bugs in chalk.

compiler-errors · 2023-04-14T19:48:09Z

compiler/rustc_hir_analysis/src/collect.rs

@@ -387,8 +387,10 @@ impl<'tcx> AstConv<'tcx> for ItemCtxt<'tcx> {

    fn ct_infer(&self, ty: Ty<'tcx>, _: Option<&ty::GenericParamDef>, span: Span) -> Const<'tcx> {
        let ty = self.tcx.fold_regions(ty, |r, _| match *r {
-            ty::ReErased => self.tcx.lifetimes.re_static,
-            _ => r,
+            // This is never reached in practice. If it ever is reached,


We do not hit this in practice, I checked. But as far as I know, ReErased is the only region we should see... anyways, this is fine to keep here I guess.

compiler-errors · 2023-04-14T19:56:13Z

compiler/rustc_hir_analysis/src/check/wfcheck.rs

-                    re
+            let opaque_ty = tcx.fold_regions(unshifted_opaque_ty, |re, _depth| {
+                match re.kind() {
+                    ty::ReEarlyBound(_) | ty::ReFree(_) => re,


Probably need to support ReError here, but actually, but I guess I never noticed that all the late-bound lifetimes are liberated here, so that's good news I guess. Simplifes the logic a bit.

compiler-errors · 2023-04-14T19:57:07Z

compiler/rustc_hir_analysis/src/check/compare_impl_item.rs

-        } else {
-            r
+            ),
+            ty::ReEarlyBound(_) | ty::ReStatic => r,


Probably need to support ReError here, but after staring at this code for the last hour or so I am starting to get exhausted providing examples 😅

compiler-errors · 2023-04-14T19:57:14Z

compiler/rustc_hir_analysis/src/check/compare_impl_item.rs

-                        _ => return region,
+                        ty::ReEarlyBound(_) |
+                        ty::ReStatic => return region,
+                        r => bug!("unexpected region: {r:?}"),


ReError here

compiler-errors · 2023-04-14T19:57:42Z

compiler/rustc_hir_analysis/src/astconv/mod.rs

-                let ty = tcx.fold_regions(ty_erased, |r, _| {
-                    if r.is_erased() { tcx.lifetimes.re_static } else { r }
+                let ty = tcx.fold_regions(ty_erased, |r, _| match r.kind() {
+                    ty::ReErased => tcx.lifetimes.re_static,


I think ReErased is the only kind we expect here.

compiler-errors · 2023-04-19T03:11:21Z

Not sure if you want to work on this further or split it out into other PRs, but marking this as waiting-on-author for now.

@rustbot author

nnethercote · 2023-04-19T04:12:53Z

Not sure if you want to work on this further or split it out into other PRs, but marking this as waiting-on-author for now.

One or the other, but I have some other stuff I need to finish in the meantime. I'll get back to this eventually. Thank you for the detailed comments.

nnethercote · 2023-04-26T00:46:11Z

I just created #110835, which is a massively cut down version of this, including only the changes that @compiler-errors was positive about, which wasn't many. While this was an interesting experiment, making large scale changes doesn't seem like a good idea.

nnethercote added 2 commits April 14, 2023 15:59

Rewrite RegionReplacer::fold_region.

66314e2

Make all region folders strict about which variants they accept.

043c1de

rustbot assigned oli-obk Apr 14, 2023

compiler-errors self-assigned this Apr 14, 2023

compiler-errors requested changes Apr 14, 2023

View reviewed changes

rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Apr 19, 2023

nnethercote closed this Apr 26, 2023

nnethercote deleted the strict-region-folders branch April 26, 2023 07:04

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Strict region folders #110312

Strict region folders #110312

nnethercote commented Apr 14, 2023

rustbot commented Apr 14, 2023

compiler-errors commented Apr 14, 2023

bors commented Apr 14, 2023

jackh726 commented Apr 14, 2023

jackh726 commented Apr 14, 2023

compiler-errors commented Apr 14, 2023

compiler-errors commented Apr 14, 2023

rust-log-analyzer commented Apr 14, 2023

compiler-errors left a comment

compiler-errors Apr 14, 2023

compiler-errors Apr 14, 2023 •

edited

Loading

compiler-errors Apr 14, 2023 •

edited

Loading

compiler-errors Apr 14, 2023

compiler-errors Apr 14, 2023

compiler-errors Apr 14, 2023

compiler-errors Apr 14, 2023

compiler-errors Apr 14, 2023

compiler-errors Apr 14, 2023

compiler-errors Apr 14, 2023

compiler-errors commented Apr 19, 2023

nnethercote commented Apr 19, 2023

nnethercote commented Apr 26, 2023

Strict region folders #110312

Strict region folders #110312

Conversation

nnethercote commented Apr 14, 2023

rustbot commented Apr 14, 2023

compiler-errors commented Apr 14, 2023

bors commented Apr 14, 2023

jackh726 commented Apr 14, 2023

jackh726 commented Apr 14, 2023

compiler-errors commented Apr 14, 2023

compiler-errors commented Apr 14, 2023

rust-log-analyzer commented Apr 14, 2023

compiler-errors left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

compiler-errors Apr 14, 2023 • edited Loading

Choose a reason for hiding this comment

compiler-errors Apr 14, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

compiler-errors commented Apr 19, 2023

nnethercote commented Apr 19, 2023

nnethercote commented Apr 26, 2023

compiler-errors Apr 14, 2023 •

edited

Loading

compiler-errors Apr 14, 2023 •

edited

Loading