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

Fix injected errors when running doctests on a crate named after a keyword #79775

Merged
merged 1 commit into from
Feb 13, 2021

Conversation

jyn514
Copy link
Member

@jyn514 jyn514 commented Dec 6, 2020

Closes #79771

@jyn514 jyn514 added T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. A-doctests Area: Documentation tests, run by rustdoc labels Dec 6, 2020
@rust-highfive
Copy link
Collaborator

r? @GuillaumeGomez

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Dec 6, 2020
Comment on lines 1 to 6
#![crate_name = "mod"]
//! ```
//! // NOTE: requires that the literal string 'mod' appears in the doctest for
//! // the bug to appear
//! assert_eq!(1, 1);
//! ```
Copy link
Member Author

Choose a reason for hiding this comment

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

I don't know how to test this. Currently the test does nothing because it isn't running rustdoc with --test; if I add // compile-flags: --test it errors regardless of the fix because the aux metadata isn't built:

running 1 test
test src/test/rustdoc/doctest-keyword.rs - (line 3) ... FAILED

failures:

---- src/test/rustdoc/doctest-keyword.rs - (line 3) stdout ----
error[E0463]: can't find crate for `mod`
 --> src/test/rustdoc/doctest-keyword.rs:4:1
  |
4 | extern crate r#mod;
  | ^^^^^^^^^^^^^^^^^^^ can't find crate

I can't seem to find any other tests for this? @GuillaumeGomez do you have ideas?

Copy link
Member

Choose a reason for hiding this comment

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

You can use aux-build to achieve that I think, no?

Copy link
Member Author

Choose a reason for hiding this comment

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

Hmm, I'm not sure how. Normally aux-build looks in the auxiliary directory, so I tried using // aux-build:../doctest-keyword.rs to trick it into building the current file. But it still seems to be breaking :/

running 6 tests
iiiiiF
failures:

---- [rustdoc] rustdoc/doctest-keyword.rs stdout ----


executing "/home/joshua/rustc/build/x86_64-unknown-linux-gnu/stage1/bin/rustc" "/home/joshua/rustc/src/test/rustdoc/auxiliary/../doctest-keyword.rs" "-Zthreads=1" "--target=x86_64-unknown-linux-gnu" "-C" "prefer-dynamic" "--out-dir" "/home/joshua/rustc/build/x86_64-unknown-linux-gnu/test/rustdoc/doctest-keyword/auxiliary" "-Crpath" "-O" "-Cdebuginfo=0" "-Zunstable-options" "-Lnative=/home/joshua/rustc/build/x86_64-unknown-linux-gnu/native/rust-test-helpers" "--test" "--crate-type" "dylib" "-L" "/home/joshua/rustc/build/x86_64-unknown-linux-gnu/test/rustdoc/doctest-keyword/auxiliary"
------stdout------------------------------

------stderr------------------------------

------------------------------------------
executing "/home/joshua/rustc/build/x86_64-unknown-linux-gnu/stage1/bin/rustdoc" "-L" "/home/joshua/rustc/build/x86_64-unknown-linux-gnu/stage1/lib/rustlib/x86_64-unknown-linux-gnu/lib" "-L" "/home/joshua/rustc/build/x86_64-unknown-linux-gnu/test/rustdoc/doctest-keyword/auxiliary" "-o" "/home/joshua/rustc/build/x86_64-unknown-linux-gnu/test/rustdoc/doctest-keyword" "/home/joshua/rustc/src/test/rustdoc/doctest-keyword.rs" "--test"
------stdout------------------------------

running 1 test
test src/test/rustdoc/doctest-keyword.rs - (line 7) ... FAILED

failures:

---- src/test/rustdoc/doctest-keyword.rs - (line 7) stdout ----
error[E0463]: can't find crate for `mod`
 --> src/test/rustdoc/doctest-keyword.rs:8:1
  |
4 | extern crate r#mod;
  | ^^^^^^^^^^^^^^^^^^^ can't find crate

error: aborting due to previous error

For more information about this error, try `rustc --explain E0463`.
Couldn't compile the test.

failures:
    src/test/rustdoc/doctest-keyword.rs - (line 7)

test result: FAILED. 0 passed; 1 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.02s

I did confirm that the aux file exists, but it doesn't seem to be picking up on it for some reason.

$ file /home/joshua/rustc/build/x86_64-unknown-linux-gnu/test/rustdoc/doctest-keyword/auxiliary/mod
/home/joshua/rustc/build/x86_64-unknown-linux-gnu/test/rustdoc/doctest-keyword/auxiliary/mod: ELF 64-bit LSB shared object, x86-64, version 1 (SYSV), dynamically linked, interpreter /lib64/ld-linux-x86-64.so.2, for GNU/Linux 3.2.0, BuildID[sha1]=df3038c014cbf237c458cb19f7571c62aa8b96f9, not stripped

@Mark-Simulacrum do you have ideas maybe?

Copy link
Member

@GuillaumeGomez GuillaumeGomez Dec 7, 2020

Choose a reason for hiding this comment

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

The problem here might come from the .rs file generated by rustdoc containing the test. Not sure how we link it then...

Copy link
Member Author

@jyn514 jyn514 Dec 7, 2020

Choose a reason for hiding this comment

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

@GuillaumeGomez shouldn't the generated .rs file be using the same -L aux dir that rustdoc will used normally? Or is it being ignored? That seems like a bug we should fix.

Here's how cargo does it for reference:

     Running `rustc --crate-name mod --edition=2018 src/lib.rs --error-format=json --json=diagnostic-rendered-ansi --crate-type lib --emit=dep-info,metadata,link -C embed-bitcode=no -C debuginfo=2 -C metadata=efd9726c342fe75e -C extra-filename=-efd9726c342fe75e --out-dir /home/joshua/test-rustdoc/doctest/target/debug/deps -C incremental=/home/joshua/test-rustdoc/doctest/target/debug/incremental -L dependency=/home/joshua/test-rustdoc/doctest/target/debug/deps -C link-arg=-fuse-ld=lld`
    Finished test [unoptimized + debuginfo] target(s) in 0.05s
   Doc-tests mod
     Running `rustdoc --edition=2018 --crate-type lib --test /home/joshua/test-rustdoc/doctest/src/lib.rs --crate-name mod -L dependency=/home/joshua/test-rustdoc/doctest/target/debug/deps -L dependency=/home/joshua/test-rustdoc/doctest/target/debug/deps --extern mod=/home/joshua/test-rustdoc/doctest/target/debug/deps/libmod-efd9726c342fe75e.rlib -C embed-bitcode=no`

It's passing --extern mod=/home/joshua/test-rustdoc/doctest/target/debug/deps/libmod-efd9726c342fe75e.rlib where compiletest only passes -L - maybe that's the difference?

Copy link
Member Author

Choose a reason for hiding this comment

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

I think it's unlikely this will regress, since someone would have to change it intentionally. Do you think it makes sense to add the fix before figuring out how to test it?

src/librustdoc/doctest.rs Outdated Show resolved Hide resolved
@JohnCSimon JohnCSimon added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jan 11, 2021
@camelid camelid added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jan 29, 2021
@jyn514
Copy link
Member Author

jyn514 commented Feb 11, 2021

@JohnCSimon this is waiting on review, not me.

ping @GuillaumeGomez do you have ideas for how to test this? If not, are you ok with merging the fix even without tests?

@jyn514 jyn514 added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Feb 11, 2021
@GuillaumeGomez
Copy link
Member

This isn't a bug with high priority, so I'd prefer to add the test alongside it if possible. We can take a look together if the aux-build isn't working for you if you want?

@jyn514
Copy link
Member Author

jyn514 commented Feb 11, 2021

If you can take a look, I'd appreciate it. I don't know how to do this.

@GuillaumeGomez
Copy link
Member

Sure! Taking a look then. ;)

@GuillaumeGomez
Copy link
Member

Ok, so the problem here with the test is that we need the file to be compiled first to then be used as dependency, which cannot be done currently unfortunately in the rustdoc test suites... Example:

// name this file "foo.rs"

/// ```
/// let x = foo::foo();
/// ```
pub fn foo() {}

If you run rustdoc --test foo.rs, you'll get:

running 1 test
test foo.rs - foo (line 1) ... FAILED

failures:

---- foo.rs - foo (line 1) stdout ----
error[E0463]: can't find crate for `foo`
 --> foo.rs:0:1
  |
2 | extern crate foo;
  | ^^^^^^^^^^^^^^^^^ can't find crate

We need foo to be compiled so that it can be used as a dependency by rustdoc, which is currently not possible in any of the rustdoc test suites. So unfortunately, I don't see any easy way to test it...

So if you're fine with it @jyn514, please just remove the test and we'll r+ it.

@GuillaumeGomez
Copy link
Member

Thanks! r=me once CI is green.

@rust-log-analyzer

This comment has been minimized.

@jyn514
Copy link
Member Author

jyn514 commented Feb 11, 2021

Oh, well, after all that it turns out it gets tested anyway because we hard-code the generated code in a test lol

@GuillaumeGomez
Copy link
Member

Nice surprise haha!

@rust-log-analyzer

This comment has been minimized.

…yword

Unfortunately, this can't currently be tested. The problem is that we
need the file to be compiled first to then be used as dependency, which
cannot be done currently unfortunately in the rustdoc test suites.
Example:

```rust
// name this file "foo.rs"

/// ```
/// let x = foo::foo();
/// ```
pub fn foo() {}
```

If you run `rustdoc --test foo.rs`, you'll get:

```
running 1 test
test foo.rs - foo (line 1) ... FAILED

failures:

---- foo.rs - foo (line 1) stdout ----
error[E0463]: can't find crate for `foo`
 --> foo.rs:0:1
  |
2 | extern crate foo;
  | ^^^^^^^^^^^^^^^^^ can't find crate
```

If a test were possible, it would look something like

 ````rust
 #![crate_name = "mod"]
 #![crate_type = "lib"]
 //! ```
 //! // NOTE: requires that the literal string 'mod' appears in the doctest for
 //! // the bug to appear
 //! assert_eq!(1, 1);
 //! ```
 ````
@GuillaumeGomez
Copy link
Member

Seems like it's ready to go. Thanks!

@bors: r+

@bors
Copy link
Contributor

bors commented Feb 12, 2021

📌 Commit 02ffe9e 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 Feb 12, 2021
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Feb 12, 2021
Fix injected errors when running doctests on a crate named after a keyword

Closes rust-lang#79771
bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 13, 2021
Rollup of 10 pull requests

Successful merges:

 - rust-lang#79775 (Fix injected errors when running doctests on a crate named after a keyword)
 - rust-lang#81012 (Stabilize the partition_point feature)
 - rust-lang#81479 (Allow casting mut array ref to mut ptr)
 - rust-lang#81506 (HWAddressSanitizer support)
 - rust-lang#81741 (Increment `self.index` before calling `Iterator::self.a.__iterator_ge…)
 - rust-lang#81850 (use RWlock when accessing os::env)
 - rust-lang#81911 (GAT/const_generics: Allow with_opt_const_param to return GAT param def_id)
 - rust-lang#82022 (Push a `char` instead of a `str` with len one into a String)
 - rust-lang#82023 (Remove unnecessary lint allow attrs on example)
 - rust-lang#82030 (Use `Iterator::all` instead of open-coding it)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit ab3f4f0 into rust-lang:master Feb 13, 2021
@rustbot rustbot added this to the 1.52.0 milestone Feb 13, 2021
@jyn514 jyn514 deleted the doctest branch February 13, 2021 13:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-doctests Area: Documentation tests, run by rustdoc 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.

Rustdoc creates doctests with extern crate mod;
8 participants