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

Improve the documentation of primitive types #11409

Closed
wants to merge 4 commits into from

Conversation

alexcrichton
Copy link
Member

In today's rustdoc world, if you have the simple question of "What can I do with i8?" it turns out the answer is "good luck". The reason for this is that there is no page in the rustdoc documentation which shows any information about primitive types.

This commit adds support for a lang item per primitive which is allowed to define inherent methods of primitive types. This alleviates concerns of duplicate inherent impls for primitive types, and also allows in theory removal of lots of satellite traits lying around.

Additionally, I have added support to rustdoc to render pretty pages for all primitive types. Additionally, all mentions of primitive types are hyperlinked to their definition (as one might expect).

I didn't remove any traits other than Times (for a direct inherent impl on uint), but in theory many more can be removed in the future.

The astute may notice that this is not done for string and vector types yet. I haven't quite figured out how to do that just yet.

@brson
Copy link
Contributor

brson commented Jan 9, 2014

Adding these lang items probably requires further discussion.

@brson
Copy link
Contributor

brson commented Jan 9, 2014

This is pretty darn cool.

@huonw
Copy link
Member

huonw commented Jan 9, 2014

The astute may notice that this is not done for string and vector types yet

Tuples are not supported either?

(For vectors and strings, maybe the easiest path is have lang items for the actual types (to give the sugar) and have #[lang_item="vec_slice"] struct VecSlice<T> { ptr: *T, length: uint } although this might not jive with DST?)

@@ -4979,3 +4983,39 @@ impl substs {
}
}
}

/// Register a primitive type as having it's lang-item implementation in this
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: "its"

@alexcrichton
Copy link
Member Author

Aha, you are more astute than I @huonw! I forgot to mention that yes, this doesn't cover tuples as well. The general idea is that I'm not sure how to deal with type parameters right now.

I think that ~str and &str should be pretty easily possible (seeing how there's no type parameters), but I don't think that we should change the standard library crucially to get better documentation. I think we should explore the routes of making rustdoc/rustc work better together to get the existing methods of defining vecs/strs better.

In the future it sounds like the "vector builder" will be a documentable type, but slices and fixed-size arrays will always be able to have methods on them and we should be able to document them somehow.

I think that the threshold for knowing whether the docs are sufficient is currently answering the question "What can I do with ~[T]" because the answer right now is "dig through one of 12 traits"

@huonw
Copy link
Member

huonw commented Jan 9, 2014

I don't think that we should change the standard library crucially to get better documentation

It's not just to get better documentation, having a large number of one-implementation traits isn't exactly nice from a maintainability perspective. (Although documentation is the big problem.)

Also, this lang-items-on-impls approach means that you can only have one impl uint {} ever, right? You can't have, say

impl uint {
      fn some_method(&self) {}
}

impl uint {
     fn some_other_method(&self) {}
}

This isn't too problematic for plain types like uint (other than being less flexible for organising one's code), but is problematic for generic types (and I assume this is partially what you mean when you say you're having trouble dealing with type parameters).

@thestinger
Copy link
Contributor

Can we attach the lang items them to private placeholder types and then permit an impl on them?

@alexcrichton
Copy link
Member Author

It's true, and I certainly don't think we should take the state of the standard library as-is today and document it. I totally agree that we need to remove all the one-off traits for vectors and strings. I probably should have phrased that along the lines of avoiding rearchitecting all of libstd, but small reorganizations like collecting everything in one impl seems fine to me.

It is a little limiting that you can only have on impl uint block, but it's purely a library thing that no one else will ever worry about, so I'm not worried too much about that.

My initial reaction for vectors was vec_impl, vec_eq_impl, vec_ord_impl, etc (all lang items), but again, that's a pretty unsatisfying approach.

@alexcrichton
Copy link
Member Author

Hm, so rustdoc will always have to do something special in order to deal with primitive types, but there's also a question to what degree rustc itself needs to be modified as well.

That may work though having a private type bool = bool or type Vec<T> = ~[T] in these files and then the lang item is attached to that type instead. You've then got a bunch of defids to thread through, but things like privacy/reachability have to ensure that these are not considered dead.

I haven't really given these too much thought, my main goal was to get something reasonable for primitive types, but perhaps for vectors some sort of lang item not on the impl may work best.

@thestinger
Copy link
Contributor

@alexcrichton: I was thinking that there just needs to be a single dummy lang item (#[lang = "int"] struct Foo;) and then you are permitted to do any implementations you want on int in that crate. It doesn't seem like it would be much more complicated.

/// A convenience form for basic repetition. Given a uint `x`,
/// `x.times(|| { ... })` executes the given block x times.
///
/// Equivalent to `for uint::range(0, x) |_| { ... }`.
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 just range now rather than uint::range.

Copy link
Member

Choose a reason for hiding this comment

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

(Also, the syntax is all wrong.)

Copy link
Member Author

Choose a reason for hiding this comment

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

Updated.

@bill-myers
Copy link
Contributor

What's the point of the lang items?

Surely the compiler and rustdoc can recognize an "impl u8" without the lang item?

At most it seems one might need a single lang item to mark libstd as the crate to lookup those impls in / allow it to provide impls for primitives.

Or perhaps a single "primitive_impl" lang item on each impl.

@alexcrichton
Copy link
Member Author

The current coherence infrastructure can't handle multiple impl u8 across crate blocks (and I'm unsure of whether it can handle multiple ones in the current crate), so having the impls become lang items seemed like the least intrusive way to do this. This whole change has nothing to do with anyone interfacing with libstd, it's just trying to get rustdoc to have a better understand of what primitives are and how to document them (currently it cannot do that at all).

@alexcrichton
Copy link
Member Author

ping, from what it sounds like there's no consensus on whether this is the right approach to take, and I'd like to get something like this merged. This will greatly improve the experience for looking at primitive docs. (I'll rebase this soon as well).

@brson
Copy link
Contributor

brson commented Jan 20, 2014

Let's talk about it at the meeting tomorrow.

@alexcrichton
Copy link
Member Author

r?

We decided in today's meeting that we'd like to merge this, and we can revisit str/vec when DST comes about.

@huonw
Copy link
Member

huonw commented Jan 21, 2014

Repeating my comment in response to the meeting:

I'm very much in favour of better documentation on primitives... but not so much in favour of the scheme as implemented: having a lang-item for each and every impl of builtins seems impossible to extend to generics like vectors and tuples.

Also, I don't see how DST will resolve this/make it any easier when revisiting: we'll always want impl<'a, T: Trait> &'a [T] for multiple different Traits (e.g. Eq, Clone, etc), and DST isn't abstracting that.

This solves horrible diffs where all you do is renumber literally everything.
These impls are mostly currently blank, but they will get filled out in the
future with implementations as we remove satellite traits in favor of inherent
methods on primitives.
This piggybacks on the previous commits to add impls of primitives directly.
@alexcrichton
Copy link
Member Author

This ended up hitting coherence difficulties, I'll try to get back to this

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.

None yet

7 participants