Skip to content

Conversation

@traxys
Copy link
Contributor

@traxys traxys commented Nov 3, 2019

This adds:

I am not sure the attribute added should-ice is the best for this job

@rust-highfive
Copy link
Contributor

r? @davidtwco

(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 Nov 3, 2019
@davidtwco
Copy link
Member

r? @michaelwoerister

Copy link
Member

@michaelwoerister michaelwoerister left a comment

Choose a reason for hiding this comment

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

Thanks for the PR. @traxys! This looks good in general. I wonder though if the // should-ice functionality can be achieved with existing means. I'll take a closer look later today. Meanwhile, would you mind addressing the nit listed below?

@michaelwoerister
Copy link
Member

@traxys, I'm not quite clear if // should-ice brings new functionality. Could you also write the tests as the following?

// revisions: cfail1 cfail2
// error-pattern: delayed span bug triggered by #[rustc_error(delay_span_bug_from_inside_query)]
// failure-status: 101

#![feature(rustc_attrs)]

#[rustc_error(delay_span_bug_from_inside_query)]
fn main() {}

@traxys
Copy link
Contributor Author

traxys commented Nov 5, 2019

@michaelwoerister the problem is this line

Some(101) => self.fatal_proc_rec("compiler encountered internal error", proc_res),

There is currently no way to avoid the early stop

@michaelwoerister
Copy link
Member

There is currently no way to avoid the early stop

OK, that makes sense. In that case adding // should-ice looks like the way to go.

@traxys
Copy link
Contributor Author

traxys commented Nov 7, 2019

I think the should-ice should be restricted to cfail tests, meaning if it is not a cfail test the compiletest errors. Is it a good idea ?

@michaelwoerister
Copy link
Member

Yeah, that's fine.

@traxys
Copy link
Contributor Author

traxys commented Nov 8, 2019

I don't know if I put the test in the best place, but it should now error on misuse of should-ice

@michaelwoerister
Copy link
Member

Thanks, @traxys! This looks good to me now.

@bors r+

@bors
Copy link
Collaborator

bors commented Nov 20, 2019

📌 Commit e01d941 has been approved by michaelwoerister

@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 Nov 20, 2019
Centril added a commit to Centril/rust that referenced this pull request Nov 20, 2019
Making ICEs and test them in incremental

This adds:
 - A way to make the compiler ICE
 - A way to check for ICE in `cfail` tests with `should-ice`
 - A regression test for issue rust-lang#65401

I am not sure the attribute added `should-ice` is the best for this job
bors added a commit that referenced this pull request Nov 20, 2019
Rollup of 7 pull requests

Successful merges:

 - #66060 (Making ICEs and test them in incremental)
 - #66298 (rustdoc: fixes #64305: disable search field instead of hidding it)
 - #66457 (Just derive Hashstable in librustc)
 - #66496 (rustc_metadata: Privatize more things)
 - #66514 (Fix selected crate search filter)
 - #66535 (Avoid ICE when `break`ing to an unreachable label)
 - #66573 (Ignore run-make reproducible-build-2 on Mac)

Failed merges:

r? @ghost
@bors bors merged commit e01d941 into rust-lang:master Nov 20, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants