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

update documentation of tuple/unit structs #33250

Merged
merged 6 commits into from
Jul 7, 2016
Merged

Conversation

durka
Copy link
Contributor

@durka durka commented Apr 28, 2016

I made the "tuple structs are useless" editorializing a bit weaker and moved it to the end. Feel free to overrule me on that.

I also added an example of how to unpack a tuple struct with dot notation, because it came up on IRC.

braced_empty_structs is stable now, so I updated the example for unit-like structs to use that syntax. Should we show both ways?

cc @ubsan
r? @steveklabnik or @GuillaumeGomez

I made the "tuple structs are useless" editorializing a bit weaker and moved it to the end. Feel free to overrule me on that.

I also added an example of how to unpack a tuple struct with dot notation, because it came up on IRC.

`braced_empty_structs` is stable now, so I updated the example for unit-like structs to use that syntax. Should we show both ways?

cc @ubsan
r? @steveklabnik or @GuillameGomez
@@ -163,11 +163,48 @@ struct Point(i32, i32, i32);
let black = Color(0, 0, 0);
let origin = Point(0, 0, 0);
```

Here, `black` and `origin` are not equal, even though they contain the same
values.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This sentence could be confusing even though I get what it is trying to say. Color(0, 0, 0) and Color(0, 0, 0) are also not "equal" in some sense because we didn't derive PartialEq.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Maybe it should just say "are not the same type, even though they contain the same values".

Copy link
Member

Choose a reason for hiding this comment

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

Excellent idea! Your second suggestion is just perfect. Could you replace the sentence please?

@strega-nil
Copy link
Contributor

I'm of the opinion it should show both ways, or only struct Doop;. I think that's still the preferred way to create a unit struct.

@durka
Copy link
Contributor Author

durka commented Apr 28, 2016

That's the opposite of the advice you gave me on IRC :)

The RFC doesn't say anything about this -- it says if you have struct Doop; then you should initialize/match it without braces, but it doesn't suggest one form or another. I thought the intention was that once braced_empty_structs was stable, we should prefer it for consistency.

I now think the book should show both ways, but maybe handwave over the type/value namespace thing (leave that to the reference).

@GuillaumeGomez
Copy link
Member

@durka: If the empty braces are stable then yes. Adding features in documentation would make the whole a lot more complicated. Let's wait for @steveklabnik's confirmation.

`let`, just like regular tuples:

```rust
# struct Color(i32, i32, i32);
Copy link
Member

Choose a reason for hiding this comment

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

Why the '#' at the beginning of the line?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

To hide the line from the documentation, while it's needed to make the example self-contained it's repetitive from the example above.

@steveklabnik
Copy link
Member

I am not sure what the preferred way should be, to be honest.

@steveklabnik
Copy link
Member

I am generally pro this patch, but yeah, the conventions question is interesting. @aturon ?

let integer_length = length.0;
```

It's always possible to use a `struct` than a tuple struct, and can be clearer.
Copy link
Contributor

Choose a reason for hiding this comment

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

s/than/instead of/

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Whoops.

@GuillaumeGomez
Copy link
Member

r=me @steveklabnik

@durka
Copy link
Contributor Author

durka commented May 6, 2016

Any interest in committing to a convention here?

@strega-nil
Copy link
Contributor

Hmm, I'm of the opinion that struct Struct; should be preferred, although struct Struct {} should also be introduced.

@durka
Copy link
Contributor Author

durka commented May 6, 2016

@ubsan any reasoning behind that, or just aesthetics?

@durka
Copy link
Contributor Author

durka commented May 6, 2016

To take a counter-position (I truly don't feel strongly): struct Struct {} doesn't pollute the value namespace, and it is more consistent with other {}-structs (though not with tuple structs).

@steveklabnik
Copy link
Member

I do not want to block this PR on a conventions question we don't have a good way to answer. @durka can you update it with showing both forms, please? And then r=me after.

@durka
Copy link
Contributor Author

durka commented Jul 5, 2016

Done.

@steveklabnik
Copy link
Member

@bors: r+ rollup

Thanks! :)

@bors
Copy link
Contributor

bors commented Jul 5, 2016

📌 Commit 74e9629 has been approved by steveklabnik

steveklabnik added a commit to steveklabnik/rust that referenced this pull request Jul 5, 2016
update documentation of tuple/unit structs

I made the "tuple structs are useless" editorializing a bit weaker and moved it to the end. Feel free to overrule me on that.

I also added an example of how to unpack a tuple struct with dot notation, because it came up on IRC.

`braced_empty_structs` is stable now, so I updated the example for unit-like structs to use that syntax. Should we show both ways?

cc @ubsan
r? @steveklabnik or @GuillaumeGomez
steveklabnik added a commit to steveklabnik/rust that referenced this pull request Jul 5, 2016
update documentation of tuple/unit structs

I made the "tuple structs are useless" editorializing a bit weaker and moved it to the end. Feel free to overrule me on that.

I also added an example of how to unpack a tuple struct with dot notation, because it came up on IRC.

`braced_empty_structs` is stable now, so I updated the example for unit-like structs to use that syntax. Should we show both ways?

cc @ubsan
r? @steveklabnik or @GuillaumeGomez
steveklabnik added a commit to steveklabnik/rust that referenced this pull request Jul 6, 2016
update documentation of tuple/unit structs

I made the "tuple structs are useless" editorializing a bit weaker and moved it to the end. Feel free to overrule me on that.

I also added an example of how to unpack a tuple struct with dot notation, because it came up on IRC.

`braced_empty_structs` is stable now, so I updated the example for unit-like structs to use that syntax. Should we show both ways?

cc @ubsan
r? @steveklabnik or @GuillaumeGomez
bors added a commit that referenced this pull request Jul 7, 2016
@bors bors merged commit 74e9629 into rust-lang:master Jul 7, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants