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

Use AnonConst for asm! constants #83916

Merged
merged 1 commit into from
Apr 7, 2021
Merged

Conversation

Amanieu
Copy link
Member

@Amanieu Amanieu commented Apr 6, 2021

This replaces the old system which used explicit promotion. See #83169 for more background.

The syntax for const operands is still the same as before: const <expr>.

Fixes #83169

Because the implementation is heavily based on inline consts, we suffer from the same issues:

@rust-highfive
Copy link
Collaborator

r? @petrochenkov

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

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Apr 6, 2021
@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@RalfJung
Copy link
Member

RalfJung commented Apr 6, 2021

Considering these limitations, is this still something that we want to move forward with?

Well, what are the alternatives? With array repeat expressions not being promotion points any more, and stdarch switching away from rustc_args_required_const, we are close to cutting down promotion to be just about lifetime extension (making &expr have 'static lifetime), as it was originally intended. I still think that is a worthwhile goal, so we should make sure that anything we do here is forward-compatible with such a future.

We lose the ability to use expressions derived from generics.

Note that this affects other situations as well, such as array repeat expressions. We certainly want to fix this in the future. If this is for now "good enough" elsewhere, then why not for asm!?

IMO we should just do the same kind of post-monomorphization evaluation that we do for associated consts and promoteds (AFAIK that would be rather easy to implement, we already have those codepaths for associated consts and promoteds), but @lcnr has plans to ensure that there are no post-monomorphization failures which (to my knowledge) needs a lot of design still.

@Amanieu Amanieu marked this pull request as ready for review April 6, 2021 11:25
@rust-log-analyzer

This comment has been minimized.

@petrochenkov
Copy link
Contributor

@bors r+

@bors
Copy link
Contributor

bors commented Apr 6, 2021

📌 Commit 32be124 has been approved by petrochenkov

@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 Apr 6, 2021
@petrochenkov
Copy link
Contributor

petrochenkov commented Apr 6, 2021

This PR is not about syntax, but it seems inconsistent to me that const expressions require braces outside of asm! (at least currently) but not inside of it.

If const loses its braces outside of asm!, then it will work as an unary operator (const a + b => (const a) + b), which will be inconsistent with its meaning in asm!. (See box as an example.)

On the other hand, if asm! gets a "backend" non-macro form (rust-lang/project-inline-asm#8), then it can use const { X } with braces, and surface const X can be desugared into it.
asm! can use such sugar because const ARBITRARY_EXPRESSION in it is well delimited by commas or a closing delimiter, and not by other expressions.

Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Apr 7, 2021
Use AnonConst for asm! constants

This replaces the old system which used explicit promotion. See rust-lang#83169 for more background.

The syntax for `const` operands is still the same as before: `const <expr>`.

Fixes rust-lang#83169

Because the implementation is heavily based on inline consts, we suffer from the same issues:
- We lose the ability to use expressions derived from generics. See the deleted tests in `src/test/ui/asm/const.rs`.
- We are hitting the same ICEs as inline consts, for example rust-lang#78174. It is unlikely that we will be able to stabilize this before inline consts are stabilized.
@Amanieu
Copy link
Member Author

Amanieu commented Apr 7, 2021

The way asm! operands are parsed is actually $operand_type:ident $operand_value:expr where operand_type can be one of in, out, inout, lateout, inlateout, sym, const. The const is not actually parsed as part of an expression, it's just a prefix indicating the type of operand.

bors added a commit to rust-lang-ci/rust that referenced this pull request Apr 7, 2021
Rollup of 8 pull requests

Successful merges:

 - rust-lang#83476 (Add strong_count mutation methods to Rc)
 - rust-lang#83634 (Do not emit the advanced diagnostics on macros)
 - rust-lang#83816 (Trigger `unused_doc_comments` on macros at once)
 - rust-lang#83916 (Use AnonConst for asm! constants)
 - rust-lang#83935 (forbid `impl Trait` in generic param defaults)
 - rust-lang#83936 (Disable using non-ascii identifiers in extern blocks.)
 - rust-lang#83945 (Add suggestion to reborrow mutable references when they're moved in a for loop)
 - rust-lang#83954 (Do not ICE when closure is involved in Trait Alias Impl Trait)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit b81c6cd into rust-lang:master Apr 7, 2021
@rustbot rustbot added this to the 1.53.0 milestone Apr 7, 2021
flip1995 pushed a commit to flip1995/rust that referenced this pull request Apr 8, 2021
Use AnonConst for asm! constants

This replaces the old system which used explicit promotion. See rust-lang#83169 for more background.

The syntax for `const` operands is still the same as before: `const <expr>`.

Fixes rust-lang#83169

Because the implementation is heavily based on inline consts, we suffer from the same issues:
- We lose the ability to use expressions derived from generics. See the deleted tests in `src/test/ui/asm/const.rs`.
- We are hitting the same ICEs as inline consts, for example rust-lang#78174. It is unlikely that we will be able to stabilize this before inline consts are stabilized.
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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

const operands in asm! should use inline consts rather than const promotion
8 participants