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

Split Types Chapter #454

Merged
merged 3 commits into from
Nov 5, 2018
Merged

Split Types Chapter #454

merged 3 commits into from
Nov 5, 2018

Conversation

ehuss
Copy link
Contributor

@ehuss ehuss commented Oct 17, 2018

This splits the types chapter into multiple subchapters.

I tried to avoid content changes, but I ended up with a few changes:

  • Added overview links in types.md, and a separate discussion of type expressions.
  • Split array and slice into separate chapters. Although there is some overlap between them, I feel like it was awkward explaining one and then the other. I also felt it conflated the two too much since slices can be used for more than arrays.
  • Combined Self section with the one on the paths page.
  • Added a basic redirector for external links.

Some pages may seem a little bare now, but I think each type page has potential to be expanded and improved in the future.

Closes #445.

@ehuss
Copy link
Contributor Author

ehuss commented Oct 17, 2018

Some things I'd like feedback on:

  • Should the user-defined types (struct/enum/union) be combined into a single page? I feel like its awkward that there are two places to describe them (type and item pages), and that there is already some overlap with the item pages. I think it might be better to avoid the duplication, and leave the type page relatively basic.
    • The discussion of "recursive types" could maybe be moved to this combined user-type page.
  • I'm not confident the list of types that can be specified with Type Paths is complete.

src/SUMMARY.md Outdated
- [Boolean type](types/boolean-type.md)
- [Numeric types](types/numeric-type.md)
- [Textual types](types/textual-type.md)
- [Never type](types/never-type.md)
Copy link
Contributor

Choose a reason for hiding this comment

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

The Never type should be removed. It's not stable and was added during the brief couple days it was stable.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What do you think about just adding a section to the never page that explains that it is only valid in function return type positions?

Otherwise, I would think #206 should be reverted, and the grammar updated so that function return types are _Type_ | !.

Also, it looks like there was some discussion on that PR about retaining the information about divergence (and #271), but that never happened?

Copy link
Contributor

@Havvy Havvy left a comment

Choose a reason for hiding this comment

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

I'd rather not have each page be types/whatever-type.html repeating the word "type". It just makes typing the URL manually difficult. Although I may be of the rare breed who actually does that.

@@ -0,0 +1,41 @@
<script>
Copy link
Contributor

Choose a reason for hiding this comment

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

This is nice. I usually don't think about the disruption of moving all the information around. I'd rather this be done via an mdbook plugin, but since that plugin doesn't exist, this is fine. Speaking of which, we really need to update our books to mdbook 0.2.x.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is it correct that all the books will need to upgraded to 0.2 at the same time? Otherwise, I imagine the rust-lang/rust "rustbook" tool would need to handle both versions, which sounds troublesome.

I'd be happy to help make that happen. I feel like it would be best to make a tool to automatically update the books to make life easier.

@ehuss ehuss force-pushed the split-types branch 2 times, most recently from 89059b2 to 7166c5f Compare October 23, 2018 15:53
@ehuss
Copy link
Contributor Author

ehuss commented Oct 23, 2018

I'd rather not have each page be types/whatever-type.html repeating the word "type".

No problem, I was just mimicking the expression pages.

Copy link
Contributor

@matthewjasper matthewjasper left a comment

Choose a reason for hiding this comment

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

Nice! I've left some comments but a lot of these are preexisting, so feel to ignore those that are.

src/types.md Outdated
* [Boolean] — `true` or `false`
* [Numeric] — integer and float
* [Textual] — `char` and `str`
* [Never] — `!` — a type with no value
Copy link
Contributor

Choose a reason for hiding this comment

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

no values

src/types.md Outdated
* [Trait objects]
* [Impl trait]


Copy link
Contributor

Choose a reason for hiding this comment

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

double blank line

src/types.md Outdated
boundary applies, so the use of parentheses is required. Grammar rules that
require this disambiguation use the [_TypeNoBounds_] rule instead of
[_Type_].


```rust
# use std::any::Any;
type T<'a> = &'a(Any + Send);
Copy link
Contributor

Choose a reason for hiding this comment

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

add a space before (, use dyn


## Self types

The special type `Self` has a meaning within traits and implementations: it
Copy link
Contributor

Choose a reason for hiding this comment

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

Was this removed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It was merged with the discussion of Self on the paths page.

src/types.md Outdated
@@ -768,7 +111,8 @@ let x: Vec<_> = (0..10).collect();
There should be a broader discussion of type inference somewhere.
-->

## Type parameters

### Type parameters
Copy link
Contributor

Choose a reason for hiding this comment

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

This should be on its own page

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What do you think about moving it to items/generics.md. It seems like it would be natural for it to live there.

Copy link
Contributor

Choose a reason for hiding this comment

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

No, I would prefer them to live with all of the other types.

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 feel like it would be inconsistent to do that. Currently everything that was moved to a separate page is an actual type. To me, type parameters are not types themselves (they don't have a defined meaning on their own). Similar to Self and _, they are things that can be resolved into an actual type.

Perhaps it could be moved as a second-level page under "Type System"? The same question could be applied to Inferred type which probably equally deserves to be separate (considering it probably needs a lot more detail and examples).

Copy link
Contributor

Choose a reason for hiding this comment

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

They don't resolve to an actual type until monomorphisation, so for almost all of compilation they are their own types with their own rules.

Either way, I'm happy to merge this as is once it has been rebased (again -_-).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok, I agree! 😄 I went ahead and moved the inferred type to its own page, too, since I foresee that potentially getting long by itself. It's not too hard to move these around, and with the redirects it hopefully isn't too disruptive (although it's a little rough on outstanding PRs).

An array is a fixed-size sequence of `N` elements of type `T`. The array type
is written as `[T; N]`.

An array can be allocated on either the stack or the heap. The size is an
Copy link
Contributor

Choose a reason for hiding this comment

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

This sentence can be removed, and the two paragraphs merged.

let boxed_array: Box<[i32]> = Box::new([1, 2, 3]);
```

All elements of arrays are always initialized, and access to an array is
Copy link
Contributor

Choose a reason for hiding this comment

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

I guess this should now say something like "array elements may only be moved from by slice patterns".

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'm not sure exactly what you mean here. In general it seems like a lot more can be said, though.


A function pointer type consists of a possibly-empty set of function-type
modifiers (such as `unsafe` or `extern`), a sequence of input types and an
output type.
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we remove places where we say what's already in the Syntax? This should either explain what unsafe or extern do, or not be here at all.

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 agree. There are quite a few places that I neglected to remove the prose description of the syntax. I'll leave that on my todo list.


* The unsigned word types `u8`, `u16`, `u32`, `u64`, and `u128` with values
drawn from the integer intervals [0, 2^8 - 1], [0, 2^16 - 1], [0, 2^32 - 1],
[0, 2^64 - 1], and [0, 2^128 - 1] respectively.
Copy link
Contributor

Choose a reason for hiding this comment

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

These should use <sup> tags. They also should be in a table.

@ehuss
Copy link
Contributor Author

ehuss commented Oct 23, 2018

Made a few updates. I don't mind making a few content edits, but I don't want to get too bogged down on rewriting things. A lot of these pages are very bare and will probably need significant work (probably by someone more qualified than me).

@matthewjasper
Copy link
Contributor

This needs rebasing

@ehuss
Copy link
Contributor Author

ehuss commented Nov 3, 2018

This needs rebasing

Rebased.

ehuss added 3 commits November 4, 2018 18:01
This splits the types chapter into multiple subchapters.

I tried to avoid content changes, but I ended up with a few changes:
- Added overview links in `types.md`, and a separate discussion of type expressions.
- Split array and slice into separate chapters. Although there is some overlap between them, I feel like it was awkward explaining one and then the other. I also felt it conflated the two too much since slices can be used for more than arrays.
- Combined `Self` section with the one on the paths page.
- Added a basic redirector for external links.

Some pages may seem a little bare now, but I think each type page has potential to be expanded and improved in the future.
@matthewjasper
Copy link
Contributor

Thanks!

@matthewjasper matthewjasper merged commit 5aafa12 into rust-lang:master Nov 5, 2018
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.

3 participants