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

Use doc comments from 'pub use' statements #63048

Merged
merged 1 commit into from
Aug 4, 2019

Conversation

Aaron1011
Copy link
Member

Split off from #62855

Currently, rustdoc ignores any doc comments found on 'pub use'
statements. As described in issue #58700, this makes it impossible to
properly document procedural macros. Any doc comments must be written on
the procedural macro definition, which must occur in a dedicated
proc-macro crate. This means that any doc comments or doc tests cannot
reference items defined in re-exporting crate, despite the fact that
such items may be required to use the procedural macro.

To solve this issue, this commit allows doc comments to be written on
'pub use' statements. For consistency, this applies to all 'pub use'
statements, not just those importing procedural macros.

When inlining documentation, documentation on 'pub use' statements will
be prepended to the documentation of the inlined item. For example,
the following items:

mod other_mod {
    /// Doc comment from definition
    pub struct MyStruct;
}

/// Doc comment from 'pub use'
///
pub use other_mod::MyStruct;

will caues the documentation for the re-export of 'MyStruct' to be
rendered as:

Doc comment from 'pub use'
Doc comment from definition

Note the empty line in the 'pub use' doc comments - because doc comments
are concatenated as-is, this ensure that the doc comments on the
definition start on a new line.

Split off from rust-lang#62855

Currently, rustdoc ignores any doc comments found on 'pub use'
statements. As described in issue rust-lang#58700, this makes it impossible to
properly document procedural macros. Any doc comments must be written on
the procedural macro definition, which must occur in a dedicated
proc-macro crate. This means that any doc comments or doc tests cannot
reference items defined in re-exporting crate, despite the fact that
such items may be required to use the procedural macro.

To solve this issue, this commit allows doc comments to be written on
'pub use' statements. For consistency, this applies to *all* 'pub use'
statements, not just those importing procedural macros.

When inlining documentation, documentation on 'pub use' statements will
be prepended to the documentation of the inlined item. For example,
the following items:

```rust

mod other_mod {
    /// Doc comment from definition
    pub struct MyStruct;
}

/// Doc comment from 'pub use'
///
pub use other_mod::MyStruct;
```

will caues the documentation for the re-export of 'MyStruct' to be
rendered as:

```
Doc comment from 'pub use'
Doc comment from definition
```

Note the empty line in the 'pub use' doc comments - because doc comments
are concatenated as-is, this ensure that the doc comments on the
definition start on a new line.
@Centril
Copy link
Contributor

Centril commented Jul 27, 2019

r? @GuillaumeGomez

@JohnTitor
Copy link
Member

Ping from triage: @GuillaumeGomez, waiting on your review.

@GuillaumeGomez
Copy link
Member

Sorry, completely forgot about it.

Looks good to me, thanks a lot @Aaron1011!

@bors: r+

@bors
Copy link
Contributor

bors commented Aug 4, 2019

📌 Commit 7ee9b7a has been approved by GuillaumeGomez

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Aug 4, 2019
@bors
Copy link
Contributor

bors commented Aug 4, 2019

⌛ Testing commit 7ee9b7a with merge 460072e...

bors added a commit that referenced this pull request Aug 4, 2019
…laumeGomez

Use doc comments from 'pub use' statements

Split off from #62855

Currently, rustdoc ignores any doc comments found on 'pub use'
statements. As described in issue #58700, this makes it impossible to
properly document procedural macros. Any doc comments must be written on
the procedural macro definition, which must occur in a dedicated
proc-macro crate. This means that any doc comments or doc tests cannot
reference items defined in re-exporting crate, despite the fact that
such items may be required to use the procedural macro.

To solve this issue, this commit allows doc comments to be written on
'pub use' statements. For consistency, this applies to *all* 'pub use'
statements, not just those importing procedural macros.

When inlining documentation, documentation on 'pub use' statements will
be prepended to the documentation of the inlined item. For example,
the following items:

```rust

mod other_mod {
    /// Doc comment from definition
    pub struct MyStruct;
}

/// Doc comment from 'pub use'
///
pub use other_mod::MyStruct;
```

will caues the documentation for the re-export of 'MyStruct' to be
rendered as:

```
Doc comment from 'pub use'
Doc comment from definition
```

Note the empty line in the 'pub use' doc comments - because doc comments
are concatenated as-is, this ensure that the doc comments on the
definition start on a new line.
@bors
Copy link
Contributor

bors commented Aug 4, 2019

☀️ Test successful - checks-azure
Approved by: GuillaumeGomez
Pushing 460072e to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Aug 4, 2019
@bors bors merged commit 7ee9b7a into rust-lang:master Aug 4, 2019
bors bot added a commit to taiki-e/pin-project that referenced this pull request Aug 5, 2019
18: Replace `unsafe_project` with safe `pin_projectable` attribute r=taiki-e a=Aaron1011

This PR replaces the `unsafe_project` attribute with a new, completely safe `pin_projectable` attribute. This attribtute enforces all of the guaranteees of pin projection:

* `#[repr(packed)]` types are disallowed completely, via a compile-time error.
* A `Drop` impl is unconditionally provided. A custom drop function can be provided by annotating a function with `#[pinned_drop]`. This function takes a `Pin<&mut MyType>`, which ensures that the user cannot move out of pinned fields in safe code.
* An `Unpin` impl is unconditionally provided. By default, this generates the same bounds as the (now removed) `Unpin` argument - all pin-projected fields are required to implement `Unpin`. A manual impl can be provided via the `unsafe_Unpin` attribute, and a impl of the new `unsafe` trait `UnsafeUnpin`.

This is a significant, non-backwards-compatible refactor of this crate. However, I believe it provides several significant benefits:

* Pin projection is now *a safe operation*. In the vast majority of cases, consumers of this crate can create pin projections without a single use of `unsafe`. This reduces the number of `unsafe` blocks that must be audited, and increases confidence that a crate does not trigger undefined behavior.
* The expressive power of the `#[unsafe_project]` and `#[pin_projectable]` is the same. Any code written using `#[unsafe_project]` can be adapted to `#[pin_projectable]`, even if relied in the `unsafe` nature of `#[unsafe_project]`:

  * `UnsafeUnpin` can be used to obtain complete control over the generated `Unpin` impl.
  * `Pin::get_unchecked_mut` can be used within a `#[pinned_drop]` function to obtain a `&mut MyStruct`, effectively turing `#[pinned_drop]` back into a regular `Drop` impl.
  * For `#[repr(packed)]` structs, there are two possible cases:
    * Pin projection is never used - no fields have the `#[pin]` attribute, or `project()` is never called on the base struct. In this case, using this crate for the struct is completely pointless, and the `#[unsafe_project]` attribute can be removed.
    * Pin projection *is* used. This is immediate undefined behavior - the new `#[pin_projectable`]` attribute is simply not allowing you to write broken code.
* Anything with the potential for undefined behavior now requires usage of the `unsafe` keyword, whearas the previous `#[unsafe_project]` attribute only required typing the word 'unsafe' in an argument. Using the actual `unsafe` keyword allows for proper integration with the `unsafe_code` lint, and tools [cargo geiger](https://github.com/anderejd/cargo-geiger). Note that the `unsafe_Unpin` argument still obeys this rule - the `UnsafeUnpin` trait it enables is unsafe, and failing to provide an impl of `UnsafeUnpin` is completely safe.

Unfortunately, this PR requires `pin-project` to be split into two crates - `pin-project` and `pin-project-internal`. This is due to the fact that `proc-macro` crates cannot currently export anything other than proc macros.  The crates are split as follows:
* A new `pin-project-internal` crates provides almost all of the functionality of this crate, with the exception of the `UnsafeUnpin` trait.
* The `pin-project` crate now re-exports everything from `pin-project-internal`, with added doc comments. It also provides the `UnsafeUnpin` trait.

Because the `pin-project-internal` crate must reference the `UnsafeUnpin` trait from `pin-project`, `pin-project-internal` implicitly depends on `pin-project`. To ensure that users can rename their dependency on `pin-project`, the crate [proc_macro_crate](https://crates.io/crates/proc_macro_crate) is used to dynamically determine the name of the `pin-project` crate at macro invocation time.

Due to several issues with Rustdoc's handling of procedural macros, the documentation for the `pin-project` crate will not currently render properly - however, all doctests will still run correctly. Once rust-lang/rust#62855 and rust-lang/rust#63048 are merged, the documentation will build correctly on nightly Rust.

@taiki-e: I'm happy to work with you to make any adjustments necessary to get this merged (once the referenced rustc PRs are merged).

Co-authored-by: Aaron Hill <aa1ronham@gmail.com>
@jyn514 jyn514 added T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. A-cross-crate-reexports Area: Documentation that has been re-exported from a different crate labels Aug 30, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-cross-crate-reexports Area: Documentation that has been re-exported from a different crate merged-by-bors This PR was explicitly merged by bors. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants