Implement empty structs with braces (RFC 218) #28336
Conversation
|
r? @huonw (rust_highfive has picked a reviewer for you, use r? to override) |
petrochenkov
force-pushed
the
empstr
branch
from
d74e357 to
9023920
Compare
| @@ -8,13 +8,10 @@ | |||
| // option. This file may not be copied, modified, or distributed | |||
| // except according to those terms. | |||
|
|
|||
| // compile-flags: -Z parse-only | |||
| // Empty struct defined with braces shouldn't add names into value namespace | |||
There was a problem hiding this comment.
Great to have a test for it, but.
Isn't this a surprising gotcha? struct S {} and struct S; being noticeably different?.
There was a problem hiding this comment.
I see the RFC is quite explicit about this design, so that was just something new to me.
There was a problem hiding this comment.
This is a part of the RFC and a result of long discussion.
Personally, I find this quite logical - struct S {} is a usual struct and it doesn't add anything to the value namespace, it just happens to have an empty list of fields. struct S; in its turn is syntactically special and represents usual struct + some special magic.
|
r? @pnkfelix (random member of the compiler team) |
| @@ -1178,11 +1178,20 @@ let px: i32 = match p { Point(x, _) => x }; | |||
| ``` | |||
|
|
|||
| A _unit-like struct_ is a structure without any fields, defined by leaving off | |||
| the list of fields entirely. Such types will have a single value. For example: | |||
| the list of fields entirely. Such structure implicitly defines a constant of | |||
| its type with the same name. For example: | |||
There was a problem hiding this comment.
- I like this way of phrasing the semantics better than the RFC text itself (which had that odd "flexible" versus "braced" distinction). Bravo
- Nit: I think the correct grammar here would be "Such a structure implicitly defines"; but honestly I won't let this alone block the PR. (Let's see where the review goes.)
|
This looks great. I noticed the RFC didn't specify a feature gate. I think that is due to that RFC being originally authored before we had a policy of by-default attaching some feature gate to every RFC. I personally would not mind landing this without a feature gate, but perhaps it is worth double-checking that this is what was intended. I'll add a note to the tracking issue about this. |
|
I thought about a feature gate, but decided not to add it. It's not like the semantics of |
|
Feature gate does feel a bit like overkill. I threw this on the agenda for lang mtg tomorrow. |
|
I would probably prefer a feature gate - the whole point of them is that we don't expect things to be bad but want to be careful. OTOH, I'm not too bothered. |
|
I think we should gate as a matter of course. It'd be good to lay down an explicit policy about this. FWIW, the libs team policy is that any nontrivial new API has to undergo implementation FCP for at least a cycle before being stabilized -- the same as being gated for a cycle here. We do stabilization FCPs in a batch at the beginning of a cycle, and collect 6 weeks of comments before deciding just prior to the next beta. It'd be a bit odd to be more lax with language features :P |
|
Yeah, makes sense. Basically every time I've skipped a feature gate I've On Thu, Sep 17, 2015 at 6:51 PM, Aaron Turon notifications@github.com
|
|
Updated with a feature gate. |
petrochenkov
force-pushed
the
empstr
branch
from
d4e960a to
1eb42f1
Compare
|
@bors r+ |
|
📌 Commit 1eb42f1 has been approved by |
alexcrichton
added
the
relnotes
This patch uses the same data structures for structs and enum variants in AST and HIR. These changes in data structures lead to noticeable simplification in most of code dealing with them. I didn't touch the top level, i.e. `ItemStruct` is still `ItemStruct` and not `ItemEnum` with one variant, like in the type checker. As part of this patch, structures and variants get the `kind` field making distinction between "normal" structs, tuple structs and unit structs explicit instead of relying on the number of fields and presence of constructor `NodeId`. In particular, we can now distinguish empty tuple structs from unit structs, which was impossible before! Comprehensive tests for empty structs are added and some improvements to empty struct feature gates are made. Some tests don't pass due to issue #28692 , they are still there for completeness, but are commented out. This patch fixes issue mentioned in #16819 (comment), now emptiness of tuple structs is checked after expansion. It also touches #28750 by providing span for visit_struct_def cc #28336 r? @nrc
Closes #24266
Closes #16819