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

rename mir::Constant -> mir::ConstOperand, mir::ConstKind -> mir::Const #115972

Merged
merged 2 commits into from
Sep 21, 2023

Conversation

RalfJung
Copy link
Member

Also, be more consistent with the to/eval_bits methods... we had some that take a type and some that take a size, and then sometimes the one that takes a type is called bits_for_ty.

Turns out that ty::Const/mir::ConstKind carry their type with them, so we don't need to even pass the type to those eval_bits functions at all.

However this is not properly consistent yet: in ty we have most of the methods on ty::Const, but in mir we have them on mir::ConstKind. And indeed those two types are the ones that correspond to each other. So mir::ConstantKind should actually be renamed to mir::Const. But what to do with mir::Constant? It carries around a span, that's really more like a constant operand that appears as a MIR operand... it's more suited for syntax.rs than consts.rs, but the bigger question is, which name should it get if we want to align the mir and ty types? ConstOperand? ConstOp? Literal? It's not a literal but it has a field called literal so it would at least be consistently wrong-ish...

@oli-obk any ideas?

@rustbot
Copy link
Collaborator

rustbot commented Sep 19, 2023

r? @petrochenkov

(rustbot has picked a reviewer for you, use r? to override)

@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. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. labels Sep 19, 2023
@oli-obk
Copy link
Contributor

oli-obk commented Sep 19, 2023

r? @oli-obk

ConstOperand seems best to me.

the literal field name is just an artifact from when MIR was created. we could rename it to constant

@rustbot rustbot assigned oli-obk and unassigned petrochenkov Sep 19, 2023
@rust-log-analyzer

This comment has been minimized.

@bjorn3
Copy link
Member

bjorn3 commented Sep 20, 2023

(why did triagebot not ping me for the cg_clif changes?)

@RalfJung
Copy link
Member Author

It didn't ping anyone for the interpreter changes either. rustbot just entirely missed this PR in terms of pinging people.

@rustbot
Copy link
Collaborator

rustbot commented Sep 20, 2023

Some changes occurred to MIR optimizations

cc @rust-lang/wg-mir-opt

Some changes occurred in compiler/rustc_codegen_cranelift

cc @bjorn3

This PR changes MIR

cc @oli-obk, @RalfJung, @JakobDegen, @davidtwco, @celinval, @vakaras

This PR changes Stable MIR

cc @oli-obk, @celinval, @spastorino

Some changes occurred in src/tools/clippy

cc @rust-lang/clippy

Some changes might have occurred in exhaustiveness checking

cc @Nadrieril

Some changes occurred to the CTFE / Miri engine

cc @rust-lang/miri

@RalfJung RalfJung changed the title rename mir::Constant -> mir::Const rename mir::Constant -> mir::ConstOperand, mir::ConstKind -> mir::Const Sep 20, 2023
@RalfJung
Copy link
Member Author

ConstOperand seems best to me.

Done that.

the literal field name is just an artifact from when MIR was created. we could rename it to constant

I went with const_ to match the type name.

@rust-log-analyzer

This comment has been minimized.

@oli-obk
Copy link
Contributor

oli-obk commented Sep 21, 2023

@bors r+ p=1 bitrotty

@bors
Copy link
Contributor

bors commented Sep 21, 2023

📌 Commit c94410c has been approved by oli-obk

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 21, 2023
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request Sep 21, 2023
rename mir::Constant -> mir::ConstOperand, mir::ConstKind -> mir::Const

Also, be more consistent with the `to/eval_bits` methods... we had some that take a type and some that take a size, and then sometimes the one that takes a type is called `bits_for_ty`.

Turns out that `ty::Const`/`mir::ConstKind` carry their type with them, so we don't need to even pass the type to those `eval_bits` functions at all.

However this is not properly consistent yet: in `ty` we have most of the methods on `ty::Const`, but in `mir` we have them on `mir::ConstKind`. And indeed those two types are the ones that correspond to each other. So `mir::ConstantKind` should actually be renamed to `mir::Const`. But what to do with `mir::Constant`? It carries around a span, that's really more like a constant operand that appears as a MIR operand... it's more suited for `syntax.rs` than `consts.rs`, but the bigger question is, which name should it get if we want to align the `mir` and `ty` types? `ConstOperand`? `ConstOp`? `Literal`? It's not a literal but it has a field called `literal` so it would at least be consistently wrong-ish...

`@oli-obk` any ideas?
bors added a commit to rust-lang-ci/rust that referenced this pull request Sep 21, 2023
…llaumeGomez

Rollup of 5 pull requests

Successful merges:

 - rust-lang#104385 (Raise minimum supported Apple OS versions)
 - rust-lang#115936 (Prevent promotion of const fn calls in inline consts)
 - rust-lang#115972 (rename mir::Constant -> mir::ConstOperand, mir::ConstKind -> mir::Const)
 - rust-lang#116007 (Call panic_display directly in const_panic_fmt.)
 - rust-lang#116019 (Delete obsolete `--disable-per-crate-search` rustdoc flag)

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit to rust-lang-ci/rust that referenced this pull request Sep 21, 2023
…llaumeGomez

Rollup of 5 pull requests

Successful merges:

 - rust-lang#115257 (Improve invalid UTF-8 lint by finding the expression initializer)
 - rust-lang#115936 (Prevent promotion of const fn calls in inline consts)
 - rust-lang#115972 (rename mir::Constant -> mir::ConstOperand, mir::ConstKind -> mir::Const)
 - rust-lang#116007 (Call panic_display directly in const_panic_fmt.)
 - rust-lang#116019 (Delete obsolete `--disable-per-crate-search` rustdoc flag)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 208f6ed into rust-lang:master Sep 21, 2023
11 checks passed
@bors
Copy link
Contributor

bors commented Sep 21, 2023

⌛ Testing commit c94410c with merge 66ab7e6...

@rustbot rustbot added this to the 1.74.0 milestone Sep 21, 2023
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Sep 21, 2023
Rollup merge of rust-lang#115972 - RalfJung:const-consistency, r=oli-obk

rename mir::Constant -> mir::ConstOperand, mir::ConstKind -> mir::Const

Also, be more consistent with the `to/eval_bits` methods... we had some that take a type and some that take a size, and then sometimes the one that takes a type is called `bits_for_ty`.

Turns out that `ty::Const`/`mir::ConstKind` carry their type with them, so we don't need to even pass the type to those `eval_bits` functions at all.

However this is not properly consistent yet: in `ty` we have most of the methods on `ty::Const`, but in `mir` we have them on `mir::ConstKind`. And indeed those two types are the ones that correspond to each other. So `mir::ConstantKind` should actually be renamed to `mir::Const`. But what to do with `mir::Constant`? It carries around a span, that's really more like a constant operand that appears as a MIR operand... it's more suited for `syntax.rs` than `consts.rs`, but the bigger question is, which name should it get if we want to align the `mir` and `ty` types? `ConstOperand`? `ConstOp`? `Literal`? It's not a literal but it has a field called `literal` so it would at least be consistently wrong-ish...

``@oli-obk`` any ideas?
@RalfJung RalfJung deleted the const-consistency branch September 21, 2023 14:15
@RalfJung
Copy link
Member Author

FWIW smir still uses the old names, I was not sure what the process was for changing those. We should align them with the new internal names eventually, I hope.

@oli-obk
Copy link
Contributor

oli-obk commented Sep 21, 2023

Yea, we'll clean up on the SMIR side ourselves

flip1995 pushed a commit to flip1995/rust that referenced this pull request Sep 25, 2023
rename mir::Constant -> mir::ConstOperand, mir::ConstKind -> mir::Const

Also, be more consistent with the `to/eval_bits` methods... we had some that take a type and some that take a size, and then sometimes the one that takes a type is called `bits_for_ty`.

Turns out that `ty::Const`/`mir::ConstKind` carry their type with them, so we don't need to even pass the type to those `eval_bits` functions at all.

However this is not properly consistent yet: in `ty` we have most of the methods on `ty::Const`, but in `mir` we have them on `mir::ConstKind`. And indeed those two types are the ones that correspond to each other. So `mir::ConstantKind` should actually be renamed to `mir::Const`. But what to do with `mir::Constant`? It carries around a span, that's really more like a constant operand that appears as a MIR operand... it's more suited for `syntax.rs` than `consts.rs`, but the bigger question is, which name should it get if we want to align the `mir` and `ty` types? `ConstOperand`? `ConstOp`? `Literal`? It's not a literal but it has a field called `literal` so it would at least be consistently wrong-ish...

``@oli-obk`` any ideas?
bjorn3 pushed a commit to bjorn3/rust that referenced this pull request Oct 9, 2023
rename mir::Constant -> mir::ConstOperand, mir::ConstKind -> mir::Const

Also, be more consistent with the `to/eval_bits` methods... we had some that take a type and some that take a size, and then sometimes the one that takes a type is called `bits_for_ty`.

Turns out that `ty::Const`/`mir::ConstKind` carry their type with them, so we don't need to even pass the type to those `eval_bits` functions at all.

However this is not properly consistent yet: in `ty` we have most of the methods on `ty::Const`, but in `mir` we have them on `mir::ConstKind`. And indeed those two types are the ones that correspond to each other. So `mir::ConstantKind` should actually be renamed to `mir::Const`. But what to do with `mir::Constant`? It carries around a span, that's really more like a constant operand that appears as a MIR operand... it's more suited for `syntax.rs` than `consts.rs`, but the bigger question is, which name should it get if we want to align the `mir` and `ty` types? `ConstOperand`? `ConstOp`? `Literal`? It's not a literal but it has a field called `literal` so it would at least be consistently wrong-ish...

``@oli-obk`` any ideas?
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. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants