rustdoc: look for comments when scraping attributes/crates from doctests #56793

QuietMisdreavus · 2018-12-13T21:30:07Z

When scraping out crate-level attributes and extern crate statements, we wouldn't look for comments, so any presence of comments would shunt it and everything after it into "everything else". This could cause parsing issues when looking for fn main and extern crate my_crate later on, which would in turn cause rustdoc to incorrectly wrap a test with fn main when it already had one declared.

I took the opportunity to clean up the logic a little bit, but it would still benefit from a libsyntax-based loop like the fn main detection.

rust-highfive · 2018-12-13T21:30:11Z

r? @steveklabnik

(rust_highfive has picked a reviewer for you, use r? to override)

QuietMisdreavus · 2018-12-13T21:30:38Z

r? @rust-lang/rustdoc

rust-highfive · 2018-12-13T22:53:07Z

The job x86_64-gnu-llvm-5.0 of your PR failed on Travis (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.

travis_time:end:054a3ae8:start=1544736706037518611,finish=1544736765606403193,duration=59568884582
$ git checkout -qf FETCH_HEAD
travis_fold:end:git.checkout

Encrypted environment variables have been removed for security reasons.
See https://docs.travis-ci.com/user/pull-requests/#Pull-Requests-and-Security-Restrictions
$ export SCCACHE_BUCKET=rust-lang-ci-sccache2
$ export SCCACHE_REGION=us-west-1
Setting environment variables from .travis.yml
$ export IMAGE=x86_64-gnu-llvm-5.0
---
travis_time:start:test_codegen
Check compiletest suite=codegen mode=codegen (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu)
[00:55:09] 
[00:55:09] running 121 tests
[00:55:12] i..ii...iii..iiii.....i...i..........i..iii.............i.....i......ii...i..i.ii..............i...i 100/121
[00:55:13] i..ii.i.....iiii.....
[00:55:13] 
[00:55:13]  finished in 3.386
[00:55:13] travis_fold:end:test_codegen

---
travis_time:start:test_debuginfo
Check compiletest suite=debuginfo mode=debuginfo-both (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu)
[00:55:27] 
[00:55:27] running 119 tests
[00:55:50] .iiiii...i.....i..i...i..i.i..i.ii..i.....i..i....i..........iiii.........i.i....i...i.......ii.i.i. 100/119
[00:55:54] i......iii.i.....ii
[00:55:54] 
[00:55:54]  finished in 26.839
[00:55:54] travis_fold:end:test_debuginfo

---
[01:20:07]     Finished release [optimized] target(s) in 38.77s
[01:20:07]      Running build/x86_64-unknown-linux-gnu/stage2-tools/x86_64-unknown-linux-gnu/release/deps/rustdoc-5f554cec32a0a6e3
[01:20:07] 
[01:20:07] running 41 tests
[01:20:07] ..............................F.F........
[01:20:07] 
[01:20:07] ---- test::tests::make_test_fake_main stdout ----
[01:20:07] thread 'test::tests::make_test_fake_main' panicked at 'assertion failed: `(left == right)`
[01:20:07] thread 'test::tests::make_test_fake_main' panicked at 'assertion failed: `(left == right)`
[01:20:07]   left: `("#![allow(unused)]\n//Ceci n\'est pas une `fn main`\nfn main() {\nassert_eq!(2+2, 4);\n}", 2)`,
[01:20:07]  right: `("#![allow(unused)]\nfn main() {\n//Ceci n\'est pas une `fn main`\nassert_eq!(2+2, 4);\n}", 2)`', src/librustdoc/test.rs:1085:9
[01:20:07] ---- test::tests::make_test_issues_21299_33731 stdout ----
[01:20:07] thread 'test::tests::make_test_issues_21299_33731' panicked at 'assertion failed: `(left == right)`
[01:20:07] thread 'test::tests::make_test_issues_21299_33731' panicked at 'assertion failed: `(left == right)`
[01:20:07]   left: `("#![allow(unused)]\n// fn main\nfn main() {\nassert_eq!(2+2, 4);\n}", 2)`,
[01:20:07]  right: `("#![allow(unused)]\nfn main() {\n// fn main\nassert_eq!(2+2, 4);\n}", 2)`', src/librustdoc/test.rs:1134:9
[01:20:07] 
[01:20:07] 
[01:20:07] failures:
[01:20:07]     test::tests::make_test_fake_main
[01:20:07]     test::tests::make_test_fake_main
[01:20:07]     test::tests::make_test_issues_21299_33731
[01:20:07] 
[01:20:07] test result: FAILED. 39 passed; 2 failed; 0 ignored; 0 measured; 0 filtered out
[01:20:07] 
[01:20:07] error: test failed, to rerun pass '--lib'
[01:20:07] 
[01:20:07] 
[01:20:07] command did not execute successfully: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0/bin/cargo" "test" "--target" "x86_64-unknown-linux-gnu" "-j" "4" "--release" "--locked" "--color" "always" "--manifest-path" "/checkout/src/tools/rustdoc/Cargo.toml" "-p" "rustdoc:0.0.0" "--" "--quiet"
[01:20:07] 
[01:20:07] 
[01:20:07] failed to run: /checkout/obj/build/bootstrap/debug/bootstrap test
[01:20:07] Build completed unsuccessfully in 0:35:21
[01:20:07] Build completed unsuccessfully in 0:35:21
[01:20:07] Makefile:58: recipe for target 'check' failed
[01:20:07] make: *** [check] Error 1
The command "stamp sh -x -c "$RUN_SCRIPT"" exited with 2.
travis_time:start:0089f355
$ date && (curl -fs --head https://google.com | grep ^Date: | sed 's/Date: //g' || true)
Thu Dec 13 22:53:00 UTC 2018

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)

GuillaumeGomez · 2018-12-14T10:13:08Z

src/librustdoc/test.rs

+                    trimline.chars().all(|c| c.is_whitespace()) ||
+                    (trimline.starts_with("//") && !trimline.starts_with("///"))
+                {
+                    state = PartitionState::Attrs;


Instead if setting the value everytime, why not going into a more functional way? Like:

state = if trimline.starts_with("#![") || trimline.chars().all(|c| c.is_whitespace()) || (trimline.starts_with("//") && !trimline.starts_with("///")) { PartitionState::Attrs } else if trimline.starts_with("extern crate") || trimline.starts_with("#[macro_use] extern crate") { PartitionState::Crates } else { PartitionState::Other };

Seems no different to me, but i'll make the change when i fix the tests.

GuillaumeGomez · 2018-12-14T10:13:21Z

src/librustdoc/test.rs

+                    trimline.chars().all(|c| c.is_whitespace()) ||
+                    (trimline.starts_with("//") && !trimline.starts_with("///"))
+                {
+                    state = PartitionState::Crates;


GuillaumeGomez · 2018-12-14T10:51:12Z

src/test/rustdoc/comment-in-doctest.rs

+// https://github.com/rust-lang/rust/issues/56727
+
+//! ```
+//! // crate: proc-macro-test


I don't understand this comment. What is it for?

I tried to copy the reduced example in #56727 as closely as i could without actually bringing in the stm32f30x crate. The comment is important because that's what breaks the parser right now.

QuietMisdreavus · 2018-12-14T15:21:17Z

Updated.

GuillaumeGomez · 2018-12-15T11:39:24Z

Thanks!

@bors: r+

bors · 2018-12-15T11:39:25Z

📌 Commit 8faaef6 has been approved by GuillaumeGomez

…GuillaumeGomez rustdoc: look for comments when scraping attributes/crates from doctests Fixes rust-lang#56727 When scraping out crate-level attributes and `extern crate` statements, we wouldn't look for comments, so any presence of comments would shunt it and everything after it into "everything else". This could cause parsing issues when looking for `fn main` and `extern crate my_crate` later on, which would in turn cause rustdoc to incorrectly wrap a test with `fn main` when it already had one declared. I took the opportunity to clean up the logic a little bit, but it would still benefit from a libsyntax-based loop like the `fn main` detection.

@ghost

Rollup of 20 pull requests Successful merges: - #53506 (Documentation for impl From for AtomicBool and other Atomic types) - #56343 (Remove not used mod) - #56439 (Clearer error message for dead assign) - #56640 (Add FreeBSD unsigned char platforms to std::os::raw) - #56648 (Fix BTreeMap UB) - #56672 (Document time of back operations of a Linked List) - #56706 (Make `const unsafe fn` bodies `unsafe`) - #56742 (infer: remove Box from a returned Iterator) - #56761 (Suggest using `.display()` when trying to print a `Path`) - #56781 (Update LLVM submodule) - #56789 (rustc: Add an unstable `simd_select_bitmask` intrinsic) - #56790 (Make RValue::Discriminant a normal Shallow read) - #56793 (rustdoc: look for comments when scraping attributes/crates from doctests) - #56826 (rustc: Add the `cmpxchg16b` target feature on x86/x86_64) - #56832 (std: Use `rustc_demangle` from crates.io) - #56844 (Improve CSS rule) - #56850 (Fixed issue with using `Self` ctor in typedefs) - #56855 (Remove u8 cttz hack) - #56857 (Fix a small mistake regarding NaNs in a deprecation message) - #56858 (Fix doc of `std::fs::canonicalize`) Failed merges: - #56741 (treat ref-to-raw cast like a reborrow: do a special kind of retag) r? @ghost

carols10cents · 2019-01-20T18:42:23Z

Nominating for backporting to stable if a point release to 1.32.0 happens for another reason, see #57767. Probably not worth backporting and releasing on its own. This fix is in 1.33.0-beta.1 so 🤷‍♀️

QuietMisdreavus · 2019-01-20T19:45:38Z

I don't know how common that issue is, but since it's happening to the Book, it's probably worth considering. I'm willing to sign off on the backport, but this is my PR, so i'd rather someone else from @rust-lang/rustdoc be the one to add the label. (But i also feel the same that it's probably not worth including unless there are other things that need a stable backport.)

QuietMisdreavus · 2019-01-21T20:52:31Z

Adding the stable-accepted label, since i learned that it's still up to the core/release teams to decide to create the release in the first place. If someone on Rustdoc team wants to object, we still have time.

add crates to the final doctest

22de23e

rust-highfive assigned steveklabnik Dec 13, 2018

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Dec 13, 2018

rust-highfive unassigned steveklabnik Dec 13, 2018

QuietMisdreavus mentioned this pull request Dec 13, 2018

rustdoc 2015 regression: doc test fails (unresolved import) if it contains a doc comment #56727

Closed

GuillaumeGomez reviewed Dec 14, 2018

View reviewed changes

QuietMisdreavus added 2 commits December 14, 2018 09:17

include comments in doctest partition logic

ea3078d

add test for parsing comments in doctest headers

8faaef6

QuietMisdreavus force-pushed the better-doctests branch from dc409fc to 8faaef6 Compare December 14, 2018 15:20

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 Dec 15, 2018

Centril mentioned this pull request Dec 16, 2018

Rollup of 22 pull requests #56873

Closed

Centril mentioned this pull request Dec 16, 2018

Rollup of 20 pull requests #56875

Merged

bors merged commit 8faaef6 into rust-lang:master Dec 16, 2018

QuietMisdreavus deleted the better-doctests branch December 17, 2018 20:22

ehuss mentioned this pull request Jan 20, 2019

rustdoc --test that passes with Rust 1.31.0 and fails with Rust 1.32.0, involving use paths #57767

Closed

carols10cents added the stable-nominated Nominated for backporting to the compiler in the stable channel. label Jan 20, 2019

QuietMisdreavus added the stable-accepted Accepted for backporting to the compiler in the stable channel. label Jan 21, 2019

pietroalbini removed stable-accepted Accepted for backporting to the compiler in the stable channel. labels Feb 20, 2019

Mark-Simulacrum removed the stable-nominated Nominated for backporting to the compiler in the stable channel. label Feb 27, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

rustdoc: look for comments when scraping attributes/crates from doctests #56793

rustdoc: look for comments when scraping attributes/crates from doctests #56793

QuietMisdreavus commented Dec 13, 2018

rust-highfive commented Dec 13, 2018

QuietMisdreavus commented Dec 13, 2018

rust-highfive commented Dec 13, 2018

GuillaumeGomez Dec 14, 2018

QuietMisdreavus Dec 14, 2018

GuillaumeGomez Dec 14, 2018

GuillaumeGomez Dec 14, 2018

QuietMisdreavus Dec 14, 2018

GuillaumeGomez Dec 15, 2018

QuietMisdreavus commented Dec 14, 2018

GuillaumeGomez commented Dec 15, 2018

bors commented Dec 15, 2018

carols10cents commented Jan 20, 2019 •

edited

Loading

QuietMisdreavus commented Jan 20, 2019

QuietMisdreavus commented Jan 21, 2019

rustdoc: look for comments when scraping attributes/crates from doctests #56793

rustdoc: look for comments when scraping attributes/crates from doctests #56793

Conversation

QuietMisdreavus commented Dec 13, 2018

rust-highfive commented Dec 13, 2018

QuietMisdreavus commented Dec 13, 2018

rust-highfive commented Dec 13, 2018

GuillaumeGomez Dec 14, 2018

Choose a reason for hiding this comment

QuietMisdreavus Dec 14, 2018

Choose a reason for hiding this comment

GuillaumeGomez Dec 14, 2018

Choose a reason for hiding this comment

GuillaumeGomez Dec 14, 2018

Choose a reason for hiding this comment

QuietMisdreavus Dec 14, 2018

Choose a reason for hiding this comment

GuillaumeGomez Dec 15, 2018

Choose a reason for hiding this comment

QuietMisdreavus commented Dec 14, 2018

GuillaumeGomez commented Dec 15, 2018

bors commented Dec 15, 2018

carols10cents commented Jan 20, 2019 • edited Loading

QuietMisdreavus commented Jan 20, 2019

QuietMisdreavus commented Jan 21, 2019

carols10cents commented Jan 20, 2019 •

edited

Loading