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

Flush delayed bugs before codegen #102373

Merged
merged 4 commits into from
Oct 1, 2022

Conversation

Noratrieb
Copy link
Member

Sometimes it can happen that invalid code like a TyKind::Error makes its way through the compiler without triggering any errors (this is always a bug in rustc but bugs do happen sometimes :)). These ICEs will manifest in the backend like as cg_llvm not being able to get the layout of [type error], which makes it hard to debug. By flushing before codegen, we display all the delayed bugs, making it easier to trace it to the root of the problem.

I tried this on #102366 and it showed tons of of delayed bugs and no error in cg_llvm, so it seems to be working.

Sometimes it can happen that invalid code like a TyKind::Error makes
its way through the compiler without triggering any errors (this is
always a bug in rustc but bugs do happen sometimes :)). These ICEs
will manifest in the backend like as cg_llvm not being able to get
the layout of `[type error]`, which makes it hard to debug. By flushing
before codegen, we display all the delayed bugs, making it easier to
trace it to the root of the problem.
@rust-highfive
Copy link
Collaborator

r? @nagisa

(rust-highfive has picked a reviewer for you, use r? to override)

@rustbot rustbot added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Sep 27, 2022
@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Sep 27, 2022
@Noratrieb
Copy link
Member Author

Interestingly, I've encountered exactly this problem just in this moment myself when I had bugs in my ast_lowering code that made their way to cg_llvm as well :D

@cjgillot
Copy link
Contributor

Does this deserve a test, or at least a comment on why we are exiting early?

@Noratrieb
Copy link
Member Author

I don't think we can test this, but I can add a comment, I forgot about that

@cjgillot
Copy link
Contributor

@bors r+

@bors
Copy link
Contributor

bors commented Sep 30, 2022

📌 Commit 4b1cf84 has been approved by cjgillot

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-review Status: Awaiting review from the assignee but also interested parties. labels Sep 30, 2022
@Noratrieb
Copy link
Member Author

@cjgillot there's another typo, likey, lol

you can reapprove it if you want

@jyn514
Copy link
Member

jyn514 commented Sep 30, 2022

@bors r- r+

@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 Sep 30, 2022
@bors
Copy link
Contributor

bors commented Sep 30, 2022

📌 Commit e8f1bfe has been approved by jyn514

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 Sep 30, 2022
@jyn514
Copy link
Member

jyn514 commented Sep 30, 2022

@bors r- r=cjgillot

@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 Sep 30, 2022
@bors
Copy link
Contributor

bors commented Sep 30, 2022

📌 Commit e8f1bfe has been approved by cjgillot

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 Sep 30, 2022
bors added a commit to rust-lang-ci/rust that referenced this pull request Oct 1, 2022
…iaskrgr

Rollup of 7 pull requests

Successful merges:

 - rust-lang#102361 (Fix ICE in const_trait check code)
 - rust-lang#102373 (Flush delayed bugs before codegen)
 - rust-lang#102483 (create def ids for impl traits during ast lowering)
 - rust-lang#102490 (Generate synthetic region from `impl` even in closure body within an associated fn)
 - rust-lang#102492 (Don't lower assoc bindings just to deny them)
 - rust-lang#102493 (Group together more size assertions.)
 - rust-lang#102521 (rustdoc: add missing margin to no-docblock methods)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 9ec772e into rust-lang:master Oct 1, 2022
@rustbot rustbot added this to the 1.66.0 milestone Oct 1, 2022
@Noratrieb Noratrieb deleted the cannot-get-layout-of-branch-error branch October 1, 2022 06:38
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. 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.

7 participants