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 Rustdoc's handling of procedural macros #62855

Open
wants to merge 2 commits into
base: master
from

Conversation

@Aaron1011
Copy link
Contributor

commented Jul 21, 2019

Fixes #58700
Fixes #58696
Fixes #49553
Fixes #52210

The current handling of cross-crate proc macros leaves much to be
desired. Proc macros receive special handling in the compiler compared
to all other exported items - instead of being stored in the normal
crate metadata, they are stored together in an array under a special
symbol in the produced dylib. This allows proc-macro code to be properly
invoked by the compiler, but it causes us to lose a great deal of
information about the definition about proc macros defined in another
crate. Specifically, we lose the definition Span and attributes,
which prevents Rustdoc from rednering source links and doc comments
respectively.

This behavior has lead to a number of issues (linked at the beginning of
this PR). Re-exporting proc macros is often required, due to
proc-macro crates being restricted from exporting any non-macro items.
Not only does re-exporting cause the original doc comments to be lost,
it is impossible to add new doc comments to the re-export (i.e. pub use).
If a proc macro msut be used in conjunction with a type from the
re-exported crate (e.g. due to generated code referencing that type),
it becomes impossible to write proper doc tests.

This PR makes two changes to rustdoc and associated compiler code:

  1. Re-exporting proc macros now behaves just like re-exporting any other
    type. Doc comments and source information are preserved, bring proc
    macros into line with all other types in the language in terms of
    documentation suppot.

  2. Doc comments are now supported on 'pub use' statements. In the
    rendered documentation, the 'pub use' doc comments will be concatenated
    with the doc comments from the original definition.

For example:

/// Original crate!
pub type Foo = u8;

/// Other crate!
///
pub use original_crate::Fool

Will result in the following rendered text:

Other Crate!
Original Crate!

Note the newline in the 'pub use' doc comment - since the strings are
concatenated, this causes the original doc comments to start
on the next line.

Currently, doc comments from 'pub use' statements are ignored
completely. This PR will have the effect of making visible any existing,
non-functional re-export doc comments. This could technically be
considered breaking change, but the offending doc comments can simply
be commented out (e.g. '// ///') if this is undesired.


Implementation:

This PR touches four areas of the compiler:

AST/HIR: We now keep track of additional data for all procedural macros
in a crate. Currently, this is just the Span and Vec<Attribute> from
the original definition, as Rustdoc doesn't need anything else.

librustc_metadata: We now serialize and deserialize additional data for
proc macros. This is distinct from the current dylib-based code (which
bypasses the normal RustcEncodable/RustcDecodable logic). We store this
data in the same order as we store the dylib data (i.e. the i'th entry
in the extra proc macro data corresponds to the i'th entry in the dylib
array). When we deserialize, we combine these two pieces of data to
produce a single SyntaxExtension containing all of the required data.
Some additional changes are made to expose this data through the normal
TyCtxt APIs.

rustdoc: The special handling for proc macros is removed, as we can now
retrieve their span and attributes just like any other item. New logic
is added to combine the attributes from a 'pub use' statement with the
attributes of the underlying type (this enables the concatentation of
doc comments).

More significantly, a new command-line option is added to rustdoc:
'--proc-macro-crate'. This is a more limited version of the
'--crate-type=proc-macro' option supported by rustc. When enabled, it
causes the crate to be treated as a proc macro crate. This triggers the
proc-macro related logic in the compiler - including the new logic to
make proc macro Spans and Attributes available. This ensures that
docmenting proc-macro crates behaves in the expected way.

compiletest: A new 'rustdoc-flags' option is added. This allows us to
pass in the '--proc-macro-crate' flag in the absence of Cargo.

I've opened an additional PR to Cargo to support passing in this flag.
These two PRS can be merged in any order - the Cargo changes will not
take effect until the 'cargo' submodule is updated in this repository.

@petrochenkov

This comment has been minimized.

Copy link
Contributor

commented Jul 22, 2019

@Aaron1011
Could you split the pub use change into a separate PR?
It's orthogonal to the proc macro stuff and may require a different kind of approval.
(I'll start reading the proc macro part after that, it may end up being a long story.)

@bors

This comment has been minimized.

Copy link
Contributor

commented Jul 27, 2019

☔️ The latest upstream changes (presumably #62086) made this pull request unmergeable. Please resolve the merge conflicts.

@Aaron1011 Aaron1011 force-pushed the Aaron1011:feature/rustdoc-reexport-final branch from fd5d16d to 892b81f Jul 27, 2019

@rust-highfive

This comment has been minimized.

Copy link
Collaborator

commented Jul 27, 2019

The job x86_64-gnu-llvm-6.0 of your PR failed (raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

Click to expand the log.
2019-07-27T16:57:20.0698590Z ##[command]git remote add origin https://github.com/rust-lang/rust
2019-07-27T16:57:20.0875945Z ##[command]git config gc.auto 0
2019-07-27T16:57:20.0945215Z ##[command]git config --get-all http.https://github.com/rust-lang/rust.extraheader
2019-07-27T16:57:20.1002612Z ##[command]git config --get-all http.proxy
2019-07-27T16:57:20.1130654Z ##[command]git -c http.extraheader="AUTHORIZATION: basic ***" fetch --force --tags --prune --progress --no-recurse-submodules --depth=2 origin +refs/heads/*:refs/remotes/origin/* +refs/pull/62855/merge:refs/remotes/pull/62855/merge
---
2019-07-27T16:57:52.9930332Z do so (now or later) by using -b with the checkout command again. Example:
2019-07-27T16:57:52.9930502Z 
2019-07-27T16:57:52.9930797Z   git checkout -b <new-branch-name>
2019-07-27T16:57:52.9931237Z 
2019-07-27T16:57:52.9931383Z HEAD is now at a3c144a3b Merge 892b81f2001194973e3c88e1959a45457dadb0ac into 0e9b465d729d07101b29b4d096d83edf9be82df0
2019-07-27T16:57:53.0081491Z ##[section]Starting: Collect CPU-usage statistics in the background
2019-07-27T16:57:53.0084224Z ==============================================================================
2019-07-27T16:57:53.0084305Z Task         : Bash
2019-07-27T16:57:53.0084516Z Description  : Run a Bash script on macOS, Linux, or Windows
---
2019-07-27T17:54:55.3329620Z .................................................................................................... 700/5870
2019-07-27T17:54:59.1258035Z .................................................................................................... 800/5870
2019-07-27T17:55:04.2545862Z .................................................................................................... 900/5870
2019-07-27T17:55:09.1135425Z .................................................................................................... 1000/5870
2019-07-27T17:55:14.2246693Z i...........i....................................................................................... 1100/5870
2019-07-27T17:55:17.9245900Z ..............................iiiii................................................................. 1200/5870
2019-07-27T17:55:23.4359525Z .................................................................................................... 1400/5870
2019-07-27T17:55:25.8775956Z .................................................................................................... 1500/5870
2019-07-27T17:55:29.3379459Z .................................................................................................... 1600/5870
2019-07-27T17:55:31.8200903Z .................................................................................................... 1700/5870
---
2019-07-27T17:56:41.0408046Z .................................................................................................... 3400/5870
2019-07-27T17:56:45.7685867Z .................................................................................................... 3500/5870
2019-07-27T17:56:49.4376053Z ..........................i.....................................................F................... 3600/5870
2019-07-27T17:56:53.5625671Z .................................................................................................... 3700/5870
2019-07-27T17:56:56.9085508Z ....ii...i..ii...................................................................................... 3800/5870
2019-07-27T17:57:05.2894751Z .................................................................................................... 4000/5870
2019-07-27T17:57:08.8912927Z .......................ii........................................................................... 4100/5870
2019-07-27T17:57:11.0243491Z ............................................i....................................................... 4200/5870
2019-07-27T17:57:12.9131784Z .................................................................................................... 4300/5870
---
2019-07-27T17:58:30.2050690Z diff of stderr:
2019-07-27T17:58:30.2051385Z 
2019-07-27T17:58:30.2052331Z 18   --> $DIR/same-sequence-span.rs:20:1
2019-07-27T17:58:30.2053098Z 19    |
2019-07-27T17:58:30.2053799Z 20 LL | proc_macro_sequence::make_foo!();
2019-07-27T17:58:30.2054755Z -    | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ not allowed after `expr` fragments
2019-07-27T17:58:30.2056660Z +    | |
2019-07-27T17:58:30.2056660Z +    | |
2019-07-27T17:58:30.2057644Z +    | not allowed after `expr` fragments
2019-07-27T17:58:30.2058512Z +    | in this macro invocation
2019-07-27T17:58:30.2061281Z 22    |
2019-07-27T17:58:30.2061506Z 23    = note: allowed there are: `=>`, `,` or `;`
2019-07-27T17:58:30.2061722Z 
2019-07-27T17:58:30.2062387Z 26   --> $DIR/same-sequence-span.rs:20:1
2019-07-27T17:58:30.2062548Z 27    |
2019-07-27T17:58:30.2062548Z 27    |
2019-07-27T17:58:30.2062656Z 28 LL | proc_macro_sequence::make_foo!();
2019-07-27T17:58:30.2063018Z -    | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ not allowed after `expr` fragments
2019-07-27T17:58:30.2063268Z +    | |
2019-07-27T17:58:30.2063268Z +    | |
2019-07-27T17:58:30.2063389Z +    | not allowed after `expr` fragments
2019-07-27T17:58:30.2063492Z +    | in this macro invocation
2019-07-27T17:58:30.2063594Z 30    |
2019-07-27T17:58:30.2063717Z 31    = note: allowed there are: `=>`, `,` or `;`
2019-07-27T17:58:30.2063904Z 
2019-07-27T17:58:30.2064227Z 
2019-07-27T17:58:30.2064338Z The actual stderr differed from the expected stderr.
2019-07-27T17:58:30.2065017Z Actual stderr saved to /checkout/obj/build/x86_64-unknown-linux-gnu/test/ui/macros/same-sequence-span/same-sequence-span.stderr
2019-07-27T17:58:30.2065017Z Actual stderr saved to /checkout/obj/build/x86_64-unknown-linux-gnu/test/ui/macros/same-sequence-span/same-sequence-span.stderr
2019-07-27T17:58:30.2065433Z To update references, rerun the tests and pass the `--bless` flag
2019-07-27T17:58:30.2065803Z To only update this specific test, also pass `--test-args macros/same-sequence-span.rs`
2019-07-27T17:58:30.2066066Z error: 1 errors occurred comparing output.
2019-07-27T17:58:30.2066879Z status: exit code: 1
2019-07-27T17:58:30.2066879Z status: exit code: 1
2019-07-27T17:58:30.2067887Z command: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/bin/rustc" "/checkout/src/test/ui/macros/same-sequence-span.rs" "-Zthreads=1" "--target=x86_64-unknown-linux-gnu" "--error-format" "json" "-Zui-testing" "-C" "prefer-dynamic" "--out-dir" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/ui/macros/same-sequence-span" "-Crpath" "-O" "-Cdebuginfo=0" "-Zunstable-options" "-Lnative=/checkout/obj/build/x86_64-unknown-linux-gnu/native/rust-test-helpers" "-L" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/ui/macros/same-sequence-span/auxiliary" "-A" "unused"
2019-07-27T17:58:30.2068517Z ------------------------------------------
2019-07-27T17:58:30.2068673Z 
2019-07-27T17:58:30.2069175Z ------------------------------------------
2019-07-27T17:58:30.2069412Z stderr:
2019-07-27T17:58:30.2069412Z stderr:
2019-07-27T17:58:30.2069802Z ------------------------------------------
2019-07-27T17:58:30.2070141Z error: `$x:expr` may be followed by `$y:tt`, which is not allowed for `expr` fragments
2019-07-27T17:58:30.2070630Z    |
2019-07-27T17:58:30.2070630Z    |
2019-07-27T17:58:30.2070761Z LL |     (1 $x:expr $($y:tt,)*   //~ERROR `$x:expr` may be followed by `$y:tt`
2019-07-27T17:58:30.2070872Z    |                  ^^^^^ not allowed after `expr` fragments
2019-07-27T17:58:30.2070975Z    |
2019-07-27T17:58:30.2071097Z    = note: allowed there are: `=>`, `,` or `;`
2019-07-27T17:58:30.2071414Z 
2019-07-27T17:58:30.2071565Z error: `$x:expr` may be followed by `=`, which is not allowed for `expr` fragments
2019-07-27T17:58:30.2072683Z    |
2019-07-27T17:58:30.2072683Z    |
2019-07-27T17:58:30.2072828Z LL |                $(= $z:tt)*  //~ERROR `$x:expr` may be followed by `=`
2019-07-27T17:58:30.2072967Z    |                  ^ not allowed after `expr` fragments
2019-07-27T17:58:30.2073067Z    |
2019-07-27T17:58:30.2073374Z    = note: allowed there are: `=>`, `,` or `;`
2019-07-27T17:58:30.2073516Z 
2019-07-27T17:58:30.2073948Z error: `$x:expr` may be followed by `$y:tt`, which is not allowed for `expr` fragments
2019-07-27T17:58:30.2074913Z    |
2019-07-27T17:58:30.2074913Z    |
2019-07-27T17:58:30.2075033Z LL | proc_macro_sequence::make_foo!(); //~ERROR `$x:expr` may be followed by `$y:tt`
2019-07-27T17:58:30.2076031Z    | |
2019-07-27T17:58:30.2076031Z    | |
2019-07-27T17:58:30.2076972Z    | not allowed after `expr` fragments
2019-07-27T17:58:30.2077150Z    | in this macro invocation
2019-07-27T17:58:30.2077280Z    |
2019-07-27T17:58:30.2077410Z    = note: allowed there are: `=>`, `,` or `;`
2019-07-27T17:58:30.2077522Z 
2019-07-27T17:58:30.2077692Z error: `$x:expr` may be followed by `=`, which is not allowed for `expr` fragments
2019-07-27T17:58:30.2078388Z    |
2019-07-27T17:58:30.2078388Z    |
2019-07-27T17:58:30.2078528Z LL | proc_macro_sequence::make_foo!(); //~ERROR `$x:expr` may be followed by `$y:tt`
2019-07-27T17:58:30.2078806Z    | |
2019-07-27T17:58:30.2078806Z    | |
2019-07-27T17:58:30.2078954Z    | not allowed after `expr` fragments
2019-07-27T17:58:30.2079087Z    | in this macro invocation
2019-07-27T17:58:30.2079239Z    |
2019-07-27T17:58:30.2079368Z    = note: allowed there are: `=>`, `,` or `;`
2019-07-27T17:58:30.2079951Z error: aborting due to 4 previous errors
2019-07-27T17:58:30.2080042Z 
2019-07-27T17:58:30.2080127Z 
2019-07-27T17:58:30.2080452Z ------------------------------------------
---
2019-07-27T17:58:30.2082208Z thread 'main' panicked at 'Some tests failed', src/tools/compiletest/src/main.rs:535:22
2019-07-27T17:58:30.2082363Z note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace.
2019-07-27T17:58:30.2082481Z 
2019-07-27T17:58:30.2082569Z 
2019-07-27T17:58:30.2084082Z command did not execute successfully: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-tools-bin/compiletest" "--compile-lib-path" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/lib" "--run-lib-path" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/lib/rustlib/x86_64-unknown-linux-gnu/lib" "--rustc-path" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/bin/rustc" "--src-base" "/checkout/src/test/ui" "--build-base" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/ui" "--stage-id" "stage2-x86_64-unknown-linux-gnu" "--mode" "ui" "--target" "x86_64-unknown-linux-gnu" "--host" "x86_64-unknown-linux-gnu" "--llvm-filecheck" "/usr/lib/llvm-6.0/bin/FileCheck" "--host-rustcflags" "-Crpath -O -Cdebuginfo=0 -Zunstable-options  -Lnative=/checkout/obj/build/x86_64-unknown-linux-gnu/native/rust-test-helpers" "--target-rustcflags" "-Crpath -O -Cdebuginfo=0 -Zunstable-options  -Lnative=/checkout/obj/build/x86_64-unknown-linux-gnu/native/rust-test-helpers" "--docck-python" "/usr/bin/python2.7" "--lldb-python" "/usr/bin/python2.7" "--gdb" "/usr/bin/gdb" "--quiet" "--llvm-version" "6.0.0\n" "--system-llvm" "--cc" "" "--cxx" "" "--cflags" "" "--llvm-components" "" "--llvm-cxxflags" "" "--adb-path" "adb" "--adb-test-dir" "/data/tmp/work" "--android-cross-path" "" "--color" "always"
2019-07-27T17:58:30.2084548Z 
2019-07-27T17:58:30.2084639Z 
2019-07-27T17:58:30.2086467Z failed to run: /checkout/obj/build/bootstrap/debug/bootstrap test
2019-07-27T17:58:30.2086701Z Build completed unsuccessfully in 0:53:18
2019-07-27T17:58:30.2086701Z Build completed unsuccessfully in 0:53:18
2019-07-27T17:58:31.3911823Z ##[error]Bash exited with code '1'.
2019-07-27T17:58:31.3946637Z ##[section]Starting: Checkout
2019-07-27T17:58:31.3948279Z ==============================================================================
2019-07-27T17:58:31.3948345Z Task         : Get sources
2019-07-27T17:58:31.3948383Z Description  : Get sources from a repository. Supports Git, TfsVC, and SVN repositories.

I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact @TimNN. (Feature Requests)

Aaron1011 added a commit to Aaron1011/rust that referenced this pull request Jul 27, 2019

Use doc comments from 'pub use' statements
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.
@Aaron1011

This comment has been minimized.

Copy link
Contributor Author

commented Jul 27, 2019

@petrochenkov: I've split the pub use changes into a new pull request, as you requested.

@Aaron1011

This comment has been minimized.

Copy link
Contributor Author

commented Jul 27, 2019

Note that due to changes in the metadata format, you should run ./x.py clean when switching to/from this branch locally. This is only a problem when building locally, because the version string will always be <version>-dev

@bors

This comment has been minimized.

Copy link
Contributor

commented Jul 28, 2019

☔️ The latest upstream changes (presumably #63057) made this pull request unmergeable. Please resolve the merge conflicts.

@Aaron1011 Aaron1011 force-pushed the Aaron1011:feature/rustdoc-reexport-final branch from e2efa77 to cbea48c Jul 28, 2019

@petrochenkov

This comment has been minimized.

Copy link
Contributor

commented Jul 30, 2019

(@Aaron1011, if you have time, could you squash the commits to entirely remove traces of #63048 from this PR.

I have a few busy days, but hope to return to this PR this week.)

@petrochenkov

This comment has been minimized.

Copy link
Contributor

commented Aug 2, 2019

cc @eddyb, especially regarding the metadata bits.

@bors

This comment has been minimized.

Copy link
Contributor

commented Aug 3, 2019

☔️ The latest upstream changes (presumably #63234) made this pull request unmergeable. Please resolve the merge conflicts.

@Aaron1011 Aaron1011 force-pushed the Aaron1011:feature/rustdoc-reexport-final branch from cbea48c to ef755fc Aug 4, 2019

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

Auto merge of #63048 - Aaron1011:feature/rustdoc-reexport-doc, r=Guil…
…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.
@petrochenkov

This comment has been minimized.

Copy link
Contributor

commented Aug 4, 2019

Ok, let's do the next thing.

  • Move the part of this PR not touching rustdoc or compiletest into a separate PR.
    Rustdoc is not the only consumer of the metadata from proc macro definitions.

    Tests can be written that make sure that 1) the definition span is correct and available from other crates 2) attributes like #[allow_internal_unstable], #[allow_internal_unsafe], #[unstable], #[deprecated] work when applied to proc macro definitions.

  • Assign that PR to @eddyb.
    Representation of proc macros in the metadata is the key part of the problem and I need eddyb's help with that.

    Right now I feel like the PR as implemented introduces technical debt into all the parts of the compiler that it touches. Perhaps if the metadata representation is right and stored/loaded at the right time from the right structures then it won't happen.

This is an important problem to solve and I'm glad someone is working on it, thanks.

@@ -576,6 +576,8 @@ pub struct SyntaxExtension {
pub kind: SyntaxExtensionKind,
/// Span of the macro definition.
pub span: Span,
/// Attributes declared on the macro definition
pub attrs: Vec<Attribute>,

This comment has been minimized.

Copy link
@petrochenkov

petrochenkov Aug 4, 2019

Contributor

This shouldn't be a part of SyntaxExtension.
SyntaxExtension is "lowered" form of a macro definitions in which all the attributes are converted into their semantic form (see the other fields, many of them were converted from attributes).

This comment has been minimized.

Copy link
@Aaron1011

Aaron1011 Aug 4, 2019

Author Contributor

We store attributes directly in the metadata for non-proc-macro crates - why shouldn't we do the same for proc macro crates?

This comment has been minimized.

Copy link
@petrochenkov

petrochenkov Aug 4, 2019

Contributor

SyntaxExtension is not a part of metadata, it's a representation on which name resolution and expansion work.
If you look at how non-procedural macros are loaded in fn get_macro_by_def_id, they are loaded in "unprepared" form (LoadedMacro::MacroDef(ast::Item)) first, then converted (using compile_macro) into the "prepared" form (SyntaxExtension) losing all the data not necessary for resolution/expansion (like doc comments).
Procedural macros should follow a similar scheme and reuse the attribute processing functionality from compile_macro/macro_rules::compile.

This comment has been minimized.

Copy link
@petrochenkov

petrochenkov Aug 4, 2019

Contributor

So, rustdoc is going to use the "unprepared" form for both procedural and non-procedural macros, not SyntaxExtension, since it doesn't need to do resolution/expansion (I think).

This comment has been minimized.

Copy link
@Aaron1011

Aaron1011 Aug 4, 2019

Author Contributor

In that case, should I just store it as another field in proc_macros (and replace (ast::Name, Lrc<SyntaxExtension>) with a struct)?

This comment has been minimized.

Copy link
@petrochenkov

petrochenkov Aug 4, 2019

Contributor

Yeah, somewhere "one level above" SyntaxExtension would work, proc_macros seems an appropriate place.

Improve Rustdoc's handling of procedural macros
Fixes #58700
Fixes #58696
Fixes #49553
Fixes #52210

The current handling of cross-crate proc macros leaves much to be
desired. Proc macros receive special handling in the compiler compared
to all other exported items - instead of being stored in the normal
crate metadata, they are stored together in an array under a special
symbol in the produced dylib. This allows proc-macro code to be properly
invoked by the compiler, but it causes us to lose a great deal of
information about the definition about proc macros defined in another
crate. Specifically, we lose the definition `Span` and attributes,
which prevents Rustdoc from rednering source links and doc comments
respectively.

This behavior has lead to a number of issues (linked at the beginning of
this PR). Re-exporting proc macros is often *required*, due to
proc-macro crates being restricted from exporting any non-macro items.
Not only does re-exporting cause the original doc comments to be lost,
it is impossible to add new doc comments to the re-export (i.e. `pub
use`).
If a proc macro msut be used in conjunction with a type from the
re-exported crate (e.g. due to generated code referencing that type),
it becomes impossible to write proper doc tests.

This PR makes two changes to rustdoc and associated compiler code:

1. Re-exporting proc macros now behaves just like re-exporting any other
type. Doc comments and source information are preserved, bring proc
macros into line with all other types in the language in terms of
documentation suppot.

2. Doc comments are now supported on 'pub use' statements. In the
rendered documentation, the 'pub use' doc comments will be concatenated
with the doc comments from the original definition.

For example:

/// Original crate!
pub type Foo = u8;

/// Other crate!
///
pub use original_crate::Fool

Will result in the following rendered text:

```
Other Crate!
Original Crate!
```

Note the newline in the 'pub use' doc comment - since the strings are
concatenated, this causes the original doc comments to start
on the next line.

Currently, doc comments from 'pub use' statements are ignored
completely. This PR will have the effect of making visible any existing,
non-functional re-export doc comments. This could technically be
considered  breaking change, but the offending doc comments can simply
be commented out (e.g. '// ///') if this is undesired.

---------------------------

Implementation:

This PR touches four areas of the compiler:

AST/HIR: We now keep track of additional data for all procedural macros
in a crate. Currently, this is just the `Span` and `Vec<Attribute>` from
the original definition, as Rustdoc doesn't need anything else.

librustc_metadata: We now serialize and deserialize additional data for
proc macros. This is distinct from the current dylib-based code (which
bypasses the normal RustcEncodable/RustcDecodable logic). We store this
data in the same order as we store the dylib data (i.e. the i'th entry
in the extra proc macro data corresponds to the i'th entry in the dylib
array). When we deserialize, we combine these two pieces of data to
produce a single `SyntaxExtension` containing all of the required data.
Some additional changes are made to expose this data through the normal
TyCtxt APIs.

rustdoc: The special handling for proc macros is removed, as we can now
retrieve their span and attributes just like any other item. New logic
is added to combine the attributes from a 'pub use' statement with the
attributes of the underlying type (this enables the concatentation of
doc comments).

More significantly, a new command-line option is added to rustdoc:
'--proc-macro-crate'. This is a more limited version of the
'--crate-type=proc-macro' option supported by rustc. When enabled, it
causes the crate to be treated as a proc macro crate. This triggers the
proc-macro related logic in the compiler - including the new logic to
make proc macro Spans and Attributes available. This ensures that
docmenting proc-macro crates behaves in the expected way.

compiletest: A new 'rustdoc-flags' option is added. This allows us to
pass in the '--proc-macro-crate' flag in the absence of Cargo.

I've opened an additional PR to Cargo to support passing in this flag.
These two PRS can be merged in any order - the Cargo changes will not
take effect until the 'cargo' submodule is updated in this repository.

Aaron1011 added a commit to Aaron1011/rust that referenced this pull request Aug 4, 2019

Serialize additional data for procedural macros
Split off from rust-lang#62855

This PR serializes the declaration `Span` and attributes for all
procedural macros. This allows Rustdoc to properly render doc comments
and source links when performing inlinig procedural macros across crates

@Aaron1011 Aaron1011 force-pushed the Aaron1011:feature/rustdoc-reexport-final branch from ef755fc to e38ddc4 Aug 4, 2019

@Aaron1011

This comment has been minimized.

Copy link
Contributor Author

commented Aug 4, 2019

@petrochenkov: I've split off the proc macro related changes into their own PR. Note that the tests for this PR will fail until that PR is merged.

@rust-highfive

This comment has been minimized.

Copy link
Collaborator

commented Aug 4, 2019

The job x86_64-gnu-llvm-6.0 of your PR failed (raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

Click to expand the log.
2019-08-04T21:10:39.9069354Z ##[command]git remote add origin https://github.com/rust-lang/rust
2019-08-04T21:10:39.9312043Z ##[command]git config gc.auto 0
2019-08-04T21:10:39.9374162Z ##[command]git config --get-all http.https://github.com/rust-lang/rust.extraheader
2019-08-04T21:10:39.9424258Z ##[command]git config --get-all http.proxy
2019-08-04T21:10:39.9571358Z ##[command]git -c http.extraheader="AUTHORIZATION: basic ***" fetch --force --tags --prune --progress --no-recurse-submodules --depth=2 origin +refs/heads/*:refs/remotes/origin/* +refs/pull/62855/merge:refs/remotes/pull/62855/merge
---
2019-08-04T21:11:13.0529102Z do so (now or later) by using -b with the checkout command again. Example:
2019-08-04T21:11:13.0529135Z 
2019-08-04T21:11:13.0529309Z   git checkout -b <new-branch-name>
2019-08-04T21:11:13.0529336Z 
2019-08-04T21:11:13.0529372Z HEAD is now at 045cbe7fb Merge e38ddc4773dbcb1f8624390ebb71213270f366da into f01b9f803b59f170f5dabaaa8aedc96abe45bfea
2019-08-04T21:11:13.0687310Z ##[section]Starting: Collect CPU-usage statistics in the background
2019-08-04T21:11:13.0689486Z ==============================================================================
2019-08-04T21:11:13.0689529Z Task         : Bash
2019-08-04T21:11:13.0689562Z Description  : Run a Bash script on macOS, Linux, or Windows
---
2019-08-04T22:10:44.0969139Z .................................................................................................... 1400/8827
2019-08-04T22:10:50.1387125Z .................................................................................................... 1500/8827
2019-08-04T22:11:02.9088236Z ....................................................................i...............i............... 1600/8827
2019-08-04T22:11:10.1395401Z .................................................................................................... 1700/8827
2019-08-04T22:11:25.5104843Z ......................................................iiiii......................................... 1800/8827
2019-08-04T22:11:37.0785447Z .................................................................................................... 2000/8827
2019-08-04T22:11:39.6342265Z .................................................................................................... 2100/8827
2019-08-04T22:11:42.9513693Z .................................................................................................... 2200/8827
2019-08-04T22:11:50.8300198Z .................................................................................................... 2300/8827
---
2019-08-04T22:15:38.4542732Z .................................................................................................... 5200/8827
2019-08-04T22:15:46.7682446Z .....................................................................i.............................. 5300/8827
2019-08-04T22:15:54.2151821Z .................................................................................................... 5400/8827
2019-08-04T22:16:01.1877253Z .................................................................................................... 5500/8827
2019-08-04T22:16:12.4092259Z ...............................................................ii...i..ii............i.............. 5600/8827
2019-08-04T22:16:33.7731948Z .................................................................................................... 5800/8827
2019-08-04T22:16:38.9085596Z .................................................................................................... 5900/8827
2019-08-04T22:16:38.9085596Z .................................................................................................... 5900/8827
2019-08-04T22:16:45.1421004Z ................................................................i..ii............................... 6000/8827
2019-08-04T22:17:14.8565186Z .................................................................................................... 6200/8827
2019-08-04T22:17:17.0076567Z .......i............................................................................................ 6300/8827
2019-08-04T22:17:19.0542313Z ...............................................................................i.................... 6400/8827
2019-08-04T22:17:21.6424845Z .................................................................................................... 6500/8827
---
2019-08-04T22:22:01.0209151Z  finished in 20.220
2019-08-04T22:22:01.0388435Z Check compiletest suite=codegen mode=codegen (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu)
2019-08-04T22:22:01.2059902Z 
2019-08-04T22:22:01.2060142Z running 146 tests
2019-08-04T22:22:04.4389300Z i....iii......iii..iiii....i............................i..i................i....i.........ii.i.i..i 100/146
2019-08-04T22:22:06.2708245Z iii..............i.........iii.i......ii......
2019-08-04T22:22:06.2709907Z 
2019-08-04T22:22:06.2713218Z  finished in 5.232
2019-08-04T22:22:06.2887258Z Check compiletest suite=codegen-units mode=codegen-units (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu)
2019-08-04T22:22:06.4444740Z 
---
2019-08-04T22:22:08.5098635Z  finished in 2.221
2019-08-04T22:22:08.5280709Z Check compiletest suite=assembly mode=assembly (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu)
2019-08-04T22:22:08.6897399Z 
2019-08-04T22:22:08.6898826Z running 9 tests
2019-08-04T22:22:08.6901121Z iiiiiiiii
2019-08-04T22:22:08.6901602Z 
2019-08-04T22:22:08.6901650Z  finished in 0.161
2019-08-04T22:22:08.7083942Z Check compiletest suite=incremental mode=incremental (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu)
2019-08-04T22:22:08.8733528Z 
---
2019-08-04T22:22:26.9084498Z  finished in 18.200
2019-08-04T22:22:26.9270804Z Check compiletest suite=debuginfo mode=debuginfo-gdb+lldb (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu)
2019-08-04T22:22:27.0904386Z 
2019-08-04T22:22:27.0904892Z running 122 tests
2019-08-04T22:22:49.9620711Z .iiiii...i.....i..i...i..i.i.i..i.ii..i.i.....i..i....i..........iiii..........i...ii...i.......ii.i 100/122
2019-08-04T22:22:54.4860635Z .i.i......iii.i.....ii
2019-08-04T22:22:54.4861405Z 
2019-08-04T22:22:54.4862160Z  finished in 27.559
2019-08-04T22:22:54.4870515Z Uplifting stage1 rustc (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu)
2019-08-04T22:22:54.4870786Z Copying stage2 rustc from stage1 (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu / x86_64-unknown-linux-gnu)
---
2019-08-04T22:30:22.8028924Z failures:
2019-08-04T22:30:22.8028992Z 
2019-08-04T22:30:22.8029477Z ---- [rustdoc] rustdoc/inline_cross/proc_macro.rs stdout ----
2019-08-04T22:30:22.8029511Z 
2019-08-04T22:30:22.8029553Z error: htmldocck failed!
2019-08-04T22:30:22.8029616Z status: exit code: 1
2019-08-04T22:30:22.8030123Z command: "/usr/bin/python2.7" "/checkout/src/etc/htmldocck.py" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/rustdoc/inline_cross/proc_macro" "/checkout/src/test/rustdoc/inline_cross/proc_macro.rs"
2019-08-04T22:30:22.8030401Z ------------------------------------------
2019-08-04T22:30:22.8030431Z 
2019-08-04T22:30:22.8030625Z ------------------------------------------
2019-08-04T22:30:22.8030666Z stderr:
2019-08-04T22:30:22.8030666Z stderr:
2019-08-04T22:30:22.8030878Z ------------------------------------------
2019-08-04T22:30:22.8030921Z 16: @has check failed
2019-08-04T22:30:22.8030961Z  `PATTERN` did not match
2019-08-04T22:30:22.8031181Z  // @has - 'Doc comment from the original crate'
2019-08-04T22:30:22.8031461Z Encountered 1 errors
2019-08-04T22:30:22.8031513Z 
2019-08-04T22:30:22.8031791Z ------------------------------------------
2019-08-04T22:30:22.8031820Z 
---
2019-08-04T22:30:22.8032528Z thread 'main' panicked at 'Some tests failed', src/tools/compiletest/src/main.rs:536:22
2019-08-04T22:30:22.8032584Z note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace.
2019-08-04T22:30:22.8039953Z 
2019-08-04T22:30:22.8040046Z 
2019-08-04T22:30:22.8041684Z command did not execute successfully: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-tools-bin/compiletest" "--compile-lib-path" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/lib" "--run-lib-path" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/lib/rustlib/x86_64-unknown-linux-gnu/lib" "--rustc-path" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/bin/rustc" "--rustdoc-path" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/bin/rustdoc" "--src-base" "/checkout/src/test/rustdoc" "--build-base" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/rustdoc" "--stage-id" "stage2-x86_64-unknown-linux-gnu" "--mode" "rustdoc" "--target" "x86_64-unknown-linux-gnu" "--host" "x86_64-unknown-linux-gnu" "--llvm-filecheck" "/usr/lib/llvm-6.0/bin/FileCheck" "--host-rustcflags" "-Crpath -O -Cdebuginfo=0 -Zunstable-options  -Lnative=/checkout/obj/build/x86_64-unknown-linux-gnu/native/rust-test-helpers" "--target-rustcflags" "-Crpath -O -Cdebuginfo=0 -Zunstable-options  -Lnative=/checkout/obj/build/x86_64-unknown-linux-gnu/native/rust-test-helpers" "--docck-python" "/usr/bin/python2.7" "--lldb-python" "/usr/bin/python2.7" "--gdb" "/usr/bin/gdb" "--quiet" "--llvm-version" "6.0.0\n" "--system-llvm" "--cc" "" "--cxx" "" "--cflags" "" "--llvm-components" "" "--llvm-cxxflags" "" "--adb-path" "adb" "--adb-test-dir" "/data/tmp/work" "--android-cross-path" "" "--color" "always"
2019-08-04T22:30:22.8042162Z 
2019-08-04T22:30:22.8042192Z 
2019-08-04T22:30:22.8048115Z failed to run: /checkout/obj/build/bootstrap/debug/bootstrap test
2019-08-04T22:30:22.8048375Z Build completed unsuccessfully in 1:13:15
2019-08-04T22:30:22.8048375Z Build completed unsuccessfully in 1:13:15
2019-08-04T22:30:24.7581533Z ##[error]Bash exited with code '1'.
2019-08-04T22:30:24.7663490Z ##[section]Starting: Checkout
2019-08-04T22:30:24.7665169Z ==============================================================================
2019-08-04T22:30:24.7665628Z Task         : Get sources
2019-08-04T22:30:24.7665698Z Description  : Get sources from a repository. Supports Git, TfsVC, and SVN repositories.

I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact @TimNN. (Feature Requests)

@taiki-e taiki-e referenced this pull request Aug 5, 2019

Open

Tracking issue for pin-project 0.4 #21

5 of 7 tasks complete

bors bot added a commit to taiki-e/pin-project that referenced this pull request Aug 5, 2019

Merge #18
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>

Aaron1011 added a commit to Aaron1011/rust that referenced this pull request Aug 13, 2019

Serialize additional data for procedural macros
Split off from rust-lang#62855

This PR serializes the declaration `Span` and attributes for all
procedural macros. This allows Rustdoc to properly render doc comments
and source links when performing inlinig procedural macros across crates
@gagan0723

This comment has been minimized.

Copy link
Member

commented Aug 13, 2019

This PR appears to be blocked on #63269

Aaron1011 added a commit to Aaron1011/rust that referenced this pull request Aug 15, 2019

Serialize additional data for procedural macros
Split off from rust-lang#62855

This PR serializes the declaration `Span` and attributes for all
procedural macros. This allows Rustdoc to properly render doc comments
and source links when performing inlinig procedural macros across crates

Aaron1011 added a commit to Aaron1011/rust that referenced this pull request Aug 16, 2019

Serialize additional data for procedural macros
Split off from rust-lang#62855

This PR serializes the declaration `Span` and attributes for all
procedural macros. This allows Rustdoc to properly render doc comments
and source links when performing inlinig procedural macros across crates

Aaron1011 added a commit to Aaron1011/rust that referenced this pull request Aug 16, 2019

Serialize additional data for procedural macros
Split off from rust-lang#62855

This PR serializes the declaration `Span` and attributes for all
procedural macros. This allows Rustdoc to properly render doc comments
and source links when performing inlinig procedural macros across crates

Aaron1011 added a commit to Aaron1011/rust that referenced this pull request Aug 16, 2019

Serialize additional data for procedural macros
Split off from rust-lang#62855

This PR deerializes the declaration `Span` and attributes for all
procedural macros from their underlying function definitions.
This allows Rustdoc to properly render doc comments
and source links when inlining procedural macros across crates

Aaron1011 added a commit to Aaron1011/rust that referenced this pull request Aug 16, 2019

Serialize additional data for procedural macros
Split off from rust-lang#62855

This PR deerializes the declaration `Span` and attributes for all
procedural macros from their underlying function definitions.
This allows Rustdoc to properly render doc comments
and source links when inlining procedural macros across crates

Aaron1011 added a commit to Aaron1011/rust that referenced this pull request Aug 16, 2019

Serialize additional data for procedural macros
Split off from rust-lang#62855

This PR deerializes the declaration `Span` and attributes for all
procedural macros from their underlying function definitions.
This allows Rustdoc to properly render doc comments
and source links when inlining procedural macros across crates

Centril added a commit to Centril/rust that referenced this pull request Aug 17, 2019

Rollup merge of rust-lang#63269 - Aaron1011:feature/proc-macro-data, …
…r=eddyb,petrochenkov

Serialize additional data for procedural macros

Split off from rust-lang#62855

This PR serializes the declaration `Span` and attributes for all
procedural macros. This allows Rustdoc to properly render doc comments
and source links when performing inlinig procedural macros across crates

Aaron1011 added a commit to Aaron1011/rust that referenced this pull request Aug 17, 2019

Serialize additional data for procedural macros
Split off from rust-lang#62855

This PR deerializes the declaration `Span` and attributes for all
procedural macros from their underlying function definitions.
This allows Rustdoc to properly render doc comments
and source links when inlining procedural macros across crates
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.