Skip to content

Implementation Issue for RFC 2528: type-changing struct update syntax #86555  #86618

Description

@nikomatsakis

This issue tracks the implementation of rust-lang/rfcs#2528, "type changing struct update syntax" (general tracking issue: #86555).

Activity

  1. nikomatsakis commented on Jun 26, 2021

    @nikomatsakis
    ContributorAuthor

    @rustbot assign @kolharsam

    They expressed an interest in this!

  2. self-assigned this
    on Jun 26, 2021
  3. kolharsam commented on Jun 26, 2021

    @kolharsam

    @rustbot claim

  4. nikomatsakis commented on Jun 26, 2021

    @nikomatsakis
    ContributorAuthor

    Here are some mentoring notes.

    PR 0: Create the feature gate

    We need a feature gate to control when users are accessing this feature. Instructions for adding a feature gate are here:

    https://rustc-dev-guide.rust-lang.org/feature-gates.html#adding-a-feature-gate

    Adding a feature gate can and should be its own PR!

  5. nikomatsakis commented on Jun 26, 2021

    @nikomatsakis
    ContributorAuthor

    PR 1: Write up some tests

    A good next step is to open a PR creating a directory in src/tests/ui for this feature and adding the various tests. It's ok if they get errors today! Just document the current behavior. But I've always found it's actually much easier to write tests BEFORE you start the code then after!

  6. added
    E-mentorCall for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion.
    T-compilerRelevant to the compiler team, which will review and decide on the PR/issue.
    on Jun 26, 2021
  7. nikomatsakis commented on Jun 26, 2021

    @nikomatsakis
    ContributorAuthor

    PR 2: Update the type checker

    First off, Foo { ... } expressions are type checked by this function:

    https://github.com/rust-lang/rust/blob/50cc3de2ac37969080e5afb3cada6977eba20533/compiler/rustc_typeck/src/check/expr.rs#L1114-L1122

    We create this type adt_ty that represents the resulting type of the expression. It will embed inference variables for each of the generics on the struct (e.g., Foo<?0>), unless they were specified (e.g., the user wrote Foo::<u32>):

    https://github.com/rust-lang/rust/blob/50cc3de2ac37969080e5afb3cada6977eba20533/compiler/rustc_typeck/src/check/expr.rs#L1124-L1125

    The following code checks each of the fields provided by the user, ensuring their names match and so forth. This code doesn't have to change for the most part:

    https://github.com/rust-lang/rust/blob/50cc3de2ac37969080e5afb3cada6977eba20533/compiler/rustc_typeck/src/check/expr.rs#L1140-L1148

    One thing we might want to do though is to return or otherwise get access to the list of 'remaining fields' (those whose types were not specified by the user):

    https://github.com/rust-lang/rust/blob/50cc3de2ac37969080e5afb3cada6977eba20533/compiler/rustc_typeck/src/check/expr.rs#L1211-L1216

    After checking the types of the fields, we check if there is a "base expression" (Foo { ..foo }):

    https://github.com/rust-lang/rust/blob/50cc3de2ac37969080e5afb3cada6977eba20533/compiler/rustc_typeck/src/check/expr.rs#L1149-L1150

    If so, we check that it has the type adt_ty. This is the code that has to change, because it is where you get an error if the types don't line up!

    https://github.com/rust-lang/rust/blob/50cc3de2ac37969080e5afb3cada6977eba20533/compiler/rustc_typeck/src/check/expr.rs#L1154-L1155

    What we want to be know is that:

    • For each field field that comes from the base expression
    • It has some type source_field_ty in that base expression
    • That field also has some type target_field_ty in the struct we are creating
    • The type source_field_ty from the base expression must be a subtype of target_field_ty

    Right now, we don't have any explicit "field-by-field" code, beacuse when we check that the type of the base expression is equal to adt_ty, we check that the types of all the fields in the base expression are compatible. But that is too strong.

    However, we do have code that computes the various source_field_ty types:

    https://github.com/rust-lang/rust/blob/50cc3de2ac37969080e5afb3cada6977eba20533/compiler/rustc_typeck/src/check/expr.rs#L1157-L1167

    FRU here stands for "functional record update", btw. Anyway, these types today are stored into the TypeckResults struct and are not used further by this phase of type-checking. Instead, they are used by the MIR construction code as we desugar this code into MIR. No changes to MIR should be necessary for this work, though.

    Because this is not backwards compatible, we have to make our changes conditional on the feature gate being enabled. If it is not enabled, the code should do the same as it does today. But it is IS enabled, we should remove this check and instead enforce that:

    • The base type is an ADT with the same def-id as the one we are creating.
    • Go through each of the fields in the ADT:
      • If the field is NOT in the "remaining fields" set, you can ignore it, as it was given a value by the user.
      • Otherwise, get the type from fru_fields (this is source_field_ty in my explanation above)
      • Compute the type in the target ADT (we do this in the existing code here, so we could store and remember those types)
      • Check that they are a subtype

    The final check ("that they are a subtype") is done by calling infcx.at(..).sup(..), you should be able to find some examples of code that does that.

  8. kolharsam commented on Jun 26, 2021

    @kolharsam

    Here are some mentoring notes.

    PR 0: Create the feature gate

    We need a feature gate to control when users are accessing this feature. Instructions for adding a feature gate are here:

    https://rustc-dev-guide.rust-lang.org/feature-gates.html#adding-a-feature-gate

    Adding a feature gate can and should be its own PR!

    @nikomatsakis Have made a respective PR for the same here: #86646

  9. kolharsam commented on Jun 27, 2021

    @kolharsam

    PR 1: Write up some tests

    A good next step is to open a PR creating a directory in src/tests/ui for this feature and adding the various tests. It's ok if they get errors today! Just document the current behavior. But I've always found it's actually much easier to write tests BEFORE you start the code then after!

    @nikomatsakis I've made a PR with a failing test here: #86660.
    Currently, it just document's the error that might be seen while using the suggested updates to the syntax in within the RFC.

    Please let me know if there are any other test cases that need to be added because I wasn't really sure about what else could've been added as a test.

  10. added 3 commits that reference this issue on Oct 22, 2021
  11. added a commit that references this issue on Nov 9, 2021
  12. Systemcluster commented on Jan 26, 2022

    @Systemcluster

    The current implementation seems to interfere with with type inference:

    #![feature(type_changing_struct_update)]
    
    #[derive(Default)]
    struct A {
        x: i32
    }
    
    fn main() {
        let a = A{
            ..Default::default()
        };
    }
    error[E0308]: mismatched types
      --> src/main.rs:10:11
       |
    10 |         ..Default::default()
       |           ^^^^^^^^^^^^^^^^^^ expected struct `A`, found inferred type
       |
       = note: expected struct `A`
                    found type `_`
    

    Playground

  13. nikomatsakis commented on Jan 31, 2022

    @nikomatsakis
    ContributorAuthor

    I don't believe we've implemented this feature yet =) there are mentoring notes here. I know that someone was looking into it, maybe it was @kolharsam? (If I failed to get back to you, @kolharsam, sorry, are you still interested?)

  14. kolharsam commented on Feb 1, 2022

    @kolharsam

    @nikomatsakis Yes, I was looking into it, and I'm still interested in continuing the work.

  15. Kobzol commented on Feb 2, 2022

    @Kobzol
    Member

    The feature has actually been implemented in #90035, if I'm not mistaken. It already seems to work: https://play.rust-lang.org/?version=nightly&mode=debug&edition=2021&gist=a84582797c0337611b94e3c5afaeba91.
    @Systemcluster could you please create a separate issue with your code snippet? It looks like a bug in the implementation of the feature.

  16. Mark-Simulacrum commented on Jul 27, 2022

    @Mark-Simulacrum
    Member

    We think this is implemented now, so closing this issue. #86555 is still tracking the feature.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

A-type-systemArea: Type systemE-mentorCall for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion.F-type_changing_struct_update`#![feature(type_changing_struct_update)]`T-compilerRelevant to the compiler team, which will review and decide on the PR/issue.

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions