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

Empty struct with braces tracking issue (RFC 218) #24266

Closed
nikomatsakis opened this issue Apr 10, 2015 · 9 comments
Closed

Empty struct with braces tracking issue (RFC 218) #24266

nikomatsakis opened this issue Apr 10, 2015 · 9 comments
Assignees
Labels
B-RFC-approved Blocker: Approved by a merged RFC but not yet implemented. E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion. T-lang Relevant to the language team, which will review and decide on the PR/issue.

Comments

@nikomatsakis
Copy link
Contributor

Tracking issue for rust-lang/rfcs#218.

cc @pnkfelix.

Summary

When a struct type S has no fields (a so-called "empty struct"), allow it to be defined via either struct S; or struct S {}. When defined via struct S;, allow instances of it to be constructed and pattern-matched via either S or S {}. When defined via struct S {}, require instances to be constructed and pattern-matched solely via S {}.

RFC text

@steveklabnik steveklabnik added the B-RFC-approved Blocker: Approved by a merged RFC but not yet implemented. label Apr 10, 2015
@nrc nrc added E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion. labels Jul 10, 2015
@nrc nrc self-assigned this Jul 10, 2015
@Dineshs91
Copy link

@nrc I would like to work on this

@Dineshs91
Copy link

If we remove these conditions L2213 and L4705, wouldn't that be enough

@nrc

@nrc
Copy link
Member

nrc commented Jul 20, 2015

@Dineshs91 It looks like you should remove those two checks, yes. I'm not sure that that will be enough, it might be. You will need to add a test or two to check it works and make sure all the existing tests pass (you'll probably find that some tests which checked for those conditions no longer pass and need to be removed).

@pnkfelix
Copy link
Member

@Dineshs91 No, its not enough just to remove the two checks, at least not to satisfy all the requirements of the RFC

Such as small change might have been enough to satisfy very early drafts of the RFC, but the actual RFC text that ended up being accepted has an important extra condition: structs defined via struct S { } can only be constructed and pattern-matched via S {}, while structs defined via struct S; can be constructed and patterned matched via either S or S {}.

This means that to implement this correctly, the compiler needs to track which kind of definition was used to define the struct initially -- it ends up being part of the type of the struct itself.

(Also, it may be good to attempt to put this change in behind a feature-gate while we get the initial wrinkles worked out. That's another reason that removing checks does not suffice.)


(I have updated the issue description to include the summary text and a link to the RFC, so hopefully this point will not be overlooked in the future.)

@Dineshs91
Copy link

@pnkfelix Thanks for the clarification.

So there are two things that should be taken care of

  1. Struct defined via struct S; can be constructed and pattern matched via either S or S { }.
    This works perfectly fine.
  2. Struct defined via struct S { } should only be constructed and pattern matched via S { }.
    If we try to construct using S, we get this error S is a structure name, but this expression uses it like a function name. I believe that takes care of the restriction for construction. The only thing that needs to be implemented, is a check that disallows pattern matching with S and allow pattern matching only with S { }.

I've checked the above, after removing the 2 conditions that i have mentioned in my previous comment.

A quick question

Say i construct an empty struct using struct S; and define it using S. Should pattern matching be possible using both S and S { } or just using S since we have constructed using S ?

Please correct me if i am wrong, I don't have much knowledge about the internals.

@nrc
Copy link
Member

nrc commented Jul 20, 2015

@Dineshs91 yes, if the struct is defined without braces then it can be pattern matched either with or without braces. How the struct is constructed doesn't make any difference.

The underlying reason for the difference is that Rust has two namespaces when we lookup the name of something - types and values (values includes functions). When you use struct Foo {}, that only puts Foo into the type namespace. When you write Struct Foo; that puts Foo into both namespaces. When you write let _ = Foo;, name resolution looks up Foo in the value namespace. When you write let _ = Foo {};, name resolution looks up Foo in the type namespace. (And likewise for pattern matching).

@nrc
Copy link
Member

nrc commented Jul 24, 2015

@Dineshs91 hey, how did you get along the namespacing? Can I help you out?

@Dineshs91
Copy link

@nrc I haven't started digging into namespace thing yet. I will try to work it out this weekend.
If i need any help, i will ping you. Thanks !

@alexcrichton alexcrichton added the T-lang Relevant to the language team, which will review and decide on the PR/issue. label Aug 11, 2015
@pnkfelix
Copy link
Member

(Also, it may be good to attempt to put this change in behind a feature-gate while we get the initial wrinkles worked out. That's another reason that removing checks does not suffice.)

So, it looks like PR #28336 successfully implements this without adding any additional feature gates.

@rust-lang/lang Does this feature need a feature gate? Or shall we just land PR #28336 (preferably right after the upcoming beta is cut)?

bors added a commit that referenced this issue Sep 18, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
B-RFC-approved Blocker: Approved by a merged RFC but not yet implemented. E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion. T-lang Relevant to the language team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

6 participants