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

regression: rustdoc resolution changes #61560

Closed
Mark-Simulacrum opened this issue Jun 5, 2019 · 18 comments
Closed

regression: rustdoc resolution changes #61560

Mark-Simulacrum opened this issue Jun 5, 2019 · 18 comments
Assignees
Labels
P-high High priority regression-from-stable-to-stable Performance or correctness regression from one stable version to another. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.

Comments

@Mark-Simulacrum
Copy link
Member

bint-0.1.0: start v. end; cc @folkengine

Looks like resolution for the crate itself changed in some way, not sure.

@Mark-Simulacrum Mark-Simulacrum added T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. regression-from-stable-to-beta Performance or correctness regression from stable to beta. labels Jun 5, 2019
@Mark-Simulacrum Mark-Simulacrum changed the title regression: rustdoc bint 0.1.0 regression: rustdoc resolution changes Jun 5, 2019
@Mark-Simulacrum
Copy link
Member Author

  • lv2rs-core-0.3.0: start v. end; cc @Janonard
  • oysterpack_testing-0.1.2: start v. end; cc @oysterpack (maybe this issue, couldn't test due to openssl v0.9)

@Mark-Simulacrum
Copy link
Member Author

@Mark-Simulacrum Mark-Simulacrum added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Jun 21, 2019
@pnkfelix
Copy link
Member

triage: P-high.

@pnkfelix pnkfelix added the P-high High priority label Jul 11, 2019
@Mark-Simulacrum Mark-Simulacrum added regression-from-stable-to-stable Performance or correctness regression from one stable version to another. and removed regression-from-stable-to-beta Performance or correctness regression from stable to beta. labels Jul 11, 2019
@JOOpdenhoevel
Copy link

JOOpdenhoevel commented Jul 11, 2019

What does this issue mean? Do I as a crate owner have to worry about something?

@Mark-Simulacrum
Copy link
Member Author

Presumably, the crates listed here are broken on current stable (1.36+) but this has not been verified. Someone will probably investigate soon and assign this to someone concrete to resolve.

@frehberg
Copy link
Contributor

I will investigate my crate test-generator

@frehberg
Copy link
Contributor

as for the test-generator, I tested various versions, cant see any error as in the log. There is no stdout for the error-case, any idea why it might fail?

@JOOpdenhoevel
Copy link

Btw, I found the problem with lv2rs-core and already fixed and updated it!

@nagisa
Copy link
Member

nagisa commented Jul 18, 2019

Assigning to self to bisect the exact change which caused this regression, but it otherwise looks more like a rustdoc issue than a rustc one.

@nagisa nagisa self-assigned this Jul 18, 2019
@Mark-Simulacrum Mark-Simulacrum added the E-needs-bisection Call for participation: This issue needs bisection: https://github.com/rust-lang/cargo-bisect-rustc label Sep 19, 2019
@pnkfelix
Copy link
Member

hey @nagisa , do you think you will get around the bisection at some point? Or shall I assign to someone else?

@pnkfelix
Copy link
Member

triage: reassigning to self.

@pnkfelix pnkfelix assigned pnkfelix and unassigned nagisa Sep 26, 2019
@pnkfelix
Copy link
Member

pnkfelix commented Oct 3, 2019

nominating for reassignment (want to get things that can be delegated off my plate)

@lqd lqd self-assigned this Oct 3, 2019
@lqd
Copy link
Member

lqd commented Oct 3, 2019

I can repro on lv2rs-core, but looking at the logs, this was interesting:

Before:

Doc-tests bint
[INFO] [stdout] running 0 tests

After:

Doc-tests bint
[INFO] [stdout] running 1 test

This suggests a rustdoc change that detects more tests rather than resolution changes.

Started failing in 1.36.0.
Bisected to nightly-2019-04-24, and commit 0550766 which is an update to rustdoc's markdown parser.

In this case, I think the old tests did not compile, but that wasn't easy to see until this version of rustdoc actually started testing them.

@lqd
Copy link
Member

lqd commented Oct 3, 2019

Looking around similar issues, there's #61562 and this comment mentioning the same theory.

Puzzling, the code in the specific lv2rs-core doctest may have been ... correct ? cargo test still fails on 1.37.0 but passes on 1.38.0, or the latest nightly for example.

@pnkfelix what do you want to do with this issue ?

@lqd lqd removed the E-needs-bisection Call for participation: This issue needs bisection: https://github.com/rust-lang/cargo-bisect-rustc label Oct 3, 2019
@pnkfelix
Copy link
Member

pnkfelix commented Oct 3, 2019

@lqd it sounds to me like something we can either hand off to the T-rustdoc team, or simply close outright.

@lqd
Copy link
Member

lqd commented Oct 3, 2019

OK, I'll keep bisecting the other crates, and also check if the failures reproduce on recent stables/nightlies, to see if we need to open more specific issues, and/or hand this one to T-rustdoc.

@lqd
Copy link
Member

lqd commented Oct 4, 2019

Update:

  • oysterpack_testing-0.1.2, reproduces on the beta-2019-05-30 tested in this crater run, but does not reproduce for the released 1.36 (or nightly), so it seems already fixed.
  • test-generator-0.2.2, the setup seems complicated, there are proc_macros to help writing tests. These doctests rely on the unstable #[bench] feature, and was accepted e.g. on 1.35. It fails on beta-2019-05-30, works on 1.36 and 1.37, fails since 1.38 (and nightly) and is tracked in Tracking issue for future-incompatibility lint soft_unstable #64266. (It's interesting that it seems to be a lint but is actually deny).
    It's likely that the previous rustdoc didn't see these doctests in this case as well. I'll comment on the tracking issue.
  • trimmer-0.3.6: works on 1.35, fails on this beta-2019-05-30, and works ever since 1.36, so it's likely close to the now-fixed issuelv2rs-core also had.

As a summary, it seems we have a mix of real rustc bugs, rustdoc effectively-breaking-changes-for-the-better testing new code, and invalid tests. The rustc bugs seem fixed, and the rustdoc behaviour change is #61562, and there might be miscommunication between rustdoc and rustc for the soft_unstable lint (or just its error message also having a future-compatibility lint flavor being confusing).

If need be, I could extract the doctests and bisect those to rustc commits, but I'm not sure we need to ?

@lqd
Copy link
Member

lqd commented Oct 4, 2019

As summarized earlier, this issue collects other issues, which were at the intersection of

While we could bisect those problems to look for possible missing test cases, it's likely to point at known issues since they were all fixed either in previous betas of the same crater run or 2-3 releases ago, it doesn't seem absolutely necessary to do so. So let's close this one.

@lqd lqd closed this as completed Oct 4, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P-high High priority regression-from-stable-to-stable Performance or correctness regression from one stable version to another. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

6 participants