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

Tracking issue for const_size_of_val and const_align_of_val #46571

Open
Gankra opened this issue Dec 7, 2017 · 32 comments
Open

Tracking issue for const_size_of_val and const_align_of_val #46571

Gankra opened this issue Dec 7, 2017 · 32 comments
Labels
A-const-fn Area: const fn foo(..) {..}. Pure functions which can be applied at compile time. C-enhancement Category: An issue proposing an enhancement or a PR with one. disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. finished-final-comment-period The final comment period is finished for this PR / Issue. T-lang Relevant to the language team, which will review and decide on the PR/issue.

Comments

@Gankra
Copy link
Contributor

Gankra commented Dec 7, 2017

This is a tracking issue for constness of the following stable API:

// core::mem

pub const fn size_of_val<T: ?Sized>(val: &T) -> usize;
pub const fn align_of_val<T: ?Sized>(val: &T) -> usize;

It would be useful for macros to be able to do things like:

static KEY: [u8; size_of_val($byte_string)] = $byte_string;

I believe the rust-objc devs would like something like this to properly set up statics containing messageSend selectors (which are magically processed by the objc runtime by putting them in the right linker sections).

@Gankra
Copy link
Contributor Author

Gankra commented Dec 7, 2017

I believe this is currently blocked on miri becoming the primary/only CTFE provider.

@Gankra
Copy link
Contributor Author

Gankra commented Dec 7, 2017

cc @eddyb @oli-obk

@oli-obk
Copy link
Contributor

oli-obk commented Dec 7, 2017

I think this is possible without miri const eval, but it would be wasteful to implement in old ctfe

@Gankra
Copy link
Contributor Author

Gankra commented Dec 7, 2017

Yeah I consider "the people who would implement/review this are able but unwilling" to be a blocker

@pietroalbini pietroalbini added C-enhancement Category: An issue proposing an enhancement or a PR with one. A-const-fn Area: const fn foo(..) {..}. Pure functions which can be applied at compile time. labels Jan 23, 2018
@polarathene
Copy link

I believe this is currently blocked on miri becoming the primary/only CTFE provider.

Where can that status be followed? If such a transition were quite a while out, would it be possible to support with only miri and/or feature flag of some kind?

@oli-obk
Copy link
Contributor

oli-obk commented Apr 27, 2018

Miri is here. So it's mostly a matter of opening a PR by copying over the code from https://github.com/solson/miri/blob/master/miri/intrinsic.rs#L501

@Gankra
Copy link
Contributor Author

Gankra commented May 9, 2018

i'm implementing this

@kennytm
Copy link
Member

kennytm commented May 9, 2018

Making these functions const would greatly interfere with custom DST.

Custom DSTs will need the user to implement some custom functions to derive the size. Thus, semantically, making size_of_val const would mean custom DST now need to depend on the postponed rust-lang/rfcs#2337.

Implementation-wise, if we want to support size_of_val for a thin CStr, we would need a const strlen, which means Miri would need to support some selected FFI functions (outside wasm), and also need to perform loop and dereference raw pointers (for wasm target).

Therefore I'm 👎 to making size_of_val and align_of_val const until we have a concrete plan on custom DST. I don't mind having a const fn len() on [T] and str though.

@whitequark
Copy link
Member

we would need a const strlen, which means Miri would need to support some selected FFI functions (outside wasm)

Why isn't it possible to implement strlen in Rust instead?

@oli-obk
Copy link
Contributor

oli-obk commented May 9, 2018

Why isn't it possible to implement strlen in Rust instead?

This is done for wasm, but as noted, that would require loops, which have an RFC, but it's not been accepted yet: rust-lang/rfcs#2344 and additionally requires branches, which have an accepted RFC, but no implementation yet

@kennytm
Copy link
Member

kennytm commented May 9, 2018

It also requires dereferencing a raw pointer and offset it, which probably needs the unsafe guideline.

pub unsafe fn strlen(mut s: *const c_char) -> usize {
    let mut n = 0;
    while *s != 0 {      // <-- loop & deref
        n += 1;
        s = s.offset(1); // <-- raw pointer offset
    }
    return n
}

@steveklabnik
Copy link
Member

Triage: looks like the original PR didn't get merged. We've been making more and more stuff const fn, what's the current status here?

@oli-obk
Copy link
Contributor

oli-obk commented Sep 22, 2019

@Gankra #63770 solves your problem, albeit not with size_of_val

bors added a commit to rust-lang-ci/rust that referenced this issue Jul 30, 2020
…l, r=oli-obk

Make `mem::size_of_val` and `mem::align_of_val` unstably const

Implements rust-lang#46571 but does not stabilize it. I wanted this while working on something today.

The only reason not to immediately stabilize are concerns around [custom DSTs](rust-lang#46571 (comment)). That proposal has made zero progress in the last two years and const eval is rich enough to support pretty much any user-defined `len` function as long as nightly features are allowed (`raw_ptr_deref`).

Currently, this raises a `const_err` lint when passed an `extern type`.

r? @oli-obk

cc @rust-lang/wg-const-eval
@CAD97
Copy link
Contributor

CAD97 commented Feb 21, 2022

Just recording here that these functions being const has an implication for user defined custom DSTs (specifically requiring size/align access to be const).

@ScSofts

This comment has been minimized.

@iddm

This comment was marked as off-topic.

@tgross35 tgross35 changed the title Make size_of_val a const fn (and stabilize it as such) Tracking issue for const_size_of_val and const_align_of_val Oct 14, 2024
@RalfJung
Copy link
Member

RalfJung commented Nov 1, 2024

@rust-lang/lang @rust-lang/wg-const-eval are there any concerns about making these functions const-stable? I think we should go ahead and make them const-stable. @CAD97 noted above that

these functions being const has an implication for user defined custom DSTs (specifically requiring size/align access to be const).

However, (a) that seems like a reasonable limitation, and (b) this might block custom DSTs on const trait support, but given that custom DSTs don't even have a consensus vision for what the design may look like, that's unlikely to become a problem.

@RalfJung RalfJung added I-lang-nominated Nominated for discussion during a lang team meeting. T-lang Relevant to the language team, which will review and decide on the PR/issue. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. labels Nov 1, 2024
@traviscross traviscross removed the T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. label Nov 6, 2024
@traviscross
Copy link
Contributor

traviscross commented Nov 6, 2024

@rfcbot fcp merge

The proposal is to make these const stable.

(Untagging libs-api on the advice of @Amanieu and @joshtriplett.)

@rfcbot
Copy link

rfcbot commented Nov 6, 2024

Team member @traviscross has proposed to merge this. The next step is review by the rest of the tagged team members:

No concerns currently listed.

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

cc @rust-lang/lang-advisors: FCP proposed for lang, please feel free to register concerns.
See this document for info about what commands tagged team members can give me.

@rfcbot rfcbot added proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. labels Nov 6, 2024
@scottmcm
Copy link
Member

scottmcm commented Nov 6, 2024

This sounds reasonable to me. It makes me wish that we had the Sized vs MetaSized vs Pointee distinction already (or whatever we'll eventually call it) but if we have to do a migration anyway for non-const, migrating it as stable in const probably doesn't make that any worse. And it'd clearly be useful and reasonable to have in const.

@rfcbot reviewed

@nikomatsakis
Copy link
Contributor

@rfcbot reviewed

@traviscross traviscross removed the I-lang-nominated Nominated for discussion during a lang team meeting. label Nov 6, 2024
@rfcbot rfcbot added the final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. label Nov 6, 2024
@rfcbot
Copy link

rfcbot commented Nov 6, 2024

🔔 This is now entering its final comment period, as per the review above. 🔔

@rfcbot rfcbot removed the proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. label Nov 6, 2024
@kennytm
Copy link
Member

kennytm commented Nov 7, 2024

i suppose a const size_of_val can still be compatible with #59905? that is,

#![feature(const_size_of_val)]

assert_eq!(6, (const { std::mem::size_of_val(c"hello") }));

@RalfJung
Copy link
Member

RalfJung commented Nov 7, 2024

https://doc.rust-lang.org/nightly/std/ffi/struct.CStr.html#method.from_bytes_until_nul and friends are const-callable, so yeah we can determine the size of a C string in const.

@Amanieu
Copy link
Member

Amanieu commented Nov 7, 2024

This could actually be a problem for scalable SIMD vectors where the size of std::arch::aarch64::svin64_t is only knowable at runtime by reading a register on the CPU that it is running on.

cc @davidtwco @JamieCunliffe

@RalfJung
Copy link
Member

RalfJung commented Nov 7, 2024

Hm, yeah. Though those are very non-standard types anyway and so far it is unclear how to make them work in Rust for various reasons.

We always have the option of aborting const-eval when it tries to determine the size of a type like that (similar to, e.g., ptr::is_null).

@scottmcm
Copy link
Member

scottmcm commented Nov 7, 2024

@Amanieu That's one of the things I was referring to with the trait split above. We already need to make changes -- for non-const too -- to support things like extern type and such, so I don't think this makes a huge difference for being able to do something in the future for such cases. For example, maybe we have a SizeAndAlignFromValue trait, and you need T: const SizeAndAlignFromValue to use that in const.

@kennytm
Copy link
Member

kennytm commented Nov 8, 2024

This could actually be a problem for scalable SIMD vectors where the size of std::arch::aarch64::svin64_t is only knowable at runtime by reading a register on the CPU that it is running on.

I'm not sure how scalable SIMD vector is problematic here. To use size_of_val()/align_of_val() in const, all input, well, have to be const as well:

// can't write this even if the alignment of `[u8]` is always 1.
fn x(a: &[u8]) -> usize {
    // error[E0435]: attempt to use a non-constant value in a constant
    const { std::mem::align_of_val(a) }
}

so the only way this matters in safe code is if you can construct an svfloat32_t at compile time, which I don't think should be possible.

static VALUE: svfloat32_t = ????;
const SIZE: usize = size_of_val(&VALUE);

you could bypass the constructibility question by using size_of_val_raw or other unsafe mechanisms, but during const-evaluation Miri should know that svfloat32_t can't be used here and abort.

const PTR: *const svfloat32_t = ptr::null();
// error[E0080]: evaluation of constant value failed
const SIZE: usize = unsafe { size_of_val_raw(PTR) };

conceptually the const size_of_val_raw of svfloat32_t can be defined as

const fn size_of_val_raw(_: *const svfloat32_t) -> usize {
    const fn size_of_svfloat32_t_ct() -> usize { panic!() }
    fn size_of_svfloat32_t_rt() -> usize { 4 * svcntw() }
    const_eval_select((), size_of_svfloat32_t_ct, size_of_svfloat32_t_rt)
} 

there is no need to introduce SizeAndAlignFromValue at this point.

@davidtwco
Copy link
Member

This could actually be a problem for scalable SIMD vectors where the size of std::arch::aarch64::svin64_t is only knowable at runtime by reading a register on the CPU that it is running on.

cc @davidtwco @JamieCunliffe

We're aware of the challenges related to constness and scalable vectors and the RFC we're working on addresses it. There's a similar issue with the already const-stable size_of where you might want to be able to call size_of::<svfloat32_t>(), but obviously not in a const context, and you can't work around that by relying on it being impossible to construct a svfloat32_t in a const context. It certainly reduces the options available to us in the RFC if size_of_val is const stable, but it certainly isn't worth blocking this on it as our proposals are still quite hypothetical, and like @kennytm notes, const_eval_select can help here too.

@davidtwco
Copy link
Member

This could actually be a problem for scalable SIMD vectors where the size of std::arch::aarch64::svin64_t is only knowable at runtime by reading a register on the CPU that it is running on.
cc @davidtwco @JamieCunliffe

We're aware of the challenges related to constness and scalable vectors and the RFC we're working on addresses it. There's a similar issue with the already const-stable size_of where you might want to be able to call size_of::<svfloat32_t>(), but obviously not in a const context, and you can't work around that by relying on it being impossible to construct a svfloat32_t in a const context. It certainly reduces the options available to us in the RFC if size_of_val is const stable, but it certainly isn't worth blocking this on it as our proposals are still quite hypothetical, and like @kennytm notes, const_eval_select can help here too.

Aforementioned RFC is rust-lang/rfcs#3729.

@rfcbot rfcbot added finished-final-comment-period The final comment period is finished for this PR / Issue. and removed final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. labels Nov 16, 2024
@rfcbot
Copy link

rfcbot commented Nov 16, 2024

The final comment period, with a disposition to merge, as per the review above, is now complete.

As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed.

This will be merged soon.

@rfcbot rfcbot added the to-announce Announce this issue on triage meeting label Nov 16, 2024
@apiraino apiraino removed the to-announce Announce this issue on triage meeting label Nov 21, 2024
@RalfJung
Copy link
Member

@davidtwco so just to be sure, you are fine with moving ahead here with const stabilization, and do not think this will adversely affect the future of your RFC or the entire unsized types / extern type / only-runtime-sized type situation?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-const-fn Area: const fn foo(..) {..}. Pure functions which can be applied at compile time. C-enhancement Category: An issue proposing an enhancement or a PR with one. disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. finished-final-comment-period The final comment period is finished for this PR / Issue. T-lang Relevant to the language team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests