Skip to content

Making ICEs and test them in incremental - #66060

Merged
bors merged 9 commits into
rust-lang:masterfrom
traxys:test_65401
Nov 20, 2019
Merged

Making ICEs and test them in incremental#66060
bors merged 9 commits into
rust-lang:masterfrom
traxys:test_65401

Conversation

@traxys

@traxystraxys commented Nov 3, 2019

Copy link
Copy Markdown
Contributor

This adds:

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

@rust-highfive

Copy link
Copy Markdown
Contributor

r? @davidtwco

(rust_highfive has picked a reviewer for you, use r? to override)

@rust-highfiverust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Nov 3, 2019
@davidtwco

Copy link
Copy Markdown
Member

r? @michaelwoerister

@michaelwoeristermichaelwoerister left a comment

Copy link
Copy Markdown
Member

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?

Comment threadsrc/librustc_codegen_utils/lib.rs Outdated
Comment threadsrc/librustc_codegen_utils/lib.rs Outdated
@michaelwoerister

Copy link
Copy Markdown
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)]fnmain(){}

@traxys

traxys commented Nov 5, 2019

Copy link
Copy Markdown
ContributorAuthor

@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
Copy Markdown
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.

Comment threadsrc/tools/compiletest/src/runtest.rs Outdated
@traxys

Copy link
Copy Markdown
ContributorAuthor

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
Copy Markdown
Member

Yeah, that's fine.

@traxys

Copy link
Copy Markdown
ContributorAuthor

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
Copy Markdown
Member

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

@bors r+

@bors

bors commented Nov 20, 2019

Copy link
Copy Markdown
Collaborator

📌 Commit e01d941 has been approved by michaelwoerister

@borsbors 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
@CentrilCentril mentioned this pull request Nov 20, 2019
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:masterNov 20, 2019
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S-waiting-on-borsStatus: 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

@traxys@rust-highfive@davidtwco@michaelwoerister@bors