Point at fields that make the type recursive #40857

estebank · 2017-03-27T16:43:32Z

On recursive types of infinite size, point at all the fields that make
the type recursive.

struct Foo {
    bar: Bar,
}

struct Bar {
    foo: Foo,
}

outputs

error[E0072]: recursive type `Foo` has infinite size
 --> file.rs:1:1
1 | struct Foo {
  | ^^^^^^^^^^ recursive type has infinite size
2 |     bar: Bar,
  |     -------- recursive here
  |
  = help: insert indirection (e.g., a `Box`, `Rc`, or `&`) at some point to make `Foo` representable

error[E0072]: recursive type `Bar` has infinite size
 --> file.rs:5:1
  |
5 | struct Bar {
  | ^^^^^^^^^^ recursive type has infinite size
6 |     foo: Foo,
  |     -------- recursive here
  |
  = help: insert indirection (e.g., a `Box`, `Rc`, or `&`) at some point to make `Bar` representable

rust-highfive · 2017-03-27T16:43:42Z

r? @eddyb

(rust_highfive has picked a reviewer for you, use r? to override)

estebank · 2017-03-27T16:48:22Z

Needs code clean up and differentiating between fields that have indirections and the ones that don't have indirection (and are therefore the culprit for the error), but I finally got it working last night so I'm putting it out there as an RFC on the approach and copy of the messages.

CC @jonathandturner, @GuillaumeGomez, @nagisa, @nikomatsakis.

eddyb · 2017-03-27T16:55:36Z

r? @jonathandturner

estebank · 2017-03-27T20:01:58Z

One of the things I want to confirm is on the following:

 +error[E0072]: recursive type `Foo` has infinite size
 +  --> $DIR/recursive-type-field.rs:13:1
 +   |
 +13 |   struct Foo<'a> {
 +   |  _^ starting here...
 +14 | |     bar: Bar<'a>,
 +   | |     ------------ recursive without indirection
 +15 | |     b: Rc<Bar<'a>>,
 +   | |     -------------- SHOULD WE MARK THAT IT IS RECURSIVE HERE, EVEN THOUGH IT IS NOT PART OF THE PROBLEM?
 +16 | | }
 +   | |_^ ...ending here: recursive type has infinite size
 +   |
 +   = help: insert indirection (e.g., a `Box`, `Rc`, or `&`) at some point to make `Foo` representable

estebank · 2017-03-28T04:41:44Z

Current output:

error[E0072]: recursive type `Bar` has infinite size
  --> $DIR/recursive-type-field.rs:18:1
   |
18 |   struct Bar<'a> {
   |  _^ starting here...
19 | |     y: (Foo<'a>, Foo<'a>),
   | |     --------------------- recursive without indirection
20 | |     z: Option<Bar<'a>>,
21 | |     a: &'a Foo<'a>,
22 | |     c: &'a [Bar<'a>],
23 | |     d: [Bar<'a>; 1],
   | |     --------------- recursive without indirection
24 | |     e: Foo<'a>,
   | |     ---------- recursive without indirection
25 | |     x: Bar<'a>,
   | |     ---------- recursive without indirection
26 | | }
   | |_^ ...ending here: recursive type has infinite size
   |
   = help: insert indirection (e.g., a `Box`, `Rc`, or `&`) at some point to make `Bar` representable

GuillaumeGomez · 2017-03-28T15:31:58Z

Wo, that's awesome!

sophiajt · 2017-03-29T00:11:00Z

I like the idea, that's very cool.

Ideally, we only mark the areas that are part of the problem. I can't speak to the code, but the design of the error I think is good.

estebank · 2017-04-02T15:36:21Z

Ping @jonathandturner.

sophiajt · 2017-04-03T19:43:35Z

@estebank - I already reviewed design, but you'll need to ping someone on the compiler team for code review. Maybe @arielb1 or @nikomatsakis

estebank · 2017-04-03T23:28:59Z

@jonathandturner sorry, as you were set as reviewer I assumed that you'd come back later to review the code. Rereading what you wrote you said exactly what you meant to say :)

arielb1 · 2017-04-04T19:49:23Z

I'll look at it tomorrow.

estebank · 2017-04-04T22:53:16Z

@arielb1 thanks!

arielb1 · 2017-04-06T15:10:15Z

Why are you walking the HIR instead of Ty information? (aka AdtDef, FieldDef)? You could rewrite is_representable to return a backtrace.

arielb1 · 2017-04-16T13:06:55Z

src/librustc/ty/util.rs

-                    find_nonrepresentable(tcx,
-                                          sp,
+                    let spans = def.all_fields().map(|f| {
+                        tcx.hir.span_if_local(f.did).unwrap_or(DUMMY_SP)


Do we need the DUMMY_SPs? Can you use Option<Span>?

IMO we should stop using span_if_local because tcx.def_span(f.did) works but we still need a solution for printing cross-crate spans - and I can't find the issue for it.

arielb1 · 2017-04-16T13:09:33Z

src/librustc/ty/util.rs

                                          seen,
-                                          def.all_fields().map(|f| f.ty(tcx, substs)))
+                                          def.all_fields().map(|f| (f.ty(tcx, substs), tcx.hir.span_if_local(f.did).unwrap_or(DUMMY_SP)))) {
+                        Representability::SelfRecursive(_) => Representability::SelfRecursive(spans),


Doesn't this destroy the info on what is the appropriate field?

Yeah, the last commit is mostly me trying to modify the logic here as little as possible while adding spans. It seems to me that I'll have to take some time to rewrite this to properly keep a stack.

arielb1 · 2017-04-16T13:10:54Z

Tidy error:

[00:03:37] tidy error: /checkout/src/librustc/ty/util.rs:722: line longer than 100 chars
[00:03:37] tidy error: /checkout/src/librustc/ty/util.rs:723: line longer than 100 chars

carols10cents · 2017-04-24T14:55:14Z

Friendly ping to keep this on your radar @estebank!

estebank · 2017-04-24T17:37:24Z

@carols10cents I'll be coming back to this, but it'll probably won't be until next weekend :-/

arielb1 · 2017-04-25T09:43:56Z

Friendly ping to keep this on your radar @estebank!

carols10cents · 2017-04-25T13:13:01Z

I just pinged this PR yesterday and estebank responded 17 hours before your ping @arielb1 :)

sophiajt · 2017-05-03T01:30:46Z

@estebank - any luck fixing the tidy errors on this one?

On recursive types of infinite size, point at all the fields that make the type recursive. ```rust struct Foo { bar: Bar, } struct Bar { foo: Foo, } ``` outputs ``` error[E0072]: recursive type `Foo` has infinite size --> file.rs:1:1 1 | struct Foo { | _^ starting here... 2 | | bar: Bar, | | -------- recursive here 3 | | } | |_^ ...ending here: recursive type has infinite size | = help: insert indirection (e.g., a `Box`, `Rc`, or `&`) at some point to make `Foo` representable error[E0072]: recursive type `Bar` has infinite size --> file.rs:5:1 | 5 | struct Bar { | _^ starting here... 6 | | foo: Foo, | | -------- recursive here 7 | | } | |_^ ...ending here: recursive type has infinite size | = help: insert indirection (e.g., a `Box`, `Rc`, or `&`) at some point to make `Bar` representable ```

* clean up code * point only fields that cause the type to be of infinite size * fix unittests

estebank · 2017-05-06T07:57:13Z

Updated.

arielb1 · 2017-05-07T13:57:31Z

That's perfect.

@bors r+

bors · 2017-05-07T13:57:32Z

📌 Commit a4fc925 has been approved by arielb1

bors · 2017-05-07T13:57:41Z

⌛ Testing commit a4fc925 with merge a478e46...

Point at fields that make the type recursive On recursive types of infinite size, point at all the fields that make the type recursive. ```rust struct Foo { bar: Bar, } struct Bar { foo: Foo, } ``` outputs ``` error[E0072]: recursive type `Foo` has infinite size --> file.rs:1:1 1 | struct Foo { | ^^^^^^^^^^ recursive type has infinite size 2 | bar: Bar, | -------- recursive here | = help: insert indirection (e.g., a `Box`, `Rc`, or `&`) at some point to make `Foo` representable error[E0072]: recursive type `Bar` has infinite size --> file.rs:5:1 | 5 | struct Bar { | ^^^^^^^^^^ recursive type has infinite size 6 | foo: Foo, | -------- recursive here | = help: insert indirection (e.g., a `Box`, `Rc`, or `&`) at some point to make `Bar` representable ```

bors · 2017-05-07T16:20:09Z

☀️ Test successful - status-appveyor, status-travis
Approved by: arielb1
Pushing a478e46 to master...

rust-highfive assigned eddyb Mar 27, 2017

rust-highfive assigned sophiajt and unassigned eddyb Mar 27, 2017

estebank force-pushed the recursive branch 2 times, most recently from d426564 to 02619ec Compare March 28, 2017 03:00

arielb1 self-assigned this Apr 4, 2017

Mark-Simulacrum added the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Apr 14, 2017

arielb1 reviewed Apr 16, 2017

View reviewed changes

estebank added 2 commits May 4, 2017 19:11

Only point at the fields that cause infinite size

f7108e1

* clean up code * point only fields that cause the type to be of infinite size * fix unittests

estebank force-pushed the recursive branch 4 times, most recently from 349fe7c to 234c721 Compare May 6, 2017 02:10

Move logic to is_representable instead of climbing HIR

a4fc925

estebank force-pushed the recursive branch from 234c721 to a4fc925 Compare May 6, 2017 04:58

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

bors merged commit a4fc925 into rust-lang:master May 7, 2017

estebank deleted the recursive branch November 9, 2023 05:26

Point at fields that make the type recursive #40857

Point at fields that make the type recursive #40857

Uh oh!

Conversation

estebank commented Mar 27, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rust-highfive commented Mar 27, 2017

Uh oh!

estebank commented Mar 27, 2017

Uh oh!

eddyb commented Mar 27, 2017

Uh oh!

estebank commented Mar 27, 2017

Uh oh!

estebank commented Mar 28, 2017

Uh oh!

GuillaumeGomez commented Mar 28, 2017

Uh oh!

sophiajt commented Mar 29, 2017

Uh oh!

estebank commented Apr 2, 2017

Uh oh!

sophiajt commented Apr 3, 2017

Uh oh!

estebank commented Apr 3, 2017

Uh oh!

arielb1 commented Apr 4, 2017

Uh oh!

estebank commented Apr 4, 2017

Uh oh!

arielb1 commented Apr 6, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

arielb1 Apr 16, 2017

Choose a reason for hiding this comment

Uh oh!

eddyb Apr 16, 2017

Choose a reason for hiding this comment

Uh oh!

arielb1 Apr 16, 2017

Choose a reason for hiding this comment

Uh oh!

estebank Apr 18, 2017

Choose a reason for hiding this comment

Uh oh!

arielb1 commented Apr 16, 2017

Uh oh!

carols10cents commented Apr 24, 2017

Uh oh!

estebank commented Apr 24, 2017

Uh oh!

arielb1 commented Apr 25, 2017

Uh oh!

carols10cents commented Apr 25, 2017

Uh oh!

sophiajt commented May 3, 2017

Uh oh!

estebank commented May 6, 2017

Uh oh!

arielb1 commented May 7, 2017

Uh oh!

bors commented May 7, 2017

Uh oh!

bors commented May 7, 2017

Uh oh!

bors commented May 7, 2017

Uh oh!

Uh oh!

estebank commented Mar 27, 2017 •

edited

Loading

arielb1 commented Apr 6, 2017 •

edited

Loading