Skip to content
This repository has been archived by the owner on Apr 5, 2024. It is now read-only.

Make hir::Place projections more precise #1

Closed
6 tasks done
nikomatsakis opened this issue Jun 9, 2020 · 5 comments · Fixed by rust-lang/rust#74140
Closed
6 tasks done

Make hir::Place projections more precise #1

nikomatsakis opened this issue Jun 9, 2020 · 5 comments · Fixed by rust-lang/rust#74140

Comments

@nikomatsakis
Copy link
Contributor

nikomatsakis commented Jun 9, 2020

The current HIR Place data structure includes only two kinds of projections:

  • Deref
  • Other

If you compare that to the MIR Place, you can see that the MIR version has a lot more kinds of projections. I'm making a list of them below. We may not need to add them all to HIR Place, but we probably want most of them. For each one, there is a check-box, and in some cases I may link to additional issues that give more details about how to make a change.


Within the HIR we don't need to be as precise as the MIR.

  • Downcast is use for enum variants and in Hir we can combine it with field Field(u32, VariantIdx) where the u32 is for the field within a struct.
  • Since we are capturing the complete array even if a single element is captured we don't need to preserve index information and store that information as just Index.
  • ConstIndex is used for some special cases where we know the index is a fixed constant -- mostly pattern matching like let [a, b, _] = some_array. The compiler is (sometimes) smart enough to see that these moves are distinct from (say) let [_, _, c] = some_array, but to be that smart, it needs to know the indices as constants. We are just going to capture the entire array for now, so we don't care for now.
@nikomatsakis
Copy link
Contributor Author

Some details on how HIR Places are constructed. They are defined and built in the mem categorization module. This code takes a HIR Expression, which represents the syntax for an expression, and tries to create a HIR Place from it. The name "mem categorization" is somewhat old, but it comes from the idea that we are "categorizing" what "memory" the expression refers to (e.g., a local variable, or a field of some local variable -- this is precisely what we now call a "place").

Mem categorization doesn't just take the HIR Expression as input, it also takes some information produced by the type checker, which tells us e.g. the type of the expression along with some implicit things like derefs (that is, an expression like a.b, where a is a reference like &Foo is often expanded by the type-checker to be (*a).b). This extra state from the type checker is called the TypeckTables and stored in the tables field of the MemCategorizationCtontext.

As a starting point, consider the cat_expr function, which "categorizes" an expression. It begins by processing "adjustments", which are those expanded operations like implicit derefs. Eventually it "bottoms out" in the underlying expression, which is processed by cat_expr_unadjusted.

You can see that it looks at "what kind of expression is this" by a big match.

So, if we are going to introduce more precision, this is where we start. e.g., if we wanted to isolate out a Field, as in #2, we might look at the match arm for Field. More on that in a follow-up comment.

@nikomatsakis
Copy link
Contributor Author

Looking more closely at the code for Field, you can see that it calls a helper called cat_projection . This helper is invoked for all kinds of projections apart from derefs, and you can see that it works by

@matthewjasper
Copy link

There's also this match for patterns that will also need updating.

@nikomatsakis
Copy link
Contributor Author

So a first step might be to modify cat_projection to look like

    crate fn cat_projection<N: HirNode>(
        &self,
        node: &N,
        base_place: Place<'tcx>,
        ty: Ty<'tcx>,
        projection: Projection<'tcx>,
    ) -> Place<'tcx> {

so that we can adjust the calls one by one (e.g., the field could pass in a new field variant, etc).

@arora-aman arora-aman changed the title Refactoring HIR Place Make hir::Place projections more precise Jun 12, 2020
arora-aman added a commit to sexxi-goose/rust that referenced this issue Jul 7, 2020
This commit also categorizing access as Field, Index, or Subslice.

Ideas are taken from `mir::ProjectionElem`.

Proposed changes: https://github.com/rust-lang/project-rfc-2229/blob/master/hir-place-target.md

Closes: rust-lang/project-rfc-2229#1,
rust-lang/project-rfc-2229#2

Co-authored-by: Aman Arora <me@aman-arora.com>
Co-authored-by: Chris Pardy <chrispardy36@gmail.com>
Co-authored-by: Dhruv Jauhar <dhruvjhr@gmail.com>
arora-aman added a commit to sexxi-goose/rust that referenced this issue Jul 7, 2020
This commit also categorizing access as Field, Index, or Subslice.

Ideas are taken from `mir::ProjectionElem`.

Proposed changes: https://github.com/rust-lang/project-rfc-2229/blob/master/hir-place-target.md

Closes: rust-lang/project-rfc-2229#1,
rust-lang/project-rfc-2229#2

Co-authored-by: Aman Arora <me@aman-arora.com>
Co-authored-by: Chris Pardy <chrispardy36@gmail.com>
Co-authored-by: Dhruv Jauhar <dhruvjhr@gmail.com>
arora-aman added a commit to sexxi-goose/rust that referenced this issue Jul 8, 2020
This commit also categorizing access as Field, Index, or Subslice.

Ideas are taken from `mir::ProjectionElem`.

Proposed changes: https://github.com/rust-lang/project-rfc-2229/blob/master/hir-place-target.md

Closes: rust-lang/project-rfc-2229#1,
rust-lang/project-rfc-2229#2

Co-authored-by: Aman Arora <me@aman-arora.com>
Co-authored-by: Chris Pardy <chrispardy36@gmail.com>
Co-authored-by: Dhruv Jauhar <dhruvjhr@gmail.com>
arora-aman added a commit to sexxi-goose/rust that referenced this issue Jul 9, 2020
This commit also categorizing access as Field, Index, or Subslice.

Ideas are taken from `mir::ProjectionElem`.

Proposed changes: https://github.com/rust-lang/project-rfc-2229/blob/master/hir-place-target.md

Closes: rust-lang/project-rfc-2229#1,
rust-lang/project-rfc-2229#2

Co-authored-by: Aman Arora <me@aman-arora.com>
Co-authored-by: Chris Pardy <chrispardy36@gmail.com>
Co-authored-by: Dhruv Jauhar <dhruvjhr@gmail.com>
Manishearth added a commit to Manishearth/rust that referenced this issue Jul 16, 2020
…, r=nikomatsakis

Make hir ProjectionKind more precise

This commit also categorizing access as Field, Index, or Subslice.

Ideas are taken from `mir::ProjectionElem`.

Proposed changes: https://github.com/rust-lang/project-rfc-2229/blob/master/hir-place-target.md

Closes: rust-lang/project-rfc-2229#1,
Closes: rust-lang/project-rfc-2229#2

r? @nikomatsakis @matthewjasper
Manishearth added a commit to Manishearth/rust that referenced this issue Jul 16, 2020
…, r=nikomatsakis

Make hir ProjectionKind more precise

This commit also categorizing access as Field, Index, or Subslice.

Ideas are taken from `mir::ProjectionElem`.

Proposed changes: https://github.com/rust-lang/project-rfc-2229/blob/master/hir-place-target.md

Closes: rust-lang/project-rfc-2229#1,
Closes: rust-lang/project-rfc-2229#2

r? @nikomatsakis @matthewjasper
Manishearth added a commit to Manishearth/rust that referenced this issue Jul 16, 2020
…, r=nikomatsakis

Make hir ProjectionKind more precise

This commit also categorizing access as Field, Index, or Subslice.

Ideas are taken from `mir::ProjectionElem`.

Proposed changes: https://github.com/rust-lang/project-rfc-2229/blob/master/hir-place-target.md

Closes: rust-lang/project-rfc-2229#1,
Closes: rust-lang/project-rfc-2229#2

r? @nikomatsakis @matthewjasper
@arora-aman
Copy link
Member

via rust-lang/rust#74140

@nikomatsakis nikomatsakis added this to the Feature complete milestone Feb 17, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.