Skip to content

Forbid tail calling intrinsics #144851

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

Merged
merged 1 commit into from
Aug 4, 2025
Merged

Conversation

WaffleLapkin
Copy link
Member

@WaffleLapkin WaffleLapkin commented Aug 3, 2025

There is only one intrinsic that can be called on stable, as far as I can find, (transmute). And in general tail calling intrinsics doesn't make much sense.

Alternative to #144815 (and thus closes #144815)
Fixes #144806

r? @scottmcm

@WaffleLapkin WaffleLapkin added the F-explicit_tail_calls `#![feature(explicit_tail_calls)]` label Aug 3, 2025
@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Aug 3, 2025
Copy link
Member

@compiler-errors compiler-errors left a comment

Choose a reason for hiding this comment

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

I think using Instance::try_resolve is a bit overkill since we can just use tcx.instance(def_id).is_some(), but I think resolve is ok.

Comment on lines 108 to 109
ty::Instance::try_resolve(self.tcx, self.typing_env, did, args)
&& let InstanceKind::Intrinsic(_) = instance.def
Copy link
Member

Choose a reason for hiding this comment

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

importing InstanceKind but not Instance is a bit weird 🤔

@compiler-errors
Copy link
Member

r=me unless you want a specific review

Comment on lines 107 to 109
if let Ok(Some(instance)) =
ty::Instance::try_resolve(self.tcx, self.typing_env, did, args)
&& let ty::InstanceKind::Intrinsic(_) = instance.def
Copy link
Member

Choose a reason for hiding this comment

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

Do you need the resolve here? Can probably just do https://doc.rust-lang.org/nightly/nightly-rustc/rustc_middle/ty/context/struct.TyCtxt.html#method.intrinsic.is_some()?

(Since you never get to an intrinsic through a trait impl or whatever.)

Copy link
Member

Choose a reason for hiding this comment

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

Doh, I somehow completely missed that c-e already said this 🤦

Copy link
Member

@scottmcm scottmcm left a comment

Choose a reason for hiding this comment

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

LGTM as well, with or without the change I mentioned.

I like the approach of giving a real error here -- since intrinsics are just expanded directly, there's no need to ever become them, as you say.

@WaffleLapkin
Copy link
Member Author

@bors r=compiler-errors,scottmcm rollup

@bors
Copy link
Collaborator

bors commented Aug 3, 2025

📌 Commit c539890 has been approved by compiler-errors,scottmcm

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 Aug 3, 2025
samueltardieu added a commit to samueltardieu/rust that referenced this pull request Aug 3, 2025
…mpiler-errors,scottmcm

Forbid tail calling intrinsics

There is only one intrinsic that can be called on stable, as far as I can find, (`transmute`). And in general tail calling intrinsics doesn't make much sense.

Alternative to rust-lang#144815 (and thus closes rust-lang#144815)
Fixes rust-lang#144806

r? `@scottmcm`
bors added a commit that referenced this pull request Aug 3, 2025
Rollup of 12 pull requests

Successful merges:

 - #142678 (Misc cleanups of `generic_arg_infer` related HIR logic)
 - #144070 (Implement `hash_map` macro )
 - #144738 (Remove the omit_gdb_pretty_printer_section attribute)
 - #144790 (Multiple bounds checking elision failures)
 - #144805 (compiletest: Preliminary cleanup of `ProcRes` printing/unwinding)
 - #144808 (`Interner` arg to `EarlyBinder` does not affect auto traits)
 - #144816 (Update E0562 to account for the new impl trait positions)
 - #144822 (Return a struct with named fields from `hash_owner_nodes`)
 - #144824 (Updated test links in compiler)
 - #144829 (Use full flag name in strip command for Darwin)
 - #144843 (Weekly `cargo update`)
 - #144851 (Forbid tail calling intrinsics)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 018c172 into rust-lang:master Aug 4, 2025
10 checks passed
rust-timer added a commit that referenced this pull request Aug 4, 2025
Rollup merge of #144851 - WaffleLapkin:instrinsic-deny, r=compiler-errors,scottmcm

Forbid tail calling intrinsics

There is only one intrinsic that can be called on stable, as far as I can find, (`transmute`). And in general tail calling intrinsics doesn't make much sense.

Alternative to #144815 (and thus closes #144815)
Fixes #144806

r? ``@scottmcm``
@rustbot rustbot added this to the 1.91.0 milestone Aug 4, 2025
github-actions bot pushed a commit to rust-lang/rustc-dev-guide that referenced this pull request Aug 4, 2025
Rollup of 12 pull requests

Successful merges:

 - rust-lang/rust#142678 (Misc cleanups of `generic_arg_infer` related HIR logic)
 - rust-lang/rust#144070 (Implement `hash_map` macro )
 - rust-lang/rust#144738 (Remove the omit_gdb_pretty_printer_section attribute)
 - rust-lang/rust#144790 (Multiple bounds checking elision failures)
 - rust-lang/rust#144805 (compiletest: Preliminary cleanup of `ProcRes` printing/unwinding)
 - rust-lang/rust#144808 (`Interner` arg to `EarlyBinder` does not affect auto traits)
 - rust-lang/rust#144816 (Update E0562 to account for the new impl trait positions)
 - rust-lang/rust#144822 (Return a struct with named fields from `hash_owner_nodes`)
 - rust-lang/rust#144824 (Updated test links in compiler)
 - rust-lang/rust#144829 (Use full flag name in strip command for Darwin)
 - rust-lang/rust#144843 (Weekly `cargo update`)
 - rust-lang/rust#144851 (Forbid tail calling intrinsics)

r? `@ghost`
`@rustbot` modify labels: rollup
@WaffleLapkin WaffleLapkin deleted the instrinsic-deny branch August 4, 2025 06:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
F-explicit_tail_calls `#![feature(explicit_tail_calls)]` 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.

ICE from tail call to transmute
5 participants