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

rustc_parse_format: Fix character indices in find_skips #81071

Merged
merged 1 commit into from
Jan 18, 2021

Conversation

osa1
Copy link
Contributor

@osa1 osa1 commented Jan 16, 2021

Fixes #81006

@rust-highfive
Copy link
Collaborator

r? @estebank

(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 Jan 16, 2021
@osa1
Copy link
Contributor Author

osa1 commented Jan 16, 2021

I have two different regression tests that fail in different ways without this patch. I'll add those once I figure out how to make them pass with the specified error message.

@osa1
Copy link
Contributor Author

osa1 commented Jan 16, 2021

I added two tests, both fail with errors like:

running 1 test
F
failures:

---- [ui] ui/issues/issue-81006-2.rs stdout ----

error: /home/omer/rust/rust/src/test/ui/issues/issue-81006-2.rs:6: unexpected error: '6:23: 6:25: 1 positional argument in format string, but no arguments were given'

error: 1 unexpected errors found, 0 expected errors not found
status: exit code: 1
command: "/home/omer/rust/rust/build/x86_64-unknown-linux-gnu/stage1/bin/rustc" "/home/omer/rust/rust/src/test/ui/issues/issue-81006-2.rs" "-Zthreads=1" "--target=x86_64-unknown-linux-gnu" "--error-format" "json" "-Zui-testing" "-Zdeduplicate-diagnostics=no" "-Zemit-future-incompat-report" "--emit" "metadata" "-C" "prefer-dynamic" "--out-dir" "/home/omer/rust/rust/build/x86_64-unknown-linux-gnu/test/ui/issues/issue-81006-2" "-A" "unused" "-Crpath" "-O" "-Cdebuginfo=0" "-Zunstable-options" "-Lnative=/home/omer/rust/rust/build/x86_64-unknown-linux-gnu/native/rust-test-helpers" "-L" "/home/omer/rust/rust/build/x86_64-unknown-linux-gnu/test/ui/issues/issue-81006-2/auxiliary"
unexpected errors (from JSON output): [
    Error {
        line_num: 6,
        kind: Some(
            Error,
        ),
        msg: "6:23: 6:25: 1 positional argument in format string, but no arguments were given",
    },
]

thread '[ui] ui/issues/issue-81006-2.rs' panicked at 'explicit panic', src/tools/compiletest/src/runtest.rs:1491:13
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

Any ideas why? These are expect-fail tests and the stderr files match the actual error messages.

@osa1
Copy link
Contributor Author

osa1 commented Jan 16, 2021

All fixed, this should be good to go

@rust-log-analyzer

This comment has been minimized.

@estebank
Copy link
Contributor

Great work @osa1! Could you squash your commits into a single one? After that ping me here and I'll merge it ASAP :)

@estebank estebank added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. beta-nominated Nominated for backporting to the compiler in the beta channel. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jan 16, 2021
@osa1
Copy link
Contributor Author

osa1 commented Jan 16, 2021

@estebank ah, sorry, I thought CI squashes commits automatically. Just did it myself. Thanks!

@estebank
Copy link
Contributor

For t-compiler: Nominating for beta backport to minimize time where the error is user visible. The change is small, seems quite safe and the current ICE precludes an actual usable error, leaving users that hit this to their own devices.

@estebank
Copy link
Contributor

Thanks for the quick turnaround!

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Jan 16, 2021

📌 Commit 28501c0 has been approved by estebank

@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 Jan 16, 2021
m-ou-se added a commit to m-ou-se/rust that referenced this pull request Jan 16, 2021
rustc_parse_format: Fix character indices in find_skips

Fixes rust-lang#81006
m-ou-se added a commit to m-ou-se/rust that referenced this pull request Jan 17, 2021
rustc_parse_format: Fix character indices in find_skips

Fixes rust-lang#81006
@m-ou-se
Copy link
Member

m-ou-se commented Jan 17, 2021

This failed in a rollup (#81111 (comment)), as src/test/ui/issues/issue-81006.rs added by this PR is making that directory too big:

tidy error: following path contains more than 2669 entries, you should move the test to some relevant subdirectory (current: 2671): /checkout/src/test/ui/issues

That doesn't mean that this PR is to blame, as the blame is shared with a few thousand other files. But we can't merge it like this. :(

See #73494 and #80942 (comment).

@bors r-

@bors bors removed the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label Jan 17, 2021
@bors bors added the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Jan 17, 2021
@osa1
Copy link
Contributor Author

osa1 commented Jan 17, 2021

Moved the test to ui/macros.

@m-ou-se
Copy link
Member

m-ou-se commented Jan 17, 2021

@bors r=estebank

@bors
Copy link
Contributor

bors commented Jan 17, 2021

📌 Commit 9111e9d has been approved by estebank

@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 Jan 17, 2021
bors added a commit to rust-lang-ci/rust that referenced this pull request Jan 18, 2021
Rollup of 12 pull requests

Successful merges:

 - rust-lang#81038 (Update Clippy)
 - rust-lang#81071 (rustc_parse_format: Fix character indices in find_skips)
 - rust-lang#81100 (prevent potential bug in `encode_with_shorthand`.)
 - rust-lang#81105 (Initialize a few variables directly)
 - rust-lang#81116 (ConstProp: Copy body span instead of querying it)
 - rust-lang#81121 (Avoid logging the whole MIR body in SimplifyCfg)
 - rust-lang#81123 (Update cmp.rs)
 - rust-lang#81125 (Add track_caller to .steal())
 - rust-lang#81128 (validation test: turn some const_err back into validation failures)
 - rust-lang#81131 (Edit rustc_middle::ty::cast docs)
 - rust-lang#81142 (Replace let Some(..) = with .is_some())
 - rust-lang#81153 (Remove unused linkcheck exceptions)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit c7ca540 into rust-lang:master Jan 18, 2021
@rustbot rustbot added this to the 1.51.0 milestone Jan 18, 2021
@apiraino apiraino added beta-accepted Accepted for backporting to the compiler in the beta channel. and removed beta-nominated Nominated for backporting to the compiler in the beta channel. labels Jan 21, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
beta-accepted Accepted for backporting to the compiler in the beta channel. 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.

Format string/argument mismatch triggers internal compiler error
9 participants