Skip to content

Do not ICE when we have -Zunpretty=expanded with invalid ABI - #99360

Merged
bors merged 1 commit into
rust-lang:masterfrom
compiler-errors:issue-99331
Jul 18, 2022
Merged

Do not ICE when we have -Zunpretty=expanded with invalid ABI#99360
bors merged 1 commit into
rust-lang:masterfrom
compiler-errors:issue-99331

Conversation

@compiler-errors

@compiler-errorscompiler-errors commented Jul 17, 2022

Copy link
Copy Markdown
Contributor

Fixes#99331

@rustbotrustbot added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Jul 17, 2022
@rust-highfive

rust-highfive commented Jul 17, 2022

Copy link
Copy Markdown
Contributor

r? @fee1-dead

(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 Jul 17, 2022
compiler-errors
compiler-errors commented Jul 17, 2022
View reviewed changes
Comment threadcompiler/rustc_ast_passes/src/feature_gate.rs
&format!("unrecognized ABI not caught in lowering: {}", abi),
);
if self.sess.opts.pretty.map_or(true, |ppm| ppm.needs_hir()) {
self.sess.parse_sess.span_diagnostic.delay_span_bug(

@compiler-errorscompiler-errorsJul 17, 2022•
edited
Loading

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Question: Do we want to error when we have an invalid ABI and we don't go thru HIR lowering, like with -Zunpretty=expanded?

Since we're not going thru HIR lowering, we don't emit the error at the normal place we expect it, but if we want, we could emit an error instead of delaying an error here, depending on the pretty print mode.

Right now this just allows invalid ABI strings, but I have no strong opinions either way.

@fee1-deadfee1-deadJul 17, 2022

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.

Per the reference there should be no ast-level constraint for abi. I think it is fine to leave it as is in ast.

@fee1-dead

fee1-dead commented Jul 17, 2022

Copy link
Copy Markdown
Member

@bors r+

@bors

bors commented Jul 17, 2022

Copy link
Copy Markdown
Collaborator

📌 Commit 26ecd44 has been approved by fee1-dead

It is now in the queue for this repository.

@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 Jul 17, 2022
JohnTitor added a commit to JohnTitor/rust that referenced this pull request Jul 17, 2022
@JohnTitor
Rollup merge ofrust-lang#99360- compiler-errors:issue-99331, r=fee1…
af53fea
…-dead
Do not ICE when we have `-Zunpretty=expanded` with invalid ABI
Fixesrust-lang#99331
This was referenced Jul 17, 2022
Closed
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Jul 18, 2022
@bors
Auto merge ofrust-lang#99391- JohnTitor:rollup-tdigzzo, r=JohnTitor
e1d9a20
Rollup of 6 pull requests
Successful merges:
- rust-lang#98383 (Remove restrictions on compare-exchange memory ordering.)
- rust-lang#99350 (Be more precise when suggesting removal of parens on unit ctor)
- rust-lang#99356 (Do not constraint TAITs when checking impl/trait item compatibility)
- rust-lang#99360 (Do not ICE when we have `-Zunpretty=expanded` with invalid ABI)
- rust-lang#99373 (Fix source code sidebar tree auto-expand)
- rust-lang#99374 (Fix doc for `rchunks_exact`)
Failed merges:
r? `@ghost`
`@rustbot` modify labels: rollup
@bors
bors merged commit f3a458f into rust-lang:masterJul 18, 2022
@rustbotrustbot added this to the 1.64.0 milestone Jul 18, 2022
@compiler-errors
compiler-errors deleted the issue-99331 branch August 11, 2023 20:05
@ZalatharZalathar mentioned this pull request Mar 4, 2025
Merged
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Mar 4, 2025
@matthiaskrgr
Rollup merge ofrust-lang#137975- Zalathar:needs-hir, r=compiler-errors
70372db
Remove unused `PpMode::needs_hir`
This method was added in rust-lang#99360 to avoid an overzealous `span_delayed_bug` ICE in specific circumstances, but the only caller was subsequently removed in rust-lang#136603, which presumably avoids the problem in a more principled way.
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Mar 4, 2025
@matthiaskrgr
Rollup merge ofrust-lang#137975- Zalathar:needs-hir, r=compiler-errors
98c6bd4
Remove unused `PpMode::needs_hir`
This method was added in rust-lang#99360 to avoid an overzealous `span_delayed_bug` ICE in specific circumstances, but the only caller was subsequently removed in rust-lang#136603, which presumably avoids the problem in a more principled way.
tgross35 added a commit to tgross35/rust that referenced this pull request Mar 4, 2025
@tgross35
Rollup merge ofrust-lang#137975- Zalathar:needs-hir, r=compiler-errors
8ff60c2
Remove unused `PpMode::needs_hir`
This method was added in rust-lang#99360 to avoid an overzealous `span_delayed_bug` ICE in specific circumstances, but the only caller was subsequently removed in rust-lang#136603, which presumably avoids the problem in a more principled way.
workingjubilee added a commit to workingjubilee/rustc that referenced this pull request Mar 4, 2025
@workingjubilee
Rollup merge ofrust-lang#137975- Zalathar:needs-hir, r=compiler-errors
05823f0
Remove unused `PpMode::needs_hir`
This method was added in rust-lang#99360 to avoid an overzealous `span_delayed_bug` ICE in specific circumstances, but the only caller was subsequently removed in rust-lang#136603, which presumably avoids the problem in a more principled way.
workingjubilee added a commit to workingjubilee/rustc that referenced this pull request Mar 5, 2025
@workingjubilee
Rollup merge ofrust-lang#137975- Zalathar:needs-hir, r=compiler-errors
dfbd9b2
Remove unused `PpMode::needs_hir`
This method was added in rust-lang#99360 to avoid an overzealous `span_delayed_bug` ICE in specific circumstances, but the only caller was subsequently removed in rust-lang#136603, which presumably avoids the problem in a more principled way.
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Mar 5, 2025
@rust-timer
Unrolled build forrust-lang#137975
09a7b94
Rollup merge of rust-lang#137975 - Zalathar:needs-hir, r=compiler-errors
Remove unused `PpMode::needs_hir`
This method was added in rust-lang#99360 to avoid an overzealous `span_delayed_bug` ICE in specific circumstances, but the only caller was subsequently removed in rust-lang#136603, which presumably avoids the problem in a more principled way.
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@fee1-deadfee1-deadfee1-dead approved these changes

Assignees

@fee1-deadfee1-dead

Labels

S-waiting-on-borsStatus: Waiting on bors to run and complete tests. Bors will change the label on completion.T-compilerRelevant to the compiler team, which will review and decide on the PR/issue.

Projects

None yet

Milestone

1.64.0

Development

Successfully merging this pull request may close these issues.

ICE: compiler error is promoted to ICE with -Zunpretty=expanded: "unrecognized ABI not caught in lowering"

5 participants

@compiler-errors@rust-highfive@fee1-dead@bors@rustbot