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

Pretty printing give proper error message without panic #100787

Merged
merged 2 commits into from
Sep 1, 2022

Conversation

chenyukang
Copy link
Member

Fixes #100770

@rustbot rustbot added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Aug 20, 2022
@rust-highfive
Copy link
Collaborator

r? @petrochenkov

(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 Aug 20, 2022
@chenyukang chenyukang changed the title pretty printing give proper error message without panic Pretty printing give proper error message without panic Aug 20, 2022
@chenyukang chenyukang force-pushed the fix-100770-pretty-crash branch 2 times, most recently from 25e474f to 3e41580 Compare August 22, 2022 13:45
@petrochenkov
Copy link
Contributor

Could you add a test for this (to src/test/ui/unpretty)?
@rustbot author

@rustbot rustbot 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 Aug 27, 2022
@chenyukang
Copy link
Member Author

Could you add a test for this (to src/test/ui/unpretty)? @rustbot author

Done!

@rust-log-analyzer

This comment has been minimized.

@chenyukang
Copy link
Member Author

oops, need to resolve rebase issue.

@rustbot rustbot added the A-translation Area: Translation infrastructure, and migrating existing diagnostics to SessionDiagnostic label Aug 28, 2022
@rustbot
Copy link
Collaborator

rustbot commented Aug 28, 2022

rustc_error_messages was changed

cc @davidtwco, @compiler-errors, @JohnTitor, @estebank, @TaKO8Ki

@rust-log-analyzer

This comment has been minimized.

@petrochenkov
Copy link
Contributor

@bors r+

@bors
Copy link
Contributor

bors commented Aug 28, 2022

📌 Commit 2237c6c has been approved by petrochenkov

It is now in the queue for this repository.

@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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Aug 28, 2022
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this pull request Aug 28, 2022
…, r=petrochenkov

Pretty printing give proper error message without panic

Fixes rust-lang#100770
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Aug 28, 2022
…, r=petrochenkov

Pretty printing give proper error message without panic

Fixes rust-lang#100770
@chenyukang
Copy link
Member Author

I added a #only-linux in the new testcase.

@petrochenkov
Copy link
Contributor

@bors r+

@bors
Copy link
Contributor

bors commented Aug 29, 2022

📌 Commit 6269794499c06845c0c570c4eba28c5acd35c4df has been approved by petrochenkov

It is now in the queue for this repository.

@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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Aug 29, 2022
@petrochenkov
Copy link
Contributor

@bors r-

Although it's better to remove this message from the test output entirely, using // normalize-stderr-test.
It's still brittle and may fail depending on locale and may be some other factors.

@bors bors 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-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Aug 29, 2022
@chenyukang
Copy link
Member Author

@bors r-

Although it's better to remove this message from the test output entirely, using // normalize-stderr-test. It's still brittle and may fail depending on locale and may be some other factors.

Agree, just know we have normalize-stderr-test, I will update it.

@petrochenkov
Copy link
Contributor

@bors r+

@bors
Copy link
Contributor

bors commented Aug 30, 2022

📌 Commit 77eb1ae has been approved by petrochenkov

It is now in the queue for this repository.

@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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Aug 30, 2022
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Aug 31, 2022
…, r=petrochenkov

Pretty printing give proper error message without panic

Fixes rust-lang#100770
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Aug 31, 2022
…, r=petrochenkov

Pretty printing give proper error message without panic

Fixes rust-lang#100770
bors added a commit to rust-lang-ci/rust that referenced this pull request Aug 31, 2022
…iaskrgr

Rollup of 10 pull requests

Successful merges:

 - rust-lang#100787 (Pretty printing give proper error message without panic)
 - rust-lang#100838 (Suggest moving redundant generic args of an assoc fn to its trait)
 - rust-lang#100844 (migrate rustc_query_system to use SessionDiagnostic)
 - rust-lang#101140 (Update Clippy)
 - rust-lang#101161 (Fix uintended diagnostic caused by `drain(..)`)
 - rust-lang#101165 (Use more `into_iter` rather than `drain(..)`)
 - rust-lang#101229 (Link “? operator” to relevant chapter in The Book)
 - rust-lang#101230 (lint: avoid linting diag functions with diag lints)
 - rust-lang#101236 (Avoid needless buffer zeroing in `std::sys::windows::fs`)
 - rust-lang#101240 (Fix a typo on `wasm64-unknown-unknown` doc)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 6438f4a into rust-lang:master Sep 1, 2022
@rustbot rustbot added this to the 1.65.0 milestone Sep 1, 2022
flip1995 pushed a commit to flip1995/rust that referenced this pull request Sep 9, 2022
…iaskrgr

Rollup of 10 pull requests

Successful merges:

 - rust-lang#100787 (Pretty printing give proper error message without panic)
 - rust-lang#100838 (Suggest moving redundant generic args of an assoc fn to its trait)
 - rust-lang#100844 (migrate rustc_query_system to use SessionDiagnostic)
 - rust-lang#101140 (Update Clippy)
 - rust-lang#101161 (Fix uintended diagnostic caused by `drain(..)`)
 - rust-lang#101165 (Use more `into_iter` rather than `drain(..)`)
 - rust-lang#101229 (Link “? operator” to relevant chapter in The Book)
 - rust-lang#101230 (lint: avoid linting diag functions with diag lints)
 - rust-lang#101236 (Avoid needless buffer zeroing in `std::sys::windows::fs`)
 - rust-lang#101240 (Fix a typo on `wasm64-unknown-unknown` doc)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@khyperia
Copy link
Contributor

khyperia commented Sep 22, 2022

Hmm, the avoid-crash.rs test appears to be failing for me (as in, compilation incorrectly succeeds) - I'm on windows, and have checked out rust on my D: drive.

Click to expand output of me running `./x.py test src/test/ui --test-args avoid-crash`
D:\src\rust> ./x.py test src/test/ui --test-args avoid-crash
Building rustbuild
    Finished dev [unoptimized] target(s) in 0.12s
Building stage0 std artifacts (x86_64-pc-windows-msvc -> x86_64-pc-windows-msvc)
    Finished release [optimized] target(s) in 0.43s
Copying stage0 std from stage0 (x86_64-pc-windows-msvc -> x86_64-pc-windows-msvc / x86_64-pc-windows-msvc)
Building stage0 compiler artifacts (x86_64-pc-windows-msvc -> x86_64-pc-windows-msvc)
    Finished release [optimized] target(s) in 0.76s
Copying stage0 rustc from stage0 (x86_64-pc-windows-msvc -> x86_64-pc-windows-msvc / x86_64-pc-windows-msvc)
Assembling stage1 compiler (x86_64-pc-windows-msvc)
Building stage1 std artifacts (x86_64-pc-windows-msvc -> x86_64-pc-windows-msvc)
    Finished release [optimized] target(s) in 0.43s
Copying stage1 std from stage1 (x86_64-pc-windows-msvc -> x86_64-pc-windows-msvc / x86_64-pc-windows-msvc)
Building stage0 tool compiletest (x86_64-pc-windows-msvc)
    Finished release [optimized] target(s) in 0.44s
Check compiletest suite=ui mode=ui (x86_64-pc-windows-msvc -> x86_64-pc-windows-msvc)

running 1 test
F
failures:

---- [ui] src/test\ui\unpretty\avoid-crash.rs stdout ----

error: ui test compiled successfully!
status: exit code: 0
command: PATH="D:\src\rust\build\x86_64-pc-windows-msvc\stage1\bin;C:\Program Files (x86)\Windows Kits\10\bin\x64;C:\Program Files (x86)\Windows Kits\10\bin\10.0.19041.0\x64;C:\Program Files\Microsoft Visual Studio\2022\Professional\VC\Tools\MSVC\14.31.31103\bin\HostX64\x64;C:\Program Files\Microsoft Visual Studio\2022\Professional\VC\Tools\MSVC\14.31.31103\bin\HostX64\x64;D:\src\rust\build\x86_64-pc-windows-msvc\stage0-bootstrap-tools\x86_64-pc-windows-msvc\release\deps;D:\src\rust\build\x86_64-pc-windows-msvc\stage0\bin;C:\Program Files\PowerShell\7;C:\Python310\Scripts\;C:\Python310\;C:\Program Files (x86)\Common Files\Oracle\Java\javapath;C:\Program Files\ImageMagick-7.1.0-Q16-HDRI;C:\Program Files\Oculus\Support\oculus-runtime;C:\Windows\system32;C:\Windows;C:\Windows\System32\Wbem;C:\Windows\System32\WindowsPowerShell\v1.0\;C:\Windows\System32\OpenSSH\;C:\ProgramData\chocolatey\bin;C:\Program Files\dotnet\;C:\Program Files\NVIDIA Corporation\NVIDIA NvDLISR;C:\Program Files (x86)\NVIDIA Corporation\PhysX\Common;C:\Program Files\Microsoft SQL Server\150\Tools\Binn\;C:\Program Files\OpenJDK\jdk-18.0.2\bin;C:\Program Files\CMake\bin;C:\Program Files\Git\cmd;C:\Program Files\PowerShell\7\;C:\Program Files\Perforce\;C:\Users\khype\.cargo\bin;C:\Users\khype\AppData\Local\Microsoft\WindowsApps;C:\Users\khype\Documents\unison\bin;C:\tools\neovim\Neovim\bin;C:\Users\khype\.dotnet\tools;C:\tools\neovim\nvim-win64\bin;;C:\Users\khype\AppData\Local\Programs\Microsoft VS Code\bin" "D:\\src\\rust\\build\\x86_64-pc-windows-msvc\\stage1\\bin\\rustc.exe" "D:\\src\\rust\\src/test\\ui\\unpretty\\avoid-crash.rs" "-Zthreads=1" "--target=x86_64-pc-windows-msvc" "--error-format" "json" "--json" "future-incompat" "-Ccodegen-units=1" "-Zui-testing" "-Zdeduplicate-diagnostics=no" "-Cstrip=debuginfo" "--emit" "metadata" "-C" "prefer-dynamic" "--out-dir" "D:\\src\\rust\\build\\x86_64-pc-windows-msvc\\test\\ui\\unpretty\\avoid-crash" "-A" "unused" "-Crpath" "-Cdebuginfo=0" "-Lnative=D:\\src\\rust\\build\\x86_64-pc-windows-msvc\\native\\rust-test-helpers" "-Clink-arg=-Wl,/threads:1" "-Clinker=D:\\src\\rust\\build\\x86_64-pc-windows-msvc\\stage0\\lib\\rustlib\\x86_64-pc-windows-msvc\\bin\\rust-lld" "-o/tmp/" "-Zunpretty=ast-tree" "-L" "D:\\src\\rust\\build\\x86_64-pc-windows-msvc\\test\\ui\\unpretty\\avoid-crash\\auxiliary"
stdout: none
stderr: none



failures:
    [ui] src/test\ui\unpretty\avoid-crash.rs

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

Some tests failed in compiletest suite=ui mode=ui host=x86_64-pc-windows-msvc target=x86_64-pc-windows-msvc
Build completed unsuccessfully in 0:00:19

I noticed that this created a file called D:\tmp (the test has compile-flags: -o/tmp/ -Zunpretty=ast-tree):

D:\> cat tmp
Crate {
    attrs: [],
    items: [
        Item {
            attrs: [],
            id: NodeId(4294967040),
            span: D:\src\rust\src/test\ui\unpretty\avoid-crash.rs:4:1: 4:13 (#0),
[... I truncated the file ...]

My theory is that this passed the Windows CI build because there's perhaps a C:\tmp\ directory that already exists (which makes compilation fail, i.e. the test passes), or that it has a checkout on the C: drive instead of some D: drive and has a permission error when creating the C:\tmp file.

Let me know if you'd like any more information, or if you'd like me to run more tests to figure out what's going on! ❤️

(For anyone else running into this, a workaround to make the test pass again is to mkdir D:\tmp, or on whatever drive you have rust checked out on)

@chenyukang
Copy link
Member Author

@khyperia Thanks for pointing this out.
Yes, if the directory has already existed(like /tmp/ in Linux), the test case will pass, since created it will fail.

I thought /tmp/ will always be a invalid for Windows... so it will also fail when creating it.
I don't have a PC right now, could you please make a fix on it? Thanks!

@khyperia
Copy link
Contributor

I don't have a PC right now, could you please make a fix on it? Thanks!

Unfortunately I'm not sure what the right fix is!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-translation Area: Translation infrastructure, and migrating existing diagnostics to SessionDiagnostic S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

rustc panics if output is set to directory while pretty printing
9 participants