Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RFC: Tuple struct construction with Self(v1, v2, ..) #2302

Merged
merged 9 commits into from
Jul 2, 2018

Conversation

Centril
Copy link
Contributor

@Centril Centril commented Jan 18, 2018

Rendered

Tracking issue


Tuple structs can now be constructed and pattern matched with Self(v1, v2, ..). A simple example:

struct TheAnswer(usize);

impl Default for TheAnswer {
    fn default() -> Self { Self(42) }
}

Similarly, unit structs can also be constructed and pattern matched with Self.

@Centril Centril added the T-lang Relevant to the language team, which will review and decide on the RFC. label Jan 18, 2018
@Centril Centril changed the title Rfc/tuple struct self ctor RFC: Tuple struct construction with Self(v1, v2, ..) Jan 18, 2018
@Emerentius
Copy link

The proposal unearths another unexpected limitation. A type alias for a tuple struct can not be used with tuple struct init syntax right now (without Self).

struct FooBar(u8);

type BarFoo = FooBar;

impl Default for BarFoo {
    fn default() -> Self {
        // Self(42) // proposed in rfc
        BarFoo(42) // does not compile
                   // expected function, found type alias `BarFoo`
        // BarFoo { 0: 42 }  // works
    }
}

@Centril
Copy link
Contributor Author

Centril commented Jan 18, 2018

The proposal unearths another unexpected limitation. A type alias for a tuple struct can not be used with tuple struct init syntax right now (without Self).

I think that limitation isn't possible to get around without bc-breaks tho simply because BarFoo is allowed to be a function name. In my opinion it was a mistake to allow lower cased type names (which translates into constructors and the need for explicitly quantified type variables in generics) and upper cased function names and variables... But we have to live with this now. I also think an epoch may be unfeasible since it would affect too much code.

EDIT: perhaps the break wouldn't be all that extensive so it could be lumped into the next epoch..?

Personally I think we should improve what we can here =)

@burdges
Copy link

burdges commented Jan 18, 2018

I think epochs could and should be used to deprecate any existing harmful name collisions, like BarFoo being both a type and an unrelated function in the same scope. Yes that sounds orthogonal to Self(..)

@Centril
Copy link
Contributor Author

Centril commented Jan 18, 2018

Linking RFC #1506.

impl Default for TheAnswer {
fn default() -> Self { Self(42) }
}
```
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If tuple structs are supported, then unit structs can be supported in the same way:

struct TheAnswer;

impl Default for TheAnswer {
    fn default() -> Self { Self }
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I considered it initially but didn't for some reason I can't remember. I can add that to the RFC if you like.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are three forms of instantiation (to use the word slightly fuzzily): Self { }, Self() and Self. Self { } already works. You’ve proposed adding Self() for consistency. It doesn’t seem to me that it fixes consistency properly unless Self is also made to work.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@chris-morgan Sure, I'll change the RFC accordingly Soon™.

@petrochenkov
Copy link
Contributor

```

the compiler will desugar the application by substituting `Self(v0, v1, ..)`
for `Self { 0: v0, 1: v1, .. }` and then continue on from there. The compiler
Copy link
Contributor

@petrochenkov petrochenkov Jan 20, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This desugaring is not entirely equivalent to what constructors of tuple struct can do. In

struct S(u8);

S in value namespace is a constructor function and it can be used in all contexts where a fn(u8) -> S { ... } is accepted, not only direct calls S(arg).

(S)(0u8); // OK
opt_u8.map(S); // OK
(Self)(0u8); // Not OK, not a direct call
opt_u8.map(Self); // Not OK, not a direct call

Ideally, Self should be a first-class "function alias" and refer to the constructor function (with appropriate generic parameter substitutions).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Self should be a first-class "function alias"

Or a "const alias" for unit structs. "Value alias", in general.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's strictly more powerful I guess, so I could change the desugaring to go in this direction if we want to support that. Do we want to support that? Why / why not?

@petrochenkov
Copy link
Contributor

By analogy with #2300 (comment), it's interesting to list what Self in value namespace could mean for various items.

  • extern crate, use, mod, extern block, global asm!, macro, macro_rules!. Self value doesn't make sense.
  • impl. Constructor of the type for which we implement methods/consts (if it exists), as this RFC proposes.
  • enum, struct, union. Constructor of the item's type (if it exists)
  • trait, trait alias. Nothing, traits don't have constructors and we don't know if the type parameter Self has a constructor.
  • type. Nothing, or constructor of the item's type (if it exists).
  • fn. Self can refer to the function itself (e.g. for recursion).
  • static, const. Self can refer to the const/static itself (infinite recursion).
  • type, fn, const in impls. As this RFC proposes, Self has meaning dictated by the outer impl. There are possible alternative meanings inherited from "free" type, fn and const items.
  • type, fn, static,const in traits and extern blocks. Meaning of Self can be inherited from "free" type, fn and static items.

Inconsistencies:

  • Self in type/fn/const in impls is potentially inconsistent with Self in free type/fn/const items.

Future compatibility:

  • Name resolution "barriers" for Self values still make sense for forward-compatibility, in the same way as for Self types.

@withoutboats
Copy link
Contributor

@rfcbot fcp merge

@rfcbot
Copy link
Collaborator

rfcbot commented Jan 25, 2018

Team member @withoutboats has proposed to merge this. The next step is review by the rest of the tagged teams:

No concerns currently listed.

Once a majority of reviewers approve (and none object), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

@rfcbot rfcbot added the proposed-final-comment-period Currently awaiting signoff of all team members in order to enter the final comment period. label Jan 25, 2018
@nikomatsakis
Copy link
Contributor

I'm happy as long as we think this is backwards compatible and works.

(@petrochenkov -- do you?)

@petrochenkov
Copy link
Contributor

I'm not 100% sure that Self value should see through type aliases, i.e. that the resolution should be type-based (as opposed to name-based).

Currently a struct and its constructor are almost entirely independent items, i.e. you can turn a tuple struct into a braced struct + a separate fn item if necessary (modulo patterns).
This RFC ties them more closely.

With name-based resolution you can potentially resolve Self to an independent function with the same name in the same scope, but how do you "link" a function to a type at type level without adding some novel language construction?

On the other hand, maybe making structs and their constructors "separable" is not an important goal.

@withoutboats
Copy link
Contributor

withoutboats commented Jan 26, 2018

This RFC ties them more closely.

This is interesting. You're saying that right now going from this:

pub struct Foo(pub u8);

To this:

pub struct Foo { bar: u8 }
pub fn Foo(bar: u8) -> Foo { Foo { bar } }

Would not be a breaking change.

But this RFC would make it a breaking change because anyone who used the constructor as Self would be broken?

@scottmcm
Copy link
Member

scottmcm commented Jan 26, 2018

Would not be a breaking change.

Well, it's still a breaking change because let Foo(blah) = foo;, so this RFC just adds another way.

@petrochenkov
Copy link
Contributor

petrochenkov commented Jan 27, 2018

@scottmcm
Yeah, as I said, modulo patterns.
However, I'm pretty sure the pattern breakage issue is fixable with const functions.
This, for example, already work

#![feature(const_fn)]

const fn Foo() -> u8 {
    10
}

const C: u8 = Foo();

fn main() {
    match 10u8 {
        C => println!("match!"),
        _ => println!("not match!"),
    }
}

, we just need to accept non-tuple-struct calls directly in pattern positions.

The question is whether we should go in this direction, or "separability" of structs and their constructors is not an important property.

@cramertj
Copy link
Member

cramertj commented Jan 27, 2018

@petrochenkov How do const functions help there? Wouldn't we need to be able to solve const functions backwards for that to work?

e.g. let Foo(x) = Foo(8); would have to "unapply" Foo, right?

@eddyb
Copy link
Member

eddyb commented Jan 27, 2018

Sounds like "pattern synonyms" / "pattern aliases" / "custom patterns".
const fn could be used for this purpose, with some kind of opt-in which exposes the body.
cc @Centril

@withoutboats
Copy link
Contributor

If its not a non-breaking change today then I'm not very concerned about this RFC changing that personally.

@petrochenkov
Copy link
Contributor

@cramertj

e.g. let Foo(x) = Foo(8); would have to "unapply" Foo, right?

No, it would substitute the body of Foo into the pattern roughly in the same way like constants in patterns work today.

@eddyb
Copy link
Member

eddyb commented Jan 27, 2018

@petrochenkov I don't think that's the case after miri integration (cc @oli-obk).
However, we could do a similar transformation based on MIR, as a form of "symbolic interpretation".

@petrochenkov
Copy link
Contributor

Ok, I guess if we solve the much bigger pattern problem, we'll be able to come up with some solution for Self as well.
Concern retracted.

@nrc
Copy link
Member

nrc commented Jan 29, 2018

Similarly to #2300, I have a concern here that Self is over-used and recommended where perhaps it shouldn't be, and I'd like to see some discussion of that before we permit more uses of Self in the language.

