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

ICEs should always print the top of the query stack #77493

Conversation

hosseind88
Copy link
Contributor

@hosseind88 hosseind88 commented Oct 3, 2020

see #76920

@rust-highfive
Copy link
Collaborator

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @oli-obk (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Oct 3, 2020
@jyn514 jyn514 changed the title Ic es should always print the top of the query stack ICEs should always print the top of the query stack Oct 3, 2020
@jyn514 jyn514 added A-diagnostics Area: Messages for errors, warnings, and lints T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Oct 3, 2020
@hosseind88
Copy link
Contributor Author

hosseind88 commented Oct 3, 2020

as test failure result instruction, I do ./x.py test --bless --test-args pattern/const-pat-ice.rs
but I get error
anyone knows why? :(
Screen Shot 2020-10-03 at 7 52 07 PM

@@ -9,3 +9,5 @@ note: we would appreciate a bug report: https://github.com/rust-lang/rust-clippy

note: Clippy version: foo

query stack during panic:
we're just showing a limited slice of the query stack
Copy link
Member

Choose a reason for hiding this comment

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

I think it shouldn't say this if all parts of the query stack have been printed. Even when the maximum length is limited.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@bjorn3 so I guess I should normalize it right?
and also when I run the test, it fails, and it deletes the const-pat-ice.stderr file
do you think that we should remove it?

@oli-obk
Copy link
Contributor

oli-obk commented Oct 5, 2020

as test failure result instruction, I do ./x.py test --bless --test-args pattern/const-pat-ice.rs
but I get error

That test does not exist anymore (well, it doesn't ICE anymore). So you should be able to just remove all changes you did around it.

You likely need to rebase over this repository's master branch. I'm not sure what your setup is, but if you invoke git remote -v do you see a rust-lang/rust remote in there? If not, you need to add it with git remote add upstream git@github.com:rust-lang/rust.git and then do git pull --rebase upstream/master. Never rebase over the master branch of your own repository, that will cause a huge mess (I know this, because I've done this a lot in the beginning, too ;) )

@hosseind88
Copy link
Contributor Author

as test failure result instruction, I do ./x.py test --bless --test-args pattern/const-pat-ice.rs
but I get error

That test does not exist anymore (well, it doesn't ICE anymore). So you should be able to just remove all changes you did around it.

You likely need to rebase over this repository's master branch. I'm not sure what your setup is, but if you invoke git remote -v do you see a rust-lang/rust remote in there? If not, you need to add it with git remote add upstream git@github.com:rust-lang/rust.git and then do git pull --rebase upstream/master. Never rebase over the master branch of your own repository, that will cause a huge mess (I know this, because I've done this a lot in the beginning, too ;) )

thanks, ok I will use this approach

@hosseind88 hosseind88 force-pushed the ICEs_should_always_print_the_top_of_the_query_stack branch from f3c495f to f3a3c81 Compare October 6, 2020 18:26
Comment on lines 169 to 173
if num_frames != None {
eprintln!("we're just showing a limited slice of the query stack");
} else {
eprintln!("end of query stack");
}
Copy link
Contributor

Choose a reason for hiding this comment

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

I think what bjorn3 is suggesting is

Suggested change
if num_frames != None {
eprintln!("we're just showing a limited slice of the query stack");
} else {
eprintln!("end of query stack");
}
if num_frames == None || num_frames >= Some(i) {
eprintln!("end of query stack");
} else {
eprintln!("we're just showing a limited slice of the query stack");
}

but you'll also have to pull out let mut i = 0; so that we have access to i here.

This will make sure end of query stack is printed if there are less than num_frames frames to print.

@oli-obk
Copy link
Contributor

oli-obk commented Oct 7, 2020

After doing all your changes, please use git rebase -i upstream/master and replace all but the first commit's pick with fixup. This will squash all commits down into the first commit.

@ThibsG
Copy link
Contributor

ThibsG commented Oct 8, 2020

Memo: this PR is related to #70953

if num_frames != None {
eprintln!("we're just showing a limited slice of the query stack");
} else {
if num_frames == None || num_frames >= Some(i) {
Copy link
Contributor

Choose a reason for hiding this comment

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

The output did not change... Not sure what's wrong. If num_frames is Some(2) and i is 0, this should return true (according to the playground at least: https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=57ab1b6049e331f483455f1ce170e183)

@hosseind88
Copy link
Contributor Author

guys anyone has any ideas about it? I thought a lot but I could not find the reason for this issue

@oli-obk
Copy link
Contributor

oli-obk commented Oct 12, 2020

The failing test is a clippy lint, did you run ./x.py test src/tools/clippy?

@hosseind88 hosseind88 force-pushed the ICEs_should_always_print_the_top_of_the_query_stack branch from 668aaeb to 46cc889 Compare October 14, 2020 14:50
@hosseind88
Copy link
Contributor Author

@oli-obk I guess now everything is ok

@oli-obk
Copy link
Contributor

oli-obk commented Oct 15, 2020

@bors r+

great!

@bors
Copy link
Contributor

bors commented Oct 15, 2020

📌 Commit 46cc889 has been approved by oli-obk

@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 Oct 15, 2020
JohnTitor added a commit to JohnTitor/rust that referenced this pull request Oct 15, 2020
…_the_top_of_the_query_stack, r=oli-obk

ICEs should always print the top of the query stack

see rust-lang#76920
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Oct 15, 2020
…_the_top_of_the_query_stack, r=oli-obk

ICEs should always print the top of the query stack

see rust-lang#76920
bors added a commit to rust-lang-ci/rust that referenced this pull request Oct 16, 2020
Rollup of 14 pull requests

Successful merges:

 - rust-lang#75023 (ensure arguments are included in count mismatch span)
 - rust-lang#75265 (Add `str::{Split,RSplit,SplitN,RSplitN,SplitTerminator,RSplitTerminator,SplitInclusive}::as_str` methods)
 - rust-lang#75675 (mangling: mangle impl params w/ v0 scheme)
 - rust-lang#76084 (Refactor io/buffered.rs into submodules)
 - rust-lang#76119 (Stabilize move_ref_pattern)
 - rust-lang#77493 (ICEs should always print the top of the query stack)
 - rust-lang#77619 (Use futex-based thread-parker for Wasm32.)
 - rust-lang#77646 (For backtrace, use StaticMutex instead of a raw sys Mutex.)
 - rust-lang#77648 (Static mutex is static)
 - rust-lang#77657 (Cleanup cloudabi mutexes and condvars)
 - rust-lang#77672 (Simplify doc-cfg rendering based on the current context)
 - rust-lang#77780 (rustc_parse: fix spans on cast and range exprs with attrs)
 - rust-lang#77935 (BTreeMap: make PartialCmp/PartialEq explicit and tested)
 - rust-lang#77980 (Fix intra doc link for needs_drop)

Failed merges:

r? `@ghost`
@bors bors merged commit 0e4d196 into rust-lang:master Oct 16, 2020
@rustbot rustbot added this to the 1.49.0 milestone Oct 16, 2020
bors pushed a commit to rust-lang-ci/rust that referenced this pull request Oct 24, 2020
…_the_top_of_the_query_stack, r=oli-obk

ICEs should always print the top of the query stack

see rust-lang#76920
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-diagnostics Area: Messages for errors, warnings, and lints 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.

9 participants