Remove most "```ignore" doc tests. #42777

kennytm · 2017-06-20T18:19:16Z

Unconditional ```ignore doc tests lead to outdated examples (e.g. #42729 (comment)). This PR tries to change all existing ```ignore tests into one of the following:

Add import and declarations to ensure the code is run-pass
If the code is not Rust, change to ```text/```sh/```json/```dot
If the code is expected compile-fail, change to ```compile_fail
If the code is expected run-fail, change to ```should_panic
If the code can type-check but cannot link/run, change to ```no_run
Otherwise, add an explanation after the ```ignore

The --explain handling is changed to cope with hidden lines from the error index.

Tidy is changed to reject any unexplained ```ignore and ```rust,ignore.

rust-highfive · 2017-06-20T18:19:20Z

r? @arielb1

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

kennytm · 2017-06-20T18:19:42Z

Additional info:

There are 27 remaining ```ignore tests, divided into 6 categories:
- code demo which looks better with syntax highlighting only (10 instances)
- private modules (6 instances)
- doc test with include!(...) (3 instances)
- doc test with macro exporting across multiple crates (6 instances)
- compile_fail cannot check SIMD failure (1 instance)
- doc test cannot "fail on Linux/Windows/etc but pass on macOS" (1 instance)
There are 8 error codes which are no longer emitted. I just added
#### Note: this error code is no longer emitted by the compiler., but I'm
not sure if they should be removed instead.
- E0073 (indirectly recursive struct)
- E0074 (#[simd] on generic tuple struct)
- E0082 (C-like enum overflows — apparantly merged into E0080)
- E0139 (transmute to wrapper type)
- E0193 (where clause without generic type parameters)
- E0398 (&Box<Trait> means &Box<Trait+'static> after Rust 1.3)
- E0447 (pub inside function)
E0329 (associated const of generic parameter) is just commented out since the description explicitly said "This is not supported yet".

steveklabnik · 2017-06-20T18:55:37Z

Amazing! I have always wanted to do this. Thank you so much!

I can't review right this moment, but was too excited not to leave a message 💯

Mark-Simulacrum · 2017-06-20T20:17:08Z

Looks like Travis failed.

[00:50:41] failures:
[00:50:41] 
[00:50:41] ---- /checkout/obj/build/x86_64-unknown-linux-gnu/test/error-index.md - Rust_Compiler_Error_Index (line 7427) stdout ----
[00:50:41] 	error[E0455]: native frameworks are only available on macOS targets
[00:50:41]  --> <anon>:2:1
[00:50:41]   |
[00:50:41] 2 | #[link(name = "FooCoreServices", kind = "framework")] extern {}
[00:50:41]   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
[00:50:41] 
[00:50:41] error: aborting due to previous error(s)
[00:50:41] 
[00:50:41] thread 'rustc' panicked at 'Box<Any>', /checkout/src/librustc/session/mod.rs:218
[00:50:41] 
[00:50:41] ---- /checkout/obj/build/x86_64-unknown-linux-gnu/test/error-index.md - Rust_Compiler_Error_Index (line 7887) stdout ----
[00:50:41] 	error[E0455]: native frameworks are only available on macOS targets
[00:50:41]  --> <anon>:2:1
[00:50:41]   |
[00:50:41] 2 | #[link(name = "FooCoreServices", kind = "framework")] extern {}
[00:50:41]   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
[00:50:41] 
[00:50:41] error: aborting due to previous error(s)
[00:50:41] 
[00:50:41] thread 'rustc' panicked at 'Box<Any>', /checkout/src/librustc/session/mod.rs:218
[00:50:41] 
[00:50:41] 
[00:50:41] failures:
[00:50:41]     /checkout/obj/build/x86_64-unknown-linux-gnu/test/error-index.md - Rust_Compiler_Error_Index (line 7427)
[00:50:41]     /checkout/obj/build/x86_64-unknown-linux-gnu/test/error-index.md - Rust_Compiler_Error_Index (line 7887)
[00:50:41] 
[00:50:41] test result: FAILED. 1138 passed; 2 failed; 18 ignored; 0 measured; 0 filtered out

kennytm · 2017-06-20T20:28:54Z

@Mark-Simulacrum Thanks. Turned this back into ```ignore 🙁 unless there is a way to ignore the test just for macOS.