@Centril
Copy link
Contributor Author

Centril commented Jan 29, 2018

@nrc Let's have that discussion in one place over at #2300 tho ;)

@rfcbot rfcbot added the final-comment-period Will be merged/postponed/closed in ~10 calendar days unless new substational objections are raised. label Feb 16, 2018
@nikomatsakis
Copy link
Contributor

Is there a reason we can't project Self into the value space? (Forgive me if I'm repeating stuff from the RFC itself.)

@aturon
Copy link
Member

aturon commented Apr 5, 2018

@rfcbot fcp merge

The companion RFC has been merged, having addressed @nrc's concerns. I believe this should be merged as well.

@rfcbot
Copy link
Collaborator

rfcbot commented Apr 5, 2018

Team member @aturon has proposed to merge this. The next step is review by the rest of the tagged teams:

No concerns currently listed.

Once a majority of reviewers approve (and none object), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

@rfcbot rfcbot added the proposed-final-comment-period Currently awaiting signoff of all team members in order to enter the final comment period. label Apr 5, 2018
@petrochenkov
Copy link
Contributor

petrochenkov commented Apr 5, 2018

@nikomatsakis

Is there a reason we can't project Self into the value space? (Forgive me if I'm repeating stuff from the RFC itself.)

I think this is what we should do (#2302 (comment)).
I've only now noticed that the RFC text is not updated to use Self in value namespace and still uses desugaring Self(...) => Self { ... }, so we don't have Self for unit structs and Self as a fn item pointer for tuple structs.

@Centril
Copy link
Contributor Author

Centril commented Apr 8, 2018

@petrochenkov So iirc the consensus from the last lang team meeting was that permitting Self for unit structs and foo.map(Self) would also be nice.

As you are more familiar with the alternate proposed desugaring would you be willing to update the RFC with a PR against my branch?

(Note: as written, the RFC also supports match foo { Self(x, y) => .. } } and type aliases we should keep it that way imo).

Centril added 4 commits June 21, 2018 05:47
…nit structs, change reference explanation, etc. + consequence changes.
Fix RFC 2302 to support unit structs, fn pointers, and use value namespace instead for exprs
@Centril
Copy link
Contributor Author

Centril commented Jun 21, 2018

Patched the RFC to support unit structs and use the value namespace (Self can use used as an fn pointer as a consequence...).

```
pat : ... // <-- The original grammar of `pat` prior to this RFC.
| SELF '(' ')'
| SELF '(' pat_tup ')'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The grammar doesn't change, technically, it's still PATH ( ... ) and Self is already a valid path.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So the entire ## Grammar section can be removed then?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Aight; removed it :)

@petrochenkov
Copy link
Contributor

LGTM.

@rfcbot
Copy link
Collaborator

rfcbot commented Jun 21, 2018

🔔 This is now entering its final comment period, as per the review above. 🔔

@rfcbot rfcbot added final-comment-period Will be merged/postponed/closed in ~10 calendar days unless new substational objections are raised. and removed proposed-final-comment-period Currently awaiting signoff of all team members in order to enter the final comment period. labels Jun 21, 2018
@rfcbot
Copy link
Collaborator

rfcbot commented Jul 1, 2018

The final comment period, with a disposition to merge, as per the review above, is now complete.

@rfcbot rfcbot added finished-final-comment-period The final comment period is finished for this RFC. and removed final-comment-period Will be merged/postponed/closed in ~10 calendar days unless new substational objections are raised. labels Jul 1, 2018
@Centril Centril merged commit d391d85 into rust-lang:master Jul 2, 2018
@Centril
Copy link
Contributor Author

Centril commented Jul 2, 2018

Huzzah! This RFC is merged!

Tracking issue: rust-lang/rust#51994

@Centril Centril deleted the rfc/tuple-struct-self-ctor branch July 2, 2018 17:38
@Centril Centril added A-syntax Syntax related proposals & ideas A-patterns Pattern matching related proposals & ideas A-expressions Term language related proposals & ideas A-resolve Proposals relating to name resolution. labels Nov 23, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-expressions Term language related proposals & ideas A-patterns Pattern matching related proposals & ideas A-resolve Proposals relating to name resolution. A-syntax Syntax related proposals & ideas finished-final-comment-period The final comment period is finished for this RFC. T-lang Relevant to the language team, which will review and decide on the RFC.
Projects
None yet
Development

Successfully merging this pull request may close these issues.