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

Store Type Information within projections #5

Closed
4 of 5 tasks
arora-aman opened this issue Jun 12, 2020 · 0 comments · Fixed by sexxi-goose/rust#18 or rust-lang/rust#73870
Closed
4 of 5 tasks

Store Type Information within projections #5

arora-aman opened this issue Jun 12, 2020 · 0 comments · Fixed by sexxi-goose/rust#18 or rust-lang/rust#73870
Assignees
@Azhng @roxelo
Labels
place-refactor
Projects
RFC-2229
Milestone
Feature complete

Comments

@arora-aman
Copy link
Member

arora-aman commented Jun 12, 2020 •
edited
Loading

Currently within hir::Place type information is only stored for the final value/memory being read.

Type information at each intermediate access can be useful later and computing this information again at each Projection could be complicated and costly.

The final structure is explained here:
https://github.com/rust-lang/project-rfc-2229/blob/master/hir-place-target.md

Projection is defined here: https://github.com/rust-lang/rust/blob/master/src/librustc_typeck/mem_categorization.rs#L77

Based on discussion on Zulip we also want the following to close this issue

  • Remove current type from Place
  • Add Place::base_ty -- Type information of PlaceBase
  • Extending Projection with type information. Provide interface to read ty before and after the projection is applied.
  • Removing ProjectionKind::Deref 's type
  • Associating PlaceBase::Rvalue with expression hir_id ? (This one is optional for now and can be refactored later, if required.)
@arora-aman arora-aman added this to To-do in RFC-2229 Jun 12, 2020
@nikomatsakis nikomatsakis mentioned this issue Jun 16, 2020
Closed
@arora-aman arora-aman mentioned this issue Jun 18, 2020
Closed
arora-aman added a commit to sexxi-goose/rust that referenced this issue Jun 18, 2020
@arora-aman @roxelo
Refactor hir::Place
3659406 
For the following code
```rust
let c = || bar(foo.x, foo.x)
```

We generate two different `hir::Place`s for both `foo.x`.
Handling this adds overhead for analysis we need to do for RFC 2229.

We also want to store type information at each Projection to support
analysis as part of the RFC. This resembles what we have for
`mir::Place`

