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

Rust 1.18 regression rustc-serialize doc tests have regressed #40994

Closed
alexcrichton opened this issue Apr 1, 2017 · 5 comments · Fixed by rust-lang-deprecated/rustc-serialize#182
Assignees
Labels
A-docs Area: documentation for any part of the project, including the compiler, standard library, and tools P-high High priority regression-from-stable-to-nightly Performance or correctness regression from stable to nightly.

Comments

@alexcrichton
Copy link
Member

https://ci.appveyor.com/project/alexcrichton/rustc-serialize/build/1.0.563/job/s4vnewjo4l4448j2

failures:
---- src\base64.rs - base64::DECODE_TABLE (line 333) stdout ----
	error: unexpected close delimiter: `}`
  --> <anon>:21:1
   |
21 | }
   | ^
thread 'rustc' panicked at 'Box<Any>', src\libsyntax\parse\mod.rs:216
note: Run with `RUST_BACKTRACE=1` for a backtrace.
thread 'rustc' panicked at 'couldn't compile the test', src\librustdoc\test.rs:270
failures:
    src\base64.rs - base64::DECODE_TABLE (line 333)
test result: FAILED. 20 passed; 1 failed; 1 ignored; 0 measured

Likely related to #40912

@alexcrichton alexcrichton added the regression-from-stable-to-nightly Performance or correctness regression from stable to nightly. label Apr 1, 2017
@steveklabnik
Copy link
Member

steveklabnik commented Apr 1, 2017

This is basically the same as #40979

https://github.com/rust-lang-nursery/rustc-serialize/blob/master/src/base64.rs#L332-L333

hoedown treated this all as a p tag, pulldown-cmark treats this as a <p><code>

In this case, the code block is invalid. The fix is to either fix that by removing line 352 (and then outdenting all the code), add a function header that accepts ch to make it a different kind of example, or change to rust,ignore so that it's not run as a test.

(I would also add a newline between 32 and 33; that's the whole reason it's ambiguous between implementations anyway, with a blank line, they treat them identically (and it would be the new behavior either way))

@alexcrichton
Copy link
Member Author

@steveklabnik sure yeah, but to put this in perspective: this is invalid TOML that Cargo still accepts with a warning and we even fix bugs in parsing as they arise to ensure we can keep parsing this.

I think we should totally fix the markdown, but I don't think we should immediately start failing so many builds without warning.

@steveklabnik
Copy link
Member

steveklabnik commented Apr 1, 2017

so many builds without warning.

I've seen roughly 3 other instances of this so far, and they've all already been fixed. If there's a huge impact, I agree, but I haven't actually seen any evidence of that.

@bluss
Copy link
Member

bluss commented Apr 2, 2017

It's a CI failure for rustc-serialize that does not affect anything that depends on rustc-serialize, so impact is very limited in that way

@brson brson added A-docs Area: documentation for any part of the project, including the compiler, standard library, and tools T-tools labels Apr 4, 2017
@brson brson added the P-high High priority label Apr 4, 2017
steveklabnik added a commit to rust-lang-deprecated/rustc-serialize that referenced this issue Apr 4, 2017
Previously, hoedown did not understand that this was an example. pulldown-cmark does. This fixes the example to be compile-able.

Fixes rust-lang/rust#40994
@steveklabnik
Copy link
Member

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-docs Area: documentation for any part of the project, including the compiler, standard library, and tools P-high High priority regression-from-stable-to-nightly Performance or correctness regression from stable to nightly.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants