# the `let _ = x` problem rfc 2229

Basic example:

fn foo() {
    let x = 3;
    let c = || {
        let _ = x;
    };
}

* If we don't see a read or other use of `x` (which we don't, in this example), then the capture list will not contain `x` or any place that starts with `x`.
* Backwards compatibility question around `move` closures but that is a separate concern.
* Challenge:
* When we build the THIR, we need to translate `let _ = x` into something
* normally we translate upvars like `x` into expressions that reference closure fields (e.g., `*self.x` in this case)
* but there is no field and there is no expression we can generate that represents `x` in this example
* fundamentally what should [this function](https://github.com/rust-lang/rust/blob/f7801d6c7cc19ab22bdebcc8efa894a564c53469/compiler/rustc_mir_build/src/thir/cx/expr.rs#L888-L892) return if there is no corresponding upvar?
* similar problem [in the liveness code](https://github.com/rust-lang/rust/pull/78762/commits/b4851fcd17aa3d3fa73210df7cd42fc25aca15de#diff-ee61ee4a6309983c53c63a6d9d0ab7904e5c6443a3fa543869b9d260e0b971aaR1355-R1367), but in that context, just skipping the access seems ok

Looking a bit forward, in the simple minimal capture case:

let x = (3, 4);
let c = || drop(x.0);
    // Today:    {x: &(u32, u32)}
    // Tomorrow: {x_0: &u32}

* We expect to have a field corresponding to `x.0` here
* We can't translate the HIR expression `x` into a THIR expression on its own
* we can only translate `x.0` into a THIR expression (`*self.x_0`)

Maybe refactor `convert_var` to return an enum:

enum ConvertedExpr<'tcx> {
    Expr(Expr<'tcx>),
    NonexistentUpvar,
}

* `convert_var` and friends would return this enum
* the `to_ref` that generates an `Expr<'tcx>` would assert that it got a `ConvertedExpr::Expr`
* the handling of let ([link](https://github.com/rust-lang/rust/blob/master/compiler/rustc_mir_build/src/thir/cx/block.rs#L93)) would be the only place we accept a `NonexistentUpvar` result without ICEing
* and we would assert that the pattern has no bindings
* and probably just generate "no statement" or maybe a no-op statement of some kind
* we can just [not push into the vector](https://github.com/rust-lang/rust/blob/master/compiler/rustc_mir_build/src/thir/cx/block.rs#L85)

----

Later we may refactor the enum to include "partial path" (XXX it looks like we will have to do this in MIR)

* we converted `x` but we only have a field for `x.0` -- we need to return some partial information so that when we convert the `.0` HIR node we can create the `self.x_0` THIR expression

the Hir for `x.0` is going to look like:

* ExprKind::Field ([code that handles this today](https://github.com/rust-lang/rust/blob/master/compiler/rustc_mir_build/src/thir/cx/expr.rs#L546-L549))
* source: ExprKind::Path for `x`
* invokes `convert_var` (today, returns an `Expr`)
* tomorrow: return a ExprKind::Partial(`x`)
* today: wraps in a ExprKind::Field
* tomorrow:
* if we got back a partial path, and adding `x.0` makes a complete upvar match,
* then return `self.x_0` reference to the field (same logic as `convert_var` uses today)
*

today:

        hir::ExprKind::Field(ref source, ..) => ExprKind::Field {
            lhs: source.to_ref(),
            name: Field::new(cx.tcx.field_index(expr.hir_id, cx.typeck_results)),
        },

tomorrow:

match source.to_expr_blah_blah_blah() {
    ConvertedExpr::Expr(source_expr) => {
        hir::ExprKind::Field(ref source, ..) => ExprKind::Field {
            lhs: source_expr,
            name: Field::new(cx.tcx.field_index(expr.hir_id, cx.typeck_results)),
        }
    }
    
    ConvertedExpr::NonexistentUpvar =>
        return ConvertedExpr::NonexistentUpvar,
        
    ConvertedExpr::PartialPath(place) => {
        let place1 = place with field projection; // `x.0` in our example
        if /* there is a complete upvar `f` for place1` */ {
            return /* expression for self.f, appropriately deref'd */ ;
        } else {
            return PartialPath(place1);
        }
    }
}

fn foo() {
    let x = ((1, 2), 3);
    let c = || {
        x.0.0 + x.1
    };
}

two fields in our closure:

* `x_0_0`
* `x_1`

our capture table is something like this:

* [`x.0.0`: ByRef, `x.1`: ByRef]

when we are converting `x.0.0` that is HIR like

* Field (`0`)
* Field (`0`)
* Path (`x`)
* return `PartialPath(x)`
* return `PartialPath(x.0)`
* construct `*self.x_0_0` and return that

Another interesting example:

fn foo() {
    let x = ((1, 2), 3);
    let c = || {
        drop(x.0);
        x.0.0 + x.1
    };
}

two fields in our closure:

* `x_0`
* `x_1`
* [`x.0`: ByRef, `x.1`: ByRef]

how to handle `x.0.0`

* Field (`0`)
* Field (`0`)
* Path (`x`)
* return `PartialPath(x)`
* return `*self.x_0`
* return `*self.x_0.0` (like normal)

But what about...

let x = (1, 2, 3);
|| {
    let (y, z, _) = x; // capture x.0, x.1
}

* We can't generate the THIR here because:
* translating `x` yields a `PartialPath`
* but we need to generate a `let` like `(y, z) = translation(x)`

Possible solutions:

* Mess up the language by calling this a read of `x`
* pro: easy
* con: affects users
* Extend THIR with "partial path expressions" and push the upvar->field conversation into MIR construction
* seems like the only viable path
* Extend THIR with ability to desugar let/match statements
* I believe exhaustiveness checking operates on THIR

----

MIR:

* Constructing the closure
* Accessing upvars from within the closure

Assume:

* Extend THIR with a `ExprKind::UpvarRef(HirId)` node
* remove `ExprKind::SelfRef` if this is only used with closures

Example:

let x = (((1, 2), 3), 4);
let c = || ... drop(x.0.1.0) ...; // captures `x.0.1`

THIR:

* N0: Field(1)
* N1: Field(0)
* N2: UpvarRef(`x`)

MIR construction:

* If `UpvarRef` were a normal local...
* `expr_as_place` [doc](https://github.com/rust-lang/rust/blob/7f5a42b073dc2bee2aa625052eb066ee07072048/compiler/rustc_mir_build/src/build/expr/as_place.rs#L122-L129)
* so long as we are building places, stays as a `PlaceBuilder`
* [`into_place`](https://doc.rust-lang.org/nightly/nightly-rustc/rustc_mir_build/build/expr/as_place/struct.PlaceBuilder.html#method.into_place) eventually called
* here is how it works (for `x.0`):
* call `expr_as_place` on N0
* recursively calls `expr_as_place` on N1
* recursively calls `expr_as_place` on N2
* returns new `PlaceBuilder` ``[x, []]`
* appends to placehlder `[x, [0]]` and returns
* appends to placehlder `[x, [0, 1]]` and returns
* But it's not, it's an upvar...
* placebuilder's `local` field becomes "place start" or something
* `enum { Local(Local), Upvar(HirId) }`
* call stack
* call `expr_as_place` on N0
* recursively calls `expr_as_place` on N1
* recursively calls `expr_as_place` on N2
* recursively calls `expr_as_place` on N3
* returns new `PlaceBuilder` ``[Upvar(x), []]`
* appends to placeholder `[Upvar(x), [0]]` and returns
* observe that `[Upvar(x), [0, 1]]` is in our capture set
* let's call it a by-ref capture
* return a new placebuilder with
* `[Local(1) /*self*/, [`
* `Field(0) /*0th capture*/`
* `Deref`
* `]`
* `*self.x`
* (as above)
* when you call `into_place`:
* it's a `span_bug` if this is not yet a MIR place

what about matches:

match x { // captured is x.1
    (_, y) => ...
}

* because there is a "synthetic read" in "expr use visitor" this shouldn't matter
* but we should revisit this later, open a FIXME issue
* the match builder will want to be refactored to use `PlaceBuilder` or some enum that allows us to express "HIR vs MIR"

possible approach that might be more compatible with match builder:

enum OrHIRPlace<T> {
    HIRPlace(...),
    MIRPlace(T),
}

impl<T> OrHIRPlace<T> {
    ...
}

// OrHirPlace<PlaceBuilder<'tcx>>
//
// in the matches code replace `Place<'tcx>` with:
//
// OrHirPlace<Place<'tcx>>

sketch in more detail:

* `expr_as_place(...) -> BlockAnd<PlaceBuilder<'tcx>>` (today)
* original approach: change `PlaceBuilder` to "maybe" be a HIR place
* alternative approach: change return type of `expr_as_place` to `BlockAnd<OrHIRPlace<PlaceBuilder<'tcx>>`

refactoring steps:

* extend HIR with `UpvarRef` and remove the code that generates `self.x` etc for upvars
* extend MIR builder to convert `UpvarRef` into MIR places
* this is possible because there is still a 1-to-1 correspondence between upvars and closure fields
* extend `PlaceBuilder` to covert upvars (but never use it)
* plain refactoring to modify placebuilder (a commit)
* actually use it
* modify closure construction etc
* fix the closure borrow checker diagnostics

this is actually a great stopping point, remaining steps:

* fixme for patterns (better stopping point)

polish-y things:

* migration code
* optimization and measurements