This commit modifies the Place as follows:
- Rename to `PlaceWithHirId`, where there `hir_id` is that of the
expressioin.
- Move any other information that describes the access out to another
struct now called `Place`.
- Removed `Span`, it can be accessed using the [hir
API](https://doc.rust-lang.org/nightly/nightly-rustc/rustc_middle/hir/map/struct.Map.html#method.span)
- Modify `Projection` to be a strucutre of its own, that currently only
contains the `ProjectionKind`.

Adding type information to projections wil be completed as part of rust-lang/project-rfc-2229#5

Closes rust-lang/project-rfc-2229#3

Co-authored-by: Aman Arora <me@aman-arora.com>
Co-authored-by: Roxane Fruytier <roxane.fruytier@hotmail.com>
@arora-aman arora-aman mentioned this issue Jun 19, 2020
Merged
Manishearth added a commit to Manishearth/rust that referenced this issue Jun 19, 2020
@Manishearth
Rollup merge of rust-lang#73489 - sexxi-goose:init_place_refactor, r=…
a88182f 
…nikomatsakis

Refactor hir::Place

For the following code
```rust
let c = || bar(foo.x, foo.x)
```

We generate two different `hir::Place`s for both `foo.x`.
Handling this adds overhead for analysis we need to do for RFC 2229.

We also want to store type information at each Projection to support
analysis as part of the RFC. This resembles what we have for
`mir::Place`

This commit modifies the Place as follows:
- Rename to `PlaceWithHirId`, where there `hir_id` is that of the
expressioin.
- Move any other information that describes the access out to another
struct now called `Place`.
- Removed `Span`, it can be accessed using the [hir
API](https://doc.rust-lang.org/nightly/nightly-rustc/rustc_middle/hir/map/struct.Map.html#method.span)
- Modify `Projection` to be a strucutre of its own, that currently only
contains the `ProjectionKind`.

Adding type information to projections wil be completed as part of rust-lang/project-rfc-2229#5

Closes rust-lang/project-rfc-2229#3
@arora-aman arora-aman moved this from To-do to In progress in RFC-2229 Jun 20, 2020
flip1995 pushed a commit to flip1995/rust-clippy that referenced this issue Jun 22, 2020
@arora-aman @roxelo
Refactor hir::Place
922ff8e 
For the following code
```rust
let c = || bar(foo.x, foo.x)
```

We generate two different `hir::Place`s for both `foo.x`.
Handling this adds overhead for analysis we need to do for RFC 2229.

We also want to store type information at each Projection to support
analysis as part of the RFC. This resembles what we have for
`mir::Place`

This commit modifies the Place as follows:
- Rename to `PlaceWithHirId`, where there `hir_id` is that of the
expressioin.
- Move any other information that describes the access out to another
struct now called `Place`.
- Removed `Span`, it can be accessed using the [hir
API](https://doc.rust-lang.org/nightly/nightly-rustc/rustc_middle/hir/map/struct.Map.html#method.span)
- Modify `Projection` to be a strucutre of its own, that currently only
contains the `ProjectionKind`.

Adding type information to projections wil be completed as part of rust-lang/project-rfc-2229#5

Closes rust-lang/project-rfc-2229#3

Co-authored-by: Aman Arora <me@aman-arora.com>
Co-authored-by: Roxane Fruytier <roxane.fruytier@hotmail.com>
@arora-aman arora-aman mentioned this issue Jun 25, 2020
Merged
@Azhng Azhng mentioned this issue Jun 25, 2020
Merged
@Azhng Azhng mentioned this issue Jun 29, 2020
Merged
@arora-aman arora-aman moved this from In progress to In review in RFC-2229 Jun 30, 2020
arora-aman pushed a commit to sexxi-goose/rust that referenced this issue Jul 6, 2020
@Azhng @arora-aman
typeck: adding type information to projection
fe2c6a3 
This commit modifies the Place as follow:
* remove 'ty' from ProjectionKind
* add type information into to Projection
* replace 'ty' in Place with 'base_ty'
* introduce 'ty()' in `Place` to return the final type of the `Place`
* introduce `ty_before_projection()` in `Place` to return the type of
  a `Place` before i'th projection is applied

Closes rust-lang/project-rfc-2229#5
Manishearth added a commit to Manishearth/rust that referenced this issue Jul 6, 2020
@Manishearth
Rollup merge of rust-lang#73870 - sexxi-goose:projection-ty, r=nikoma…
96a8f37 
…tsakis

typeck: adding type information to projection

This commit modifies the Place as follow:
* remove 'ty' from ProjectionKind
* add type information into to Projection
* replace 'ty' in Place with 'base_ty'
* introduce 'ty()' in `Place` to return the final type of the `Place`
* introduce `ty_before_projection()` in `Place` to return the type of
  a `Place` before i'th projection is applied

Closes rust-lang/project-rfc-2229#5
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this issue Jul 6, 2020
@Dylan-DPC
Rollup merge of rust-lang#73870 - sexxi-goose:projection-ty, r=nikoma…
4d24341 
…tsakis

typeck: adding type information to projection

This commit modifies the Place as follow:
* remove 'ty' from ProjectionKind
* add type information into to Projection
* replace 'ty' in Place with 'base_ty'
* introduce 'ty()' in `Place` to return the final type of the `Place`
* introduce `ty_before_projection()` in `Place` to return the type of
  a `Place` before i'th projection is applied

Closes rust-lang/project-rfc-2229#5
Manishearth added a commit to Manishearth/rust that referenced this issue Jul 7, 2020
@Manishearth
Rollup merge of rust-lang#73870 - sexxi-goose:projection-ty, r=nikoma…
7942d9a 
…tsakis

typeck: adding type information to projection

This commit modifies the Place as follow:
* remove 'ty' from ProjectionKind
* add type information into to Projection
* replace 'ty' in Place with 'base_ty'
* introduce 'ty()' in `Place` to return the final type of the `Place`
* introduce `ty_before_projection()` in `Place` to return the type of
  a `Place` before i'th projection is applied

Closes rust-lang/project-rfc-2229#5
@arora-aman arora-aman moved this from In review to Done in RFC-2229 Jul 7, 2020
flip1995 pushed a commit to flip1995/rust-clippy that referenced this issue Jul 8, 2020
@Azhng
typeck: adding type information to projection
dfecaef 
This commit modifies the Place as follow:
* remove 'ty' from ProjectionKind
* add type information into to Projection
* replace 'ty' in Place with 'base_ty'
* introduce 'ty()' in `Place` to return the final type of the `Place`
* introduce `ty_before_projection()` in `Place` to return the type of
  a `Place` before i'th projection is applied

Closes rust-lang/project-rfc-2229#5
flip1995 pushed a commit to flip1995/rust-clippy that referenced this issue Jul 8, 2020
@Manishearth
Rollup merge of #73870 - sexxi-goose:projection-ty, r=nikomatsakis
245b006 
typeck: adding type information to projection

This commit modifies the Place as follow:
* remove 'ty' from ProjectionKind
* add type information into to Projection
* replace 'ty' in Place with 'base_ty'
* introduce 'ty()' in `Place` to return the final type of the `Place`
* introduce `ty_before_projection()` in `Place` to return the type of
  a `Place` before i'th projection is applied

Closes rust-lang/project-rfc-2229#5
@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.
Assignees

@Azhng Azhng

@roxelo roxelo

Labels
place-refactor
Projects
RFC-2229
  
Done
Milestone
Feature complete
4 participants
@nikomatsakis @arora-aman @Azhng @roxelo