estebank

Great work!

Nitpicks inline.

estebank · 2017-06-20T23:35:32Z

src/libcore/ptr.rs

-    /// let val: *const u8 = &10u8 as *const u8;
+    /// ```
+    /// let val = 10u8;
+    /// let ptr: *const u8 = &val as *const u8;


What's the reasoning behind this change?

@estebank Reverted. Thought this was ```ignored because &10u8 would be dropped too early (it was ```ignored to avoid introducing a #[feature] to the example when .as_ref() was unstable).

estebank · 2017-06-20T23:35:57Z

src/libcore/ptr.rs

-    /// let val: *mut u8 = &mut 10u8 as *mut u8;
+    /// ```
+    /// let mut val = 10u8;
+    /// let ptr: *mut u8 = &mut val as *mut u8;


Same as above.

@estebank Reverted.

estebank · 2017-06-20T23:39:27Z

src/librustc/diagnostics.rs

+# use std::ptr;
+# let v = Some("value");
+# type SomeType = &'static [u8];
+# unsafe {
 ptr::read(&v as *const _ as *const SomeType) // `v` transmuted to `SomeType`


Should the unsafe block visible to the documentation?

@estebank Made visible.

estebank · 2017-06-20T23:45:05Z

src/librustc_borrowck/borrowck/check_loans.rs

    /// p.x = 22; // ok, even though `p` is uninitialized
    ///
-    /// let p: Box<Point>;
+    /// let mut p: Box<Point>;
    /// (*p).x = 22; // not ok, p is uninitialized, can't deref


May be these two examples should be separated into a code block that succeeds (above) and one that fails with E0381 (bellow).

@estebank Separated into 3 code blocks.

estebank · 2017-06-20T23:47:48Z

src/librustc_borrowck/diagnostics.rs

@@ -966,7 +972,7 @@ fn main() {
    };
    let borrowed = &mut cave;

-    borrowed.knight.nothing_is_true(); // E0507
+//  borrowed.knight.nothing_is_true(); // E0507
    mem::replace(&mut borrowed.knight, TheDarkKnight).nothing_is_true(); // ok!
 }


Separate the success and failure cases to catch any regressions. For the second block it'll need a bunch of duplicated hidden code :-/

@estebank Reworded and separated into 2 examples.

estebank · 2017-06-20T23:54:36Z

src/librustc_typeck/diagnostics.rs

@@ -2935,12 +2966,13 @@ impl<T, U> CoerceUnsized<MyType<U>> for MyType<T>
 [`CoerceUnsized`]: https://doc.rust-lang.org/std/ops/trait.CoerceUnsized.html
 "##,

+/*


Why is this description commented out? Add a reason to the comment to avoid confusion.

@estebank Added a comment to explain why this is hidden at the moment.

kennytm · 2017-06-21T08:44:48Z

@estebank Thanks! All addressed.

estebank · 2017-06-21T17:24:08Z

@bors r+ rollup

bors · 2017-06-21T17:24:10Z

📌 Commit 7edba12 has been approved by estebank

…bank Remove most "```ignore" doc tests. Unconditional ` ```ignore ` doc tests lead to outdated examples (e.g. rust-lang#42729 (comment)). This PR tries to change all existing ` ```ignore ` tests into one of the following: * Add import and declarations to ensure the code is run-pass * If the code is not Rust, change to ` ```text `/` ```sh `/` ```json `/` ```dot ` * If the code is expected compile-fail, change to ` ```compile_fail ` * If the code is expected run-fail, change to ` ```should_panic ` * If the code can type-check but cannot link/run, change to ` ```no_run ` * Otherwise, add an explanation after the ` ```ignore ` The `--explain` handling is changed to cope with hidden lines from the error index. Tidy is changed to reject any unexplained ` ```ignore ` and ` ```rust,ignore `.

kennytm · 2017-06-22T05:20:11Z

@estebank #[cfg]'ed out Windows on the E0060 in 5dd6538 due to AppVeyor failure on rollup #42817. (I can't repro it locally, but I don't think the other 6 PRs cause the linker issue.)

estebank · 2017-06-22T17:50:55Z

@bors r-

estebank · 2017-06-22T17:51:04Z

@bors r+ rollup

bors · 2017-06-22T17:51:05Z

📌 Commit 5dd6538 has been approved by estebank

estebank · 2017-06-22T17:56:30Z

@kennytm can you file a ticket for that? We need to figure out what the actual problem is, specially because we don't want to have documentation that is incorrect or misleading. It should contain code to reproduce the problem under Windows, and a link to the workaround commit in this PR. Probably the output of AppVeyor too:

failures:
---- C:\projects\rust\build\x86_64-pc-windows-msvc\test\error-index.md - Rust_Compiler_Error_Index (line 1285) stdout ----
	error: linking with `link.exe` failed: exit code: 1120
  |
  = note: "C:\\Program Files (x86)\\Microsoft Visual Studio 14.0\\VC\\bin\\amd64\\link.exe" "/NOLOGO" "/NXCOMPAT" "/LIBPATH:C:\\projects\\rust\\build\\x86_64-pc-windows-msvc\\stage2\\lib\\rustlib\\x86_64-pc-windows-msvc\\lib" "C:\\Users\\appveyor\\AppData\\Local\\Temp\\1\\rustdoctest.RWH8fWEAi4XJ\\rust_out.0.o" "/OUT:C:\\Users\\appveyor\\AppData\\Local\\Temp\\1\\rustdoctest.RWH8fWEAi4XJ\\rust_out.exe" "/OPT:REF,NOICF" "/DEBUG" "/LIBPATH:C:\\projects\\rust\\build\\x86_64-pc-windows-msvc\\stage2\\lib\\rustlib\\x86_64-pc-windows-msvc\\lib" "/LIBPATH:C:\\projects\\rust\\build\\x86_64-pc-windows-msvc\\stage2\\lib\\rustlib\\x86_64-pc-windows-msvc\\lib" "std-c5f066fd9d9c9409.dll.lib" "C:\\projects\\rust\\build\\x86_64-pc-windows-msvc\\stage2\\lib\\rustlib\\x86_64-pc-windows-msvc\\lib\\libcompiler_builtins-096fad10dd181792.rlib" "advapi32.lib" "ws2_32.lib" "userenv.lib" "shell32.lib" "msvcrt.lib"
  = note: rust_out.0.o : error LNK2019: unresolved external symbol printf referenced in function _ZN8rust_out4main17ha22022d8940e3e88E
          C:\Users\appveyor\AppData\Local\Temp\1\rustdoctest.RWH8fWEAi4XJ\rust_out.exe : fatal error LNK1120: 1 unresolved externals
          
error: aborting due to previous error(s)
thread 'rustc' panicked at 'Box<Any>', src\librustc_errors\lib.rs:512
thread 'rustc' panicked at 'couldn't compile the test', src\librustdoc\test.rs:278
---- C:\projects\rust\build\x86_64-pc-windows-msvc\test\error-index.md - Rust_Compiler_Error_Index (line 1355) stdout ----
	error: linking with `link.exe` failed: exit code: 1120
  |
  = note: "C:\\Program Files (x86)\\Microsoft Visual Studio 14.0\\VC\\bin\\amd64\\link.exe" "/NOLOGO" "/NXCOMPAT" "/LIBPATH:C:\\projects\\rust\\build\\x86_64-pc-windows-msvc\\stage2\\lib\\rustlib\\x86_64-pc-windows-msvc\\lib" "C:\\Users\\appveyor\\AppData\\Local\\Temp\\1\\rustdoctest.pJgryzUQ8mFi\\rust_out.0.o" "/OUT:C:\\Users\\appveyor\\AppData\\Local\\Temp\\1\\rustdoctest.pJgryzUQ8mFi\\rust_out.exe" "/OPT:REF,NOICF" "/DEBUG" "/LIBPATH:C:\\projects\\rust\\build\\x86_64-pc-windows-msvc\\stage2\\lib\\rustlib\\x86_64-pc-windows-msvc\\lib" "/LIBPATH:C:\\projects\\rust\\build\\x86_64-pc-windows-msvc\\stage2\\lib\\rustlib\\x86_64-pc-windows-msvc\\lib" "std-c5f066fd9d9c9409.dll.lib" "C:\\projects\\rust\\build\\x86_64-pc-windows-msvc\\stage2\\lib\\rustlib\\x86_64-pc-windows-msvc\\lib\\libcompiler_builtins-096fad10dd181792.rlib" "advapi32.lib" "ws2_32.lib" "userenv.lib" "shell32.lib" "msvcrt.lib"
  = note: rust_out.0.o : error LNK2019: unresolved external symbol printf referenced in function _ZN8rust_out4main17ha22022d8940e3e88E
          C:\Users\appveyor\AppData\Local\Temp\1\rustdoctest.pJgryzUQ8mFi\rust_out.exe : fatal error LNK1120: 1 unresolved externals
          
error: aborting due to previous error(s)
thread 'rustc' panicked at 'Box<Any>', src\librustc_errors\lib.rs:512
thread 'rustc' panicked at 'couldn't compile the test', src\librustdoc\test.rs:278
failures:
    C:\projects\rust\build\x86_64-pc-windows-msvc\test\error-index.md - Rust_Compiler_Error_Index (line 1285)
    C:\projects\rust\build\x86_64-pc-windows-msvc\test\error-index.md - Rust_Compiler_Error_Index (line 1355)
test result: FAILED. 1138 passed; 2 failed; 20 ignored; 0 measured; 0 filtered out

kennytm · 2017-06-22T20:22:10Z

@estebank Found the solution from #42830. Rebased. (My local compiler was VC 2010 which is why I couldn't reproduce it)

estebank · 2017-06-22T21:20:42Z

@bors r-

estebank · 2017-06-22T21:21:10Z

@bors r+

bors · 2017-06-22T21:21:11Z

📌 Commit 513b962 has been approved by estebank

Mark-Simulacrum · 2017-06-22T21:52:27Z

Canceled try build.

brson · 2017-06-22T22:28:23Z

Wowee. Super cleanup patch @kennytm.

Mark-Simulacrum · 2017-06-22T22:31:52Z

@bors r+ retry

bors · 2017-06-22T22:31:53Z

💡 This pull request was already approved, no need to approve it again.

There's another pull request that is currently being tested, blocking this pull request: New error codes #42614

bors · 2017-06-22T22:31:54Z

📌 Commit 513b962 has been approved by Mark-Simulacrum

bors · 2017-06-22T22:32:02Z

⌛ Testing commit 513b962 with merge 5752d8b...

Remove most "```ignore" doc tests. Unconditional ` ```ignore ` doc tests lead to outdated examples (e.g. #42729 (comment)). This PR tries to change all existing ` ```ignore ` tests into one of the following: * Add import and declarations to ensure the code is run-pass * If the code is not Rust, change to ` ```text `/` ```sh `/` ```json `/` ```dot ` * If the code is expected compile-fail, change to ` ```compile_fail ` * If the code is expected run-fail, change to ` ```should_panic ` * If the code can type-check but cannot link/run, change to ` ```no_run ` * Otherwise, add an explanation after the ` ```ignore ` The `--explain` handling is changed to cope with hidden lines from the error index. Tidy is changed to reject any unexplained ` ```ignore ` and ` ```rust,ignore `.

Mark-Simulacrum · 2017-06-22T22:32:26Z

ah, whatever, I guess try actually has to pass for bors to be happy..

@bors r=estebank

bors · 2017-06-22T22:32:27Z

💡 This pull request was already approved, no need to approve it again.

This pull request is currently being tested. If there's no response from the continuous integration service, you may use retry to trigger a build again.

bors · 2017-06-22T22:32:27Z

📌 Commit 513b962 has been approved by estebank

…bank Remove most "```ignore" doc tests. Unconditional ` ```ignore ` doc tests lead to outdated examples (e.g. rust-lang#42729 (comment)). This PR tries to change all existing ` ```ignore ` tests into one of the following: * Add import and declarations to ensure the code is run-pass * If the code is not Rust, change to ` ```text `/` ```sh `/` ```json `/` ```dot ` * If the code is expected compile-fail, change to ` ```compile_fail ` * If the code is expected run-fail, change to ` ```should_panic ` * If the code can type-check but cannot link/run, change to ` ```no_run ` * Otherwise, add an explanation after the ` ```ignore ` The `--explain` handling is changed to cope with hidden lines from the error index. Tidy is changed to reject any unexplained ` ```ignore ` and ` ```rust,ignore `.

Mark-Simulacrum · 2017-06-23T00:33:54Z

@bors r-

tidy check (x86_64-pc-windows-msvc)
tidy error: C:\projects\rust\src\librustc_typeck\diagnostics.rs:4443: unexplained "```ignore" doctest; try one:
* make the test actually pass, by adding necessary imports and declarations, or
* use "```text", if the code is not Rust code, or
* use "```compile_fail,Ennnn", if the code is expected to fail at compile time, or
* use "```should_panic", if the code is expected to fail at run time, or
* use "```no_run", if the code should type-check but not necessary linkable/runnable, or
* explain it like "```ignore (cannot-test-this-because-xxxx)", if the annotation cannot be avoided.
tidy error: C:\projects\rust\src\librustc_typeck\diagnostics.rs:4450: unexplained "```ignore" doctest; try one:
* make the test actually pass, by adding necessary imports and declarations, or
* use "```text", if the code is not Rust code, or
* use "```compile_fail,Ennnn", if the code is expected to fail at compile time, or
* use "```should_panic", if the code is expected to fail at run time, or
* use "```no_run", if the code should type-check but not necessary linkable/runnable, or
* explain it like "```ignore (cannot-test-this-because-xxxx)", if the annotation cannot be avoided.
some tidy checks failed

bors · 2017-06-23T01:43:20Z

☀️ Test successful - status-travis
State: approved= try=False

Replaced by adding extra imports, adding hidden code (`# ...`), modifying examples to be runnable (sorry Homura), specifying non-Rust code, and converting to should_panic, no_run, or compile_fail. Remaining "```ignore"s received an explanation why they are being ignored.

…ocks.

kennytm · 2017-06-23T07:37:09Z

@Mark-Simulacrum Rebased to remove the two ```ignore from E0615.

Mark-Simulacrum · 2017-06-23T12:01:47Z

@bors r=estebank

bors · 2017-06-23T12:01:48Z

📌 Commit 9addd3b has been approved by estebank

…bank Remove most "```ignore" doc tests. Unconditional ` ```ignore ` doc tests lead to outdated examples (e.g. rust-lang#42729 (comment)). This PR tries to change all existing ` ```ignore ` tests into one of the following: * Add import and declarations to ensure the code is run-pass * If the code is not Rust, change to ` ```text `/` ```sh `/` ```json `/` ```dot ` * If the code is expected compile-fail, change to ` ```compile_fail ` * If the code is expected run-fail, change to ` ```should_panic ` * If the code can type-check but cannot link/run, change to ` ```no_run ` * Otherwise, add an explanation after the ` ```ignore ` The `--explain` handling is changed to cope with hidden lines from the error index. Tidy is changed to reject any unexplained ` ```ignore ` and ` ```rust,ignore `.

Rollup of 8 pull requests - Successful merges: #42777, #42783, #42787, #42821, #42822, #42825, #42829, #42833 - Failed merges:

rust-highfive assigned arielb1 Jun 20, 2017

kennytm force-pushed the kill-ignore-doctest branch from 0712a27 to 524a892 Compare June 20, 2017 20:27

arielb1 added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jun 20, 2017

estebank approved these changes Jun 21, 2017

View reviewed changes

kennytm force-pushed the kill-ignore-doctest branch from 524a892 to 7edba12 Compare June 21, 2017 08:40

frewsxcv mentioned this pull request Jun 22, 2017

Rollup of 8 pull requests #42815

Closed

frewsxcv mentioned this pull request Jun 22, 2017

Rollup of 7 pull requests #42817

Closed

kennytm mentioned this pull request Jun 22, 2017

[rustbuild] cannot link to printf with MSVC on AppVeyor #42830

Closed

kennytm force-pushed the kill-ignore-doctest branch 2 times, most recently from 5612473 to 513b962 Compare June 22, 2017 20:19

Mark-Simulacrum 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 Jun 22, 2017

Mark-Simulacrum mentioned this pull request Jun 22, 2017

Rollup of 9 pull requests #42845

Closed

kennytm added 3 commits June 23, 2017 15:31

Modify --explain to handle hidden code (# ...) and indented code bl…

2c89165

…ocks.

Added a tidy check to disallow "ignore" and "rust,ignore".

9addd3b

kennytm force-pushed the kill-ignore-doctest branch from 513b962 to 9addd3b Compare June 23, 2017 07:33

Mark-Simulacrum mentioned this pull request Jun 23, 2017

Rollup of 8 pull requests #42856

Merged

bors added a commit that referenced this pull request Jun 23, 2017

Auto merge of #42856 - Mark-Simulacrum:rollup, r=Mark-Simulacrum

229d0d3

Rollup of 8 pull requests - Successful merges: #42777, #42783, #42787, #42821, #42822, #42825, #42829, #42833 - Failed merges:

bors merged commit 9addd3b into rust-lang:master Jun 23, 2017

kennytm deleted the kill-ignore-doctest branch June 26, 2017 15:24

Remove most "```ignore" doc tests. #42777

Remove most "```ignore" doc tests. #42777

Conversation

kennytm commented Jun 20, 2017

rust-highfive commented Jun 20, 2017

kennytm commented Jun 20, 2017 • edited Loading

steveklabnik commented Jun 20, 2017

Mark-Simulacrum commented Jun 20, 2017

kennytm commented Jun 20, 2017

estebank left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kennytm commented Jun 21, 2017

estebank commented Jun 21, 2017

bors commented Jun 21, 2017

kennytm commented Jun 22, 2017

estebank commented Jun 22, 2017

estebank commented Jun 22, 2017

bors commented Jun 22, 2017

estebank commented Jun 22, 2017 • edited Loading

kennytm commented Jun 22, 2017 • edited Loading

estebank commented Jun 22, 2017

estebank commented Jun 22, 2017

bors commented Jun 22, 2017

Mark-Simulacrum commented Jun 22, 2017

brson commented Jun 22, 2017

Mark-Simulacrum commented Jun 22, 2017

bors commented Jun 22, 2017

bors commented Jun 22, 2017

bors commented Jun 22, 2017

Mark-Simulacrum commented Jun 22, 2017

bors commented Jun 22, 2017

bors commented Jun 22, 2017

Mark-Simulacrum commented Jun 23, 2017

bors commented Jun 23, 2017

kennytm commented Jun 23, 2017

Mark-Simulacrum commented Jun 23, 2017

bors commented Jun 23, 2017

kennytm commented Jun 20, 2017 •

edited

Loading

estebank commented Jun 22, 2017 •

edited

Loading

kennytm commented Jun 22, 2017 •

edited

Loading