Skip to content

Move try_to_bits from Const to Valtree#136130

Closed
FedericoBruzzone wants to merge 1 commit intorust-lang:masterfrom
FedericoBruzzone:follow-up-135158
Closed

Move try_to_bits from Const to Valtree#136130
FedericoBruzzone wants to merge 1 commit intorust-lang:masterfrom
FedericoBruzzone:follow-up-135158

Conversation

@FedericoBruzzone
Copy link
Contributor

@FedericoBruzzone FedericoBruzzone commented Jan 27, 2025

This PR is a follow-up to #135158

What's in this PR?

  • Move try_to_bits: Transitioned from Const to Valtree.
  • Rename validate_const_with_value: Renamed to extract_valtree_and_ty for better clarity.

Benefits

r? @lukas-code @oli-obk

@rustbot rustbot added PG-exploit-mitigations Project group: Exploit mitigations 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 Jan 27, 2025
@rustbot
Copy link
Collaborator

rustbot commented Jan 27, 2025

Some changes occurred in compiler/rustc_sanitizers

cc @rust-lang/project-exploit-mitigations, @rcvalle

@rust-log-analyzer

This comment has been minimized.

Sign in to view
lukas-code
lukas-code reviewed Jan 27, 2025
View reviewed changes
Copy link
Member

@lukas-code lukas-code left a comment

Choose a reason for hiding this comment

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

IMO we should eventually add a proper type for type-level const values rather that use a tuple of (Ty, ValTree) everywhere, but this PR is already an improvement :)

@FedericoBruzzone
Copy link
Contributor Author

FedericoBruzzone commented Jan 27, 2025

IMO we should eventually add a proper type for type-level const values rather that use a tuple of (Ty, ValTree) everywhere, but this PR is already an improvement :)

Currently, I introduced the proper type (ValTreeAndTy) locally to compiler/rustc_ty_utils/src/layout.rs. Let me know if a better solution could be move it externally.

@rust-log-analyzer

This comment has been minimized.

Sign in to view
@FedericoBruzzone
Move try_to_bits from Const to Valtree
9783e80 
Signed-off-by: FedericoBruzzone <federico.bruzzone.i@gmail.com>
oli-obk
oli-obk reviewed Jan 28, 2025
View reviewed changes
compiler/rustc_ty_utils/src/layout.rs
Comment on lines +37 to +40
struct ValTreeAndTy<'tcx> {
valtree: ty::ValTree<'tcx>,
ty: Ty<'tcx>,
}
Copy link
Contributor

@oli-obk oli-obk Jan 28, 2025

Choose a reason for hiding this comment

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

cc @BoxyUwU this is a practical type we could use in more places, and implement methods like try_to_bits directly on, to avoid having to pass the type separately

Copy link
Member

@BoxyUwU BoxyUwU Jan 28, 2025

Choose a reason for hiding this comment

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

Ah yeah sorry this was intended to be follow up work from the original PR that removed Ty from ty::Const but it just didn't wind up happening 👍

Copy link
Member

@lukas-code lukas-code Jan 28, 2025

Choose a reason for hiding this comment

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

I have a branch that implements this PR's approach more globally (master...lukas-code:rust:typed-valtree), will send PR soon.

Copy link
Contributor Author

@FedericoBruzzone FedericoBruzzone Jan 28, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

I will look at that PR as soon as possible.

@lukas-code lukas-code mentioned this pull request Jan 28, 2025
Merged
@FedericoBruzzone
Copy link
Contributor Author

FedericoBruzzone commented Jan 30, 2025

Closed in favor of #136180

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Jan 30, 2025
@matthiaskrgr
Rollup merge of rust-lang#136180 - lukas-code:typed-valtree, r=oli-obk
6a66a27 
Introduce a wrapper for "typed valtrees" and properly check the type before extracting the value

This PR adds a new wrapper type `ty::Value` to replace the tuple `(Ty, ty::ValTree)` and become the new canonical representation of type-level constant values.

The value extraction methods `try_to_bits`/`try_to_bool`/`try_to_target_usize` are moved to this new type. For `try_to_bits` in particular, this avoids some redundant matches on `ty::ConstKind::Value`. Furthermore, these methods and will now properly check the type before extracting the value, which fixes some ICEs.

The name `ty::Value` was chosen to be consistent with `ty::Expr`.

Commit 1 should be non-functional and commit 2 adds the type check.

---

fixes rust-lang#131102
supercedes rust-lang#136130

r? `@oli-obk`
cc `@FedericoBruzzone` `@BoxyUwU`
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Jan 31, 2025
@rust-timer
Unrolled build for rust-lang#136180
77215d3 
Rollup merge of rust-lang#136180 - lukas-code:typed-valtree, r=oli-obk

Introduce a wrapper for "typed valtrees" and properly check the type before extracting the value

This PR adds a new wrapper type `ty::Value` to replace the tuple `(Ty, ty::ValTree)` and become the new canonical representation of type-level constant values.

The value extraction methods `try_to_bits`/`try_to_bool`/`try_to_target_usize` are moved to this new type. For `try_to_bits` in particular, this avoids some redundant matches on `ty::ConstKind::Value`. Furthermore, these methods and will now properly check the type before extracting the value, which fixes some ICEs.

The name `ty::Value` was chosen to be consistent with `ty::Expr`.

Commit 1 should be non-functional and commit 2 adds the type check.

---

fixes rust-lang#131102
supercedes rust-lang#136130

r? `@oli-obk`
cc `@FedericoBruzzone` `@BoxyUwU`
github-actions bot pushed a commit to rust-lang/miri that referenced this pull request Jan 31, 2025
@matthiaskrgr
Rollup merge of #136180 - lukas-code:typed-valtree, r=oli-obk
6cce6cb 
Introduce a wrapper for "typed valtrees" and properly check the type before extracting the value

This PR adds a new wrapper type `ty::Value` to replace the tuple `(Ty, ty::ValTree)` and become the new canonical representation of type-level constant values.

The value extraction methods `try_to_bits`/`try_to_bool`/`try_to_target_usize` are moved to this new type. For `try_to_bits` in particular, this avoids some redundant matches on `ty::ConstKind::Value`. Furthermore, these methods and will now properly check the type before extracting the value, which fixes some ICEs.

The name `ty::Value` was chosen to be consistent with `ty::Expr`.

Commit 1 should be non-functional and commit 2 adds the type check.

---

fixes rust-lang/rust#131102
supercedes rust-lang/rust#136130

r? `@oli-obk`
cc `@FedericoBruzzone` `@BoxyUwU`
@FedericoBruzzone FedericoBruzzone deleted the follow-up-135158 branch February 1, 2025 11:34
christian-schilling pushed a commit to christian-schilling/rustc_codegen_cranelift that referenced this pull request Jan 27, 2026
@matthiaskrgr
Rollup merge of #136180 - lukas-code:typed-valtree, r=oli-obk
d1d7825 
Introduce a wrapper for "typed valtrees" and properly check the type before extracting the value

This PR adds a new wrapper type `ty::Value` to replace the tuple `(Ty, ty::ValTree)` and become the new canonical representation of type-level constant values.

The value extraction methods `try_to_bits`/`try_to_bool`/`try_to_target_usize` are moved to this new type. For `try_to_bits` in particular, this avoids some redundant matches on `ty::ConstKind::Value`. Furthermore, these methods and will now properly check the type before extracting the value, which fixes some ICEs.

The name `ty::Value` was chosen to be consistent with `ty::Expr`.

Commit 1 should be non-functional and commit 2 adds the type check.

---

fixes rust-lang/rust#131102
supercedes rust-lang/rust#136130

r? `@oli-obk`
cc `@FedericoBruzzone` `@BoxyUwU`
christian-schilling pushed a commit to christian-schilling/rustc_codegen_cranelift that referenced this pull request Jan 27, 2026
@matthiaskrgr
Rollup merge of #136180 - lukas-code:typed-valtree, r=oli-obk
8af5b83 
Introduce a wrapper for "typed valtrees" and properly check the type before extracting the value

This PR adds a new wrapper type `ty::Value` to replace the tuple `(Ty, ty::ValTree)` and become the new canonical representation of type-level constant values.

The value extraction methods `try_to_bits`/`try_to_bool`/`try_to_target_usize` are moved to this new type. For `try_to_bits` in particular, this avoids some redundant matches on `ty::ConstKind::Value`. Furthermore, these methods and will now properly check the type before extracting the value, which fixes some ICEs.

The name `ty::Value` was chosen to be consistent with `ty::Expr`.

Commit 1 should be non-functional and commit 2 adds the type check.

---

fixes rust-lang/rust#131102
supercedes rust-lang/rust#136130

r? `@oli-obk`
cc `@FedericoBruzzone` `@BoxyUwU`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@oli-obk oli-obk oli-obk left review comments

@BoxyUwU BoxyUwU BoxyUwU left review comments

@lukas-code lukas-code lukas-code left review comments

Assignees

@lukas-code lukas-code

Labels

PG-exploit-mitigations Project group: Exploit mitigations 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.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@FedericoBruzzone @rustbot @rust-log-analyzer @oli-obk @BoxyUwU @lukas-code