Give `global_asm` a fake body to store typeck results, represent `sym fn` as a hir expr to fix `sym fn` operands with lifetimes #137180

compiler-errors · 2025-02-17T16:40:18Z

There are a few intertwined problems with sym fn operands in both inline and global asm macros.

Specifically, unlike other anon consts, they may evaluate to a type with free regions in them without actually having an item-level type annotation to give them a "proper" type. This is in contrast to named constants, which always have an item-level type annotation, or unnamed constants which are constrained by their position (e.g. a const arg in a turbofish, or a const array length).

Today, we infer the type of the operand by looking at the HIR typeck results; however, those results are region-erased, so during borrowck we ICE since we don't expect to encounter erased regions. We can't just fill this type with something like 'static, since we may want to use real (free) regions:

fn foo<'a>() {
  asm!("/* ... */", sym bar::<&'a ()>);
}

The first idea may be to represent sym fn operands using inline consts instead of anon consts. This makes sense, since inline consts can reference regions from the parent body (like the 'a in the example above). However, this introduces a problem with global_asm!, which doesn't have a parent body; inline consts must be associated with a parent body since they are not a body owner of their own. In #116087, I attempted to fix this by using two separate sym operands for global and inline asm. However, this led to a lot of confusion and also some unattractive code duplication.

In this PR, I adjust the lowering of global_asm! so that it's lowered in a "fake" HIR body. This body contains a single expression which is ExprKind::InlineAsm; we don't use this HIR body, but it's used in typeck and borrowck so that we can properly infer and validate the the lifetimes of sym fn operands.

I then adjust the lowering of sym fn to instead be represented with a HIR expression. This is both because it's no longer necessary to represent this operand as an anon const, since it's just a path expression, and also more importantly to sidestep yet another ICE (#137179), which has to do with the existing code breaking an invariant of def-id creation and anon consts. Specifically, we are not allowed to synthesize a def-id for an anon const when that anon const contains expressions with def-ids whose parent is not that anon const. This is somewhat related to #130443 (comment), which is also a place in the compiler where synthesizing anon consts leads to def-id parenting issue.

As a side-effect, this consolidates the type checking for inline and global asm, so it allows us to simplify InlineAsmCtxt a bit. It also allows us to delete a bit of hacky code from anon const type_of which was there to detect sym fn operands specifically. This also could be generalized to support const asm operands with types with lifetimes in them. Since we specifically reject these consts today, I'm not going to change the representation of those consts (but they'd just be turned into inline consts).

r? oli-obk -- mostly b/c you're patient and also understand the breadth of the code that this touches, please reassign if you don't want to review this.

Fixes #111709
Fixes #96304
Fixes #137179

rustbot · 2025-02-17T16:40:30Z

Some changes occurred in compiler/rustc_codegen_cranelift

cc @bjorn3

Some changes occurred in src/tools/clippy

cc @rust-lang/clippy

oli-obk · 2025-02-17T17:02:33Z

I was sold when I read the title 😅. Gonna review some time this week when the flue hopefully has finished running its course through my household.

bors · 2025-02-18T08:45:33Z

☔ The latest upstream changes (presumably #137162) made this pull request unmergeable. Please resolve the merge conflicts.

oli-obk

Subtle, but I don't think this is fragile or a maintenance burden. Some nice improvements, some funny "workarounds"

oli-obk · 2025-02-21T14:24:02Z

@bors r+

bors · 2025-02-21T14:24:06Z

📌 Commit 38c2bbf has been approved by oli-obk

It is now in the queue for this repository.

…-obk Give `global_asm` a fake body to store typeck results, represent `sym fn` as a hir expr to fix `sym fn` operands with lifetimes There are a few intertwined problems with `sym fn` operands in both inline and global asm macros. Specifically, unlike other anon consts, they may evaluate to a type with free regions in them without actually having an item-level type annotation to give them a "proper" type. This is in contrast to named constants, which always have an item-level type annotation, or unnamed constants which are constrained by their position (e.g. a const arg in a turbofish, or a const array length). Today, we infer the type of the operand by looking at the HIR typeck results; however, those results are region-erased, so during borrowck we ICE since we don't expect to encounter erased regions. We can't just fill this type with something like `'static`, since we may want to use real (free) regions: ```rust fn foo<'a>() { asm!("/* ... */", sym bar::<&'a ()>); } ``` The first idea may be to represent `sym fn` operands using *inline* consts instead of anon consts. This makes sense, since inline consts can reference regions from the parent body (like the `'a` in the example above). However, this introduces a problem with `global_asm!`, which doesn't *have* a parent body; inline consts *must* be associated with a parent body since they are not a body owner of their own. In rust-lang#116087, I attempted to fix this by using two separate `sym` operands for global and inline asm. However, this led to a lot of confusion and also some unattractive code duplication. In this PR, I adjust the lowering of `global_asm!` so that it's lowered in a "fake" HIR body. This body contains a single expression which is `ExprKind::InlineAsm`; we don't *use* this HIR body, but it's used in typeck and borrowck so that we can properly infer and validate the the lifetimes of `sym fn` operands. I then adjust the lowering of `sym fn` to instead be represented with a HIR expression. This is both because it's no longer necessary to represent this operand as an anon const, since it's *just* a path expression, and also more importantly to sidestep yet another ICE (rust-lang#137179), which has to do with the existing code breaking an invariant of def-id creation and anon consts. Specifically, we are not allowed to synthesize a def-id for an anon const when that anon const contains expressions with def-ids whose parent is *not* that anon const. This is somewhat related to rust-lang#130443 (comment), which is also a place in the compiler where synthesizing anon consts leads to def-id parenting issue. As a side-effect, this consolidates the type checking for inline and global asm, so it allows us to simplify `InlineAsmCtxt` a bit. It also allows us to delete a bit of hacky code from anon const `type_of` which was there to detect `sym fn` operands specifically. This also could be generalized to support `const` asm operands with types with lifetimes in them. Since we specifically reject these consts today, I'm not going to change the representation of those consts (but they'd just be turned into inline consts). r? oli-obk -- mostly b/c you're patient and also understand the breadth of the code that this touches, please reassign if you don't want to review this. Fixes rust-lang#111709 Fixes rust-lang#96304 Fixes rust-lang#137179

…iaskrgr Rollup of 8 pull requests Successful merges: - rust-lang#136458 (Do not deduplicate list of associated types provided by dyn principal) - rust-lang#136474 ([`compiletest`-related cleanups 3/7] Make the distinction between sources root vs test suite sources root in compiletest less confusing) - rust-lang#136592 (Make sure we don't overrun the stack in canonicalizer) - rust-lang#136787 (Remove `lifetime_capture_rules_2024` feature) - rust-lang#137180 (Give `global_asm` a fake body to store typeck results, represent `sym fn` as a hir expr to fix `sym fn` operands with lifetimes) - rust-lang#137257 (Ignore fake borrows for packed field check) - rust-lang#137348 (More sophisticated span trimming for suggestions) - rust-lang#137399 (fix ICE in layout computation with unnormalizable const) r? `@ghost` `@rustbot` modify labels: rollup

matthiaskrgr · 2025-02-22T00:00:34Z

@bors r-
#137402 (comment)

compiler-errors · 2025-02-22T00:12:48Z

Added some necessary // needs-asm-support

@bors r=oli-obk

bors · 2025-02-22T00:12:50Z

📌 Commit 6ba39f7 has been approved by oli-obk

It is now in the queue for this repository.

…-obk Give `global_asm` a fake body to store typeck results, represent `sym fn` as a hir expr to fix `sym fn` operands with lifetimes There are a few intertwined problems with `sym fn` operands in both inline and global asm macros. Specifically, unlike other anon consts, they may evaluate to a type with free regions in them without actually having an item-level type annotation to give them a "proper" type. This is in contrast to named constants, which always have an item-level type annotation, or unnamed constants which are constrained by their position (e.g. a const arg in a turbofish, or a const array length). Today, we infer the type of the operand by looking at the HIR typeck results; however, those results are region-erased, so during borrowck we ICE since we don't expect to encounter erased regions. We can't just fill this type with something like `'static`, since we may want to use real (free) regions: ```rust fn foo<'a>() { asm!("/* ... */", sym bar::<&'a ()>); } ``` The first idea may be to represent `sym fn` operands using *inline* consts instead of anon consts. This makes sense, since inline consts can reference regions from the parent body (like the `'a` in the example above). However, this introduces a problem with `global_asm!`, which doesn't *have* a parent body; inline consts *must* be associated with a parent body since they are not a body owner of their own. In rust-lang#116087, I attempted to fix this by using two separate `sym` operands for global and inline asm. However, this led to a lot of confusion and also some unattractive code duplication. In this PR, I adjust the lowering of `global_asm!` so that it's lowered in a "fake" HIR body. This body contains a single expression which is `ExprKind::InlineAsm`; we don't *use* this HIR body, but it's used in typeck and borrowck so that we can properly infer and validate the the lifetimes of `sym fn` operands. I then adjust the lowering of `sym fn` to instead be represented with a HIR expression. This is both because it's no longer necessary to represent this operand as an anon const, since it's *just* a path expression, and also more importantly to sidestep yet another ICE (rust-lang#137179), which has to do with the existing code breaking an invariant of def-id creation and anon consts. Specifically, we are not allowed to synthesize a def-id for an anon const when that anon const contains expressions with def-ids whose parent is *not* that anon const. This is somewhat related to rust-lang#130443 (comment), which is also a place in the compiler where synthesizing anon consts leads to def-id parenting issue. As a side-effect, this consolidates the type checking for inline and global asm, so it allows us to simplify `InlineAsmCtxt` a bit. It also allows us to delete a bit of hacky code from anon const `type_of` which was there to detect `sym fn` operands specifically. This also could be generalized to support `const` asm operands with types with lifetimes in them. Since we specifically reject these consts today, I'm not going to change the representation of those consts (but they'd just be turned into inline consts). r? oli-obk -- mostly b/c you're patient and also understand the breadth of the code that this touches, please reassign if you don't want to review this. Fixes rust-lang#111709 Fixes rust-lang#96304 Fixes rust-lang#137179

…iaskrgr Rollup of 7 pull requests Successful merges: - rust-lang#135501 (Inject `compiler_builtins` during postprocessing and ensure it is made private) - rust-lang#136543 (intrinsics: unify rint, roundeven, nearbyint in a single round_ties_even intrinsic) - rust-lang#137121 (stabilize `(const_)ptr_sub_ptr`) - rust-lang#137180 (Give `global_asm` a fake body to store typeck results, represent `sym fn` as a hir expr to fix `sym fn` operands with lifetimes) - rust-lang#137256 (compiler: untangle SIMD alignment assumptions) - rust-lang#137298 (Check signature WF when lowering MIR body) - rust-lang#137415 (Remove invalid suggestion of into_iter for extern macro) r? `@ghost` `@rustbot` modify labels: rollup

…-obk Give `global_asm` a fake body to store typeck results, represent `sym fn` as a hir expr to fix `sym fn` operands with lifetimes There are a few intertwined problems with `sym fn` operands in both inline and global asm macros. Specifically, unlike other anon consts, they may evaluate to a type with free regions in them without actually having an item-level type annotation to give them a "proper" type. This is in contrast to named constants, which always have an item-level type annotation, or unnamed constants which are constrained by their position (e.g. a const arg in a turbofish, or a const array length). Today, we infer the type of the operand by looking at the HIR typeck results; however, those results are region-erased, so during borrowck we ICE since we don't expect to encounter erased regions. We can't just fill this type with something like `'static`, since we may want to use real (free) regions: ```rust fn foo<'a>() { asm!("/* ... */", sym bar::<&'a ()>); } ``` The first idea may be to represent `sym fn` operands using *inline* consts instead of anon consts. This makes sense, since inline consts can reference regions from the parent body (like the `'a` in the example above). However, this introduces a problem with `global_asm!`, which doesn't *have* a parent body; inline consts *must* be associated with a parent body since they are not a body owner of their own. In rust-lang#116087, I attempted to fix this by using two separate `sym` operands for global and inline asm. However, this led to a lot of confusion and also some unattractive code duplication. In this PR, I adjust the lowering of `global_asm!` so that it's lowered in a "fake" HIR body. This body contains a single expression which is `ExprKind::InlineAsm`; we don't *use* this HIR body, but it's used in typeck and borrowck so that we can properly infer and validate the the lifetimes of `sym fn` operands. I then adjust the lowering of `sym fn` to instead be represented with a HIR expression. This is both because it's no longer necessary to represent this operand as an anon const, since it's *just* a path expression, and also more importantly to sidestep yet another ICE (rust-lang#137179), which has to do with the existing code breaking an invariant of def-id creation and anon consts. Specifically, we are not allowed to synthesize a def-id for an anon const when that anon const contains expressions with def-ids whose parent is *not* that anon const. This is somewhat related to rust-lang#130443 (comment), which is also a place in the compiler where synthesizing anon consts leads to def-id parenting issue. As a side-effect, this consolidates the type checking for inline and global asm, so it allows us to simplify `InlineAsmCtxt` a bit. It also allows us to delete a bit of hacky code from anon const `type_of` which was there to detect `sym fn` operands specifically. This also could be generalized to support `const` asm operands with types with lifetimes in them. Since we specifically reject these consts today, I'm not going to change the representation of those consts (but they'd just be turned into inline consts). r? oli-obk -- mostly b/c you're patient and also understand the breadth of the code that this touches, please reassign if you don't want to review this. Fixes rust-lang#111709 Fixes rust-lang#96304 Fixes rust-lang#137179

…iaskrgr Rollup of 7 pull requests Successful merges: - rust-lang#135501 (Inject `compiler_builtins` during postprocessing and ensure it is made private) - rust-lang#136543 (intrinsics: unify rint, roundeven, nearbyint in a single round_ties_even intrinsic) - rust-lang#137121 (stabilize `(const_)ptr_sub_ptr`) - rust-lang#137180 (Give `global_asm` a fake body to store typeck results, represent `sym fn` as a hir expr to fix `sym fn` operands with lifetimes) - rust-lang#137256 (compiler: untangle SIMD alignment assumptions) - rust-lang#137383 (stabilize `unsigned_is_multiple_of`) - rust-lang#137415 (Remove invalid suggestion of into_iter for extern macro) r? `@ghost` `@rustbot` modify labels: rollup

…iaskrgr Rollup of 6 pull requests Successful merges: - rust-lang#135501 (Inject `compiler_builtins` during postprocessing and ensure it is made private) - rust-lang#137121 (stabilize `(const_)ptr_sub_ptr`) - rust-lang#137180 (Give `global_asm` a fake body to store typeck results, represent `sym fn` as a hir expr to fix `sym fn` operands with lifetimes) - rust-lang#137256 (compiler: untangle SIMD alignment assumptions) - rust-lang#137383 (stabilize `unsigned_is_multiple_of`) - rust-lang#137415 (Remove invalid suggestion of into_iter for extern macro) r? `@ghost` `@rustbot` modify labels: rollup

Rollup merge of rust-lang#137180 - compiler-errors:sym-regions, r=oli-obk Give `global_asm` a fake body to store typeck results, represent `sym fn` as a hir expr to fix `sym fn` operands with lifetimes There are a few intertwined problems with `sym fn` operands in both inline and global asm macros. Specifically, unlike other anon consts, they may evaluate to a type with free regions in them without actually having an item-level type annotation to give them a "proper" type. This is in contrast to named constants, which always have an item-level type annotation, or unnamed constants which are constrained by their position (e.g. a const arg in a turbofish, or a const array length). Today, we infer the type of the operand by looking at the HIR typeck results; however, those results are region-erased, so during borrowck we ICE since we don't expect to encounter erased regions. We can't just fill this type with something like `'static`, since we may want to use real (free) regions: ```rust fn foo<'a>() { asm!("/* ... */", sym bar::<&'a ()>); } ``` The first idea may be to represent `sym fn` operands using *inline* consts instead of anon consts. This makes sense, since inline consts can reference regions from the parent body (like the `'a` in the example above). However, this introduces a problem with `global_asm!`, which doesn't *have* a parent body; inline consts *must* be associated with a parent body since they are not a body owner of their own. In rust-lang#116087, I attempted to fix this by using two separate `sym` operands for global and inline asm. However, this led to a lot of confusion and also some unattractive code duplication. In this PR, I adjust the lowering of `global_asm!` so that it's lowered in a "fake" HIR body. This body contains a single expression which is `ExprKind::InlineAsm`; we don't *use* this HIR body, but it's used in typeck and borrowck so that we can properly infer and validate the the lifetimes of `sym fn` operands. I then adjust the lowering of `sym fn` to instead be represented with a HIR expression. This is both because it's no longer necessary to represent this operand as an anon const, since it's *just* a path expression, and also more importantly to sidestep yet another ICE (rust-lang#137179), which has to do with the existing code breaking an invariant of def-id creation and anon consts. Specifically, we are not allowed to synthesize a def-id for an anon const when that anon const contains expressions with def-ids whose parent is *not* that anon const. This is somewhat related to rust-lang#130443 (comment), which is also a place in the compiler where synthesizing anon consts leads to def-id parenting issue. As a side-effect, this consolidates the type checking for inline and global asm, so it allows us to simplify `InlineAsmCtxt` a bit. It also allows us to delete a bit of hacky code from anon const `type_of` which was there to detect `sym fn` operands specifically. This also could be generalized to support `const` asm operands with types with lifetimes in them. Since we specifically reject these consts today, I'm not going to change the representation of those consts (but they'd just be turned into inline consts). r? oli-obk -- mostly b/c you're patient and also understand the breadth of the code that this touches, please reassign if you don't want to review this. Fixes rust-lang#111709 Fixes rust-lang#96304 Fixes rust-lang#137179

nbdd0121 · 2025-02-24T10:14:24Z

Thank you a lot for doing this. When trying to implement #128464 I determined that this is exactly what is needed to make lifetimes work for const operands in global_asm, although I haven't had time to implement this change. I think you just solved one of my biggest blocker!

Cc @folkertdev

rustbot assigned oli-obk Feb 17, 2025

compiler-errors force-pushed the sym-regions branch 2 times, most recently from ca4c7d9 to c8dd164 Compare February 17, 2025 16:44

This comment has been minimized.

Sign in to view

This comment was marked as off-topic.

Sign in to view

compiler-errors force-pushed the sym-regions branch from c8dd164 to 38c2bbf Compare February 18, 2025 18:16

oli-obk approved these changes Feb 21, 2025

View reviewed changes

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 Feb 21, 2025

matthiaskrgr mentioned this pull request Feb 21, 2025

Rollup of 9 pull requests #137401

Closed

matthiaskrgr mentioned this pull request Feb 21, 2025

Rollup of 8 pull requests #137402

Closed

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 Feb 22, 2025

compiler-errors added 3 commits February 22, 2025 00:05

Make asm a named field

2a6daaf

Initial cleanups of InlineAsmCtxt

37060aa

Make a fake body to store typeck results for global_asm

6ba39f7

compiler-errors force-pushed the sym-regions branch from 38c2bbf to 6ba39f7 Compare February 22, 2025 00:12

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 Feb 22, 2025

matthiaskrgr mentioned this pull request Feb 22, 2025

Rollup of 8 pull requests #137431

Closed

matthiaskrgr mentioned this pull request Feb 22, 2025

Rollup of 7 pull requests #137433

Closed

matthiaskrgr mentioned this pull request Feb 22, 2025

Rollup of 7 pull requests #137436

Closed

matthiaskrgr mentioned this pull request Feb 22, 2025

Rollup of 6 pull requests #137446

Merged

bors merged commit 4115f51 into rust-lang:master Feb 23, 2025
6 checks passed

rustbot added this to the 1.87.0 milestone Feb 23, 2025

matthiaskrgr mentioned this pull request Feb 23, 2025

ICE: Unexpected def kind GlobalAsm #137471

Open

taiki-e mentioned this pull request Feb 24, 2025

Regression: nightly-2025-02-24 rejects raw pointer in asm! #137512

Closed

nbdd0121 mentioned this pull request Feb 26, 2025

Handle asm const similar to inline const #137686

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Give `global_asm` a fake body to store typeck results, represent `sym fn` as a hir expr to fix `sym fn` operands with lifetimes #137180

Give `global_asm` a fake body to store typeck results, represent `sym fn` as a hir expr to fix `sym fn` operands with lifetimes #137180

compiler-errors commented Feb 17, 2025 •

edited

Loading

rustbot commented Feb 17, 2025

oli-obk commented Feb 17, 2025

This comment has been minimized.

bors commented Feb 18, 2025

This comment was marked as off-topic.

oli-obk left a comment

oli-obk commented Feb 21, 2025

bors commented Feb 21, 2025

matthiaskrgr commented Feb 22, 2025

compiler-errors commented Feb 22, 2025

bors commented Feb 22, 2025

nbdd0121 commented Feb 24, 2025

Give global_asm a fake body to store typeck results, represent sym fn as a hir expr to fix sym fn operands with lifetimes #137180

Give global_asm a fake body to store typeck results, represent sym fn as a hir expr to fix sym fn operands with lifetimes #137180

Conversation

compiler-errors commented Feb 17, 2025 • edited Loading

rustbot commented Feb 17, 2025

oli-obk commented Feb 17, 2025

This comment has been minimized.

bors commented Feb 18, 2025

This comment was marked as off-topic.

oli-obk left a comment

Choose a reason for hiding this comment

oli-obk commented Feb 21, 2025

bors commented Feb 21, 2025

matthiaskrgr commented Feb 22, 2025

compiler-errors commented Feb 22, 2025

bors commented Feb 22, 2025

nbdd0121 commented Feb 24, 2025

Give `global_asm` a fake body to store typeck results, represent `sym fn` as a hir expr to fix `sym fn` operands with lifetimes #137180

Give `global_asm` a fake body to store typeck results, represent `sym fn` as a hir expr to fix `sym fn` operands with lifetimes #137180

compiler-errors commented Feb 17, 2025 •

edited

Loading