rename mir::Constant -> mir::ConstOperand, mir::ConstKind -> mir::Const by RalfJung · Pull Request #115972 · rust-lang/rust

RalfJung · 2023-09-19T15:52:32Z

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 · 2023-09-19T15:52:41Z

r? @petrochenkov

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

oli-obk · 2023-09-19T15:56:13Z

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

bjorn3 · 2023-09-20T14:40:56Z

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

RalfJung · 2023-09-20T16:40:19Z

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

rustbot · 2023-09-20T18:51:35Z

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 · 2023-09-20T18:52:20Z

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.

oli-obk · 2023-09-21T08:10:49Z

@bors r+ p=1 bitrotty

bors · 2023-09-21T08:10:51Z

📌 Commit c94410c has been approved by oli-obk

It is now in the queue for this repository.

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?

…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

…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 · 2023-09-21T14:02:50Z

⌛ Testing commit c94410c with merge 66ab7e6...

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 · 2023-09-21T15:19:36Z

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 · 2023-09-21T15:23:38Z

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

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?

rustbot assigned petrochenkov Sep 19, 2023

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

rustbot assigned oli-obk and unassigned petrochenkov Sep 19, 2023

This comment has been minimized.

Sign in to view

the Const::eval_bits methods don't need to be given the Ty

a2374e6

RalfJung mentioned this pull request Sep 20, 2023

rustbot failed to ping people in a PR rust-lang/triagebot#1723

Open

RalfJung force-pushed the const-consistency branch from 1ef572a to 5f773d2 Compare September 20, 2023 18:51

RalfJung changed the title ~~rename mir::Constant -> mir::Const~~ rename mir::Constant -> mir::ConstOperand, mir::ConstKind -> mir::Const Sep 20, 2023

This comment has been minimized.

Sign in to view

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

c94410c

RalfJung force-pushed the const-consistency branch from 5f773d2 to c94410c Compare September 21, 2023 06: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-review Status: Awaiting review from the assignee but also interested parties. labels Sep 21, 2023

GuillaumeGomez mentioned this pull request Sep 21, 2023

Rollup of 5 pull requests #116025

Closed

GuillaumeGomez mentioned this pull request Sep 21, 2023

Rollup of 5 pull requests #116027

Merged

bors merged commit 208f6ed into rust-lang:master Sep 21, 2023

rustbot added this to the 1.74.0 milestone Sep 21, 2023

RalfJung deleted the const-consistency branch September 21, 2023 14:15

rust-timer mentioned this pull request Oct 9, 2023

Rollup of 6 pull requests #116569

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

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

rename mir::Constant -> mir::ConstOperand, mir::ConstKind -> mir::Const#115972
bors merged 2 commits intorust-lang:masterfrom
RalfJung:const-consistency

RalfJung commented Sep 19, 2023

Uh oh!

rustbot commented Sep 19, 2023

Uh oh!

oli-obk commented Sep 19, 2023

Uh oh!

This comment has been minimized.

bjorn3 commented Sep 20, 2023

Uh oh!

RalfJung commented Sep 20, 2023

Uh oh!

rustbot commented Sep 20, 2023

Uh oh!

RalfJung commented Sep 20, 2023

Uh oh!

This comment has been minimized.

oli-obk commented Sep 21, 2023

Uh oh!

bors commented Sep 21, 2023

Uh oh!

bors commented Sep 21, 2023

Uh oh!

RalfJung commented Sep 21, 2023

Uh oh!

oli-obk commented Sep 21, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

Uh oh!

Conversation

RalfJung commented Sep 19, 2023

Uh oh!

rustbot commented Sep 19, 2023

Uh oh!

oli-obk commented Sep 19, 2023

Uh oh!

This comment has been minimized.

bjorn3 commented Sep 20, 2023

Uh oh!

RalfJung commented Sep 20, 2023

Uh oh!

rustbot commented Sep 20, 2023

Uh oh!

RalfJung commented Sep 20, 2023

Uh oh!

This comment has been minimized.

oli-obk commented Sep 21, 2023

Uh oh!

bors commented Sep 21, 2023

Uh oh!

bors commented Sep 21, 2023

Uh oh!

RalfJung commented Sep 21, 2023

Uh oh!

oli-obk commented Sep 21, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants