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 future-incompatibility lint const_evaluatable_unchecked #76200

Open
lcnr opened this issue Sep 1, 2020 · 14 comments
Open
Labels
A-const-eval Area: Constant evaluation, covers all const contexts (static, const fn, ...) A-const-generics Area: const generics (parameters and arguments) A-lint Area: Lints (warnings about flaws in source code) such as unused_mut. C-future-incompatibility Category: Future-incompatibility lints C-tracking-issue Category: An issue tracking the progress of sth. like the implementation of an RFC T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-lang Relevant to the language team, which will review and decide on the PR/issue.

Comments

@lcnr
Copy link
Contributor

lcnr commented Sep 1, 2020

This is the summary issue for the CONST_EVALUATABLE_UNCHECKED future-compatibility warning. The goal of this page is to describe why this change was made and how you can fix code that is affected by it. It also provides a place to ask questions
or register a complaint if you feel the change should not be made. For more information on the policy around future-compatibility warnings, see our breaking change policy guidelines.

What is the warning for?

In version 1.43 we accidentally allowed some uses of generic parameters in repeat expressions. This has a few problems which are outlined in the following document for now.

When will this warning become a hard error?

Most probably after const well-formedness bounds get stabilized, which is currently not yet implemented. There might be a subset of cases here which we can allow without needing any changes, but these require further careful consideration.

How to fix this?

As we only allowed generic constants where the actual type did not matter, it should always be possible to replace the generic parameters with dummy types. For example [0; std::mem::size_of::<*mut T>()] can be changed to [0; std::mem::size_of::<*mut ()>()].

Please report your case here so we may take it into consideration when deciding on how to proceed.

@lcnr lcnr added C-tracking-issue Category: An issue tracking the progress of sth. like the implementation of an RFC A-const-generics Area: const generics (parameters and arguments) labels Sep 1, 2020
@jonas-schievink jonas-schievink added A-const-eval Area: Constant evaluation, covers all const contexts (static, const fn, ...) C-future-incompatibility Category: Future-incompatibility lints T-lang Relevant to the language team, which will review and decide on the PR/issue. and removed C-tracking-issue Category: An issue tracking the progress of sth. like the implementation of an RFC labels Sep 1, 2020
@m-ou-se
Copy link
Member

m-ou-se commented Sep 12, 2020

Please report your case here

Merging #74532 just now ran into this problem. I worked around it by using () as dummy type:

-        let [] = [(); align_of::<Self>() - align_of::<*mut T>()];
+        let [] = [(); align_of::<AtomicPtr<()>>() - align_of::<*mut ()>()];

Here, not only *mut T was a problem, but Self (which is AtomicPtr<T>) was a problem as well.

m-ou-se added a commit to fusion-engineering-forks/rust that referenced this issue Sep 13, 2020
@danielhenrymantilla
Copy link
Contributor

Following from https://users.rust-lang.org/t/cant-understand-warning-when-using-const-evaluatable-checked/55630/6?u=yandros, the following snippet causes three warnings associated with the lint of this tracking issue, and a hard error:

const _10: usize = 10;

fn write_struct<T> ()
where
    can_be_evaluated!(_10 - 0):,
{
    foo::<T, 10>(); // Warning
    foo::<T, { _10 }>(); // Error (same for `{ 10 }`)
    WithConst::<10>::foo::<T>(); // Warning
    WithConst::<{ _10 }>::foo::<T>(); // Warning
    
    WithConst::<10>::write_struct::<T>(); // OK
    WithConst::<{ _10 }>::write_struct::<T>(); // OK
}

where foo in both cases features, for some generic constant N:

  • some non-generic type with an evaluatable constraint / that depends on a(n assumed) fallible computation of N (N - 0 in the examples; note that I am not talking about N - 0 being infallible, but rather, of a computation that does not fail for a concrete value of N (10 in the example));

  • an unrelated (to the above type) type parameter T.

In the case of WithConst, I am using a helper type to introduce N (and the evaluatable bound!) clearly before the generic type T is ever introduced, to ensure the evaluatable bound cannot possibly depend on T.

As you can see, a caller that:

  • forwards the generic type parameter,

  • but using a concrete constant parameter (one that does verify the evaluatable bound),

now triggers either this warning, or some related error when the constant is braced (which is sadly required when using a non-literal constant; even when the function features an evaluatable bound on said non-literal constant).

Finally, the last two calls in this example are interesting: by moving the logic of the function within the generic-and-evaluatable-bounded impl block for WithConst, we can later introduce that <T> type parameter without issues, and we can then call that helper function from outside with a fixed constant and it doesn't trigger a warning whatsoever.


This leads to two questions:

  • is this warning behavior really intended to affect these things?

  • And what about the workaround: is that intended too?

@lcnr
Copy link
Contributor Author

lcnr commented Feb 26, 2021

From what I can tell this is not expected, is_const_evaluatable seems to be incorrect here if feature(const_evaluatable_checked) is active.

Afaict all of the examples you've given should work without warnings or errors.

@is8ac
Copy link

is8ac commented Sep 17, 2021

Please report your case here

#![feature(generic_const_exprs)]

fn foo<T: Default + Copy, const L: usize>(_: [T; L]) -> [T; L + 1] {
    [T::default(); L + 1]
}

fn bar(x: [u8; 2]) -> [u8; 3] {
    foo::<u8, 2>(x)
}

fn baz<T: Default + Copy>(x: [T; 2]) -> [T; 3] {
    foo::<T, 2>(x)
}

fn main() {}

In 1.57.0-nightly (e4828d5b7 2021-09-16) baz produces warning: cannot use constants which depend on generic parameters in types. bar does not.

@tspiteri
Copy link
Contributor

While checking whether the fixed crate can be ported from typenum to nightly with generic_const_exprs, I hit this warning. I am not sure whether it's a false positive or whether the behavior I want is meant to be disallowed, but I think it should be allowed when enabling generic_const_exprs.

I've reduced the code though I left some stuff to show why it would be useful.

#![feature(generic_const_exprs)]

pub struct If<const CONDITION: bool>;
pub trait True {}
impl True for If<true> {}

pub struct FixedI8<const FRAC: u32> {
    pub bits: i8,
}

impl<const FRAC_LHS: u32, const FRAC_RHS: u32> PartialEq<FixedI8<FRAC_RHS>> for FixedI8<FRAC_LHS>
where
    If<{ FRAC_RHS <= 8 }>: True,
{
    fn eq(&self, _rhs: &FixedI8<FRAC_RHS>) -> bool {
        unimplemented!()
    }
}

impl<const FRAC: u32> PartialEq<i8> for FixedI8<FRAC> {
    fn eq(&self, rhs: &i8) -> bool {
        let rhs_as_fixed = FixedI8::<0> { bits: *rhs };
        PartialEq::eq(self, &rhs_as_fixed)
    }
}

@lcnr
Copy link
Contributor Author

lcnr commented Feb 23, 2022

@tspiteri this is a bug which only happens with feature(generic_const_exprs), opened #94293 to track that

@peter-lyons-kehl
Copy link
Contributor

peter-lyons-kehl commented May 15, 2022

I admit not understanding this (and reasoning behind it) fully. But I have use case for an associative type with a bound that depends on its owner (trait)'s const generic parameter. Any alternatives/workarounds, please?

Short example failing no Nightly (thanks to ilyvion#9946 for narrowing it down): https://play.rust-lang.org/?version=nightly&mode=debug&edition=2021&gist=c133c09b2fa155bb3f4b9ee45e6c011e.

My use case: https://github.com/ranging-rs/slicing-rs/blob/main/src/slices/mod.rs#L30=. I know it's a semantic/voluntary-like constraint only, but I believe it deserves its place. (Feel free to follow up at ranging-rs/slicing-rs#1.)

@lcnr
Copy link
Contributor Author

lcnr commented May 16, 2022

@peter-kehl this is #94293 again, this lint is only relevant when not using feature(generic_const_exprs)

@Stargateur
Copy link
Contributor

Stargateur commented May 1, 2023

I don't really understand the rationality of this limitation, also, it's weird to need to read it on external link and not directly in github. I think this subject didn't get much attention.

Recent question where an user would want to use it How can I give a name to the constant size of an array which is a member of a struct?

@KizzyCode
Copy link

KizzyCode commented May 7, 2024

Disclaimer: I'm neither a compiler engineer, nor a typesystem-language-lawyer – I'm sure, from a technical point of view, the rationale behind this is probably sound.

However, if I look at this from the developer-UX-perspective, I'm not happy with this warning...

Specific use-case

I have a struct (simplified):

/// Raw SPI command interface for RFM95
pub struct Rfm95<Peripheral, Tx, Rx, Sck, Select, Reset>
where
    Peripheral: SpiDevice,
    Tx: PinId + ValidPinIdTx<Peripheral>,
    Rx: PinId + ValidPinIdRx<Peripheral>,
    Sck: PinId + ValidPinIdSck<Peripheral>,
    Select: PinId,
    Reset: PinId,
{
    /// The SPI bus
    bus: SpiBus<Peripheral, Tx, Rx, Sck>,
    /// The chip select line
    select: PinChipSelect<Select>,
    /// The chip reset line
    reset: PinReset<Reset>,
}
impl<Peripheral, Tx, Rx, Sck, Select, Reset> Rfm95<Peripheral, Tx, Rx, Sck, Select, Reset>
where
    Peripheral: SpiDevice,
    Tx: PinId + ValidPinIdTx<Peripheral>,
    Rx: PinId + ValidPinIdRx<Peripheral>,
    Sck: PinId + ValidPinIdSck<Peripheral>,
    Select: PinId,
    Reset: PinId,
{
    /// The total FIFO size
    const FIFO_SIZE: usize = 0xFF;
    
    /// The register address to access the FIFO address pointer
    const REGISTER_FIFO_ADDRESS_POINTER: u8 = 0x0D;

    // ... implementation and member functions ...
}

Now, as naive developer, I tried to create an array in a member function

// Copy data into mutable buffer
let mut data_mut = [0; Self::FIFO_SIZE];

which raises this warning. I really don't understand why this restriction exists and this warning is emitted... the constant is fully constant; it's not dependent on any of the generic types, has no dynamic parts whatsoever, it's always 0xFF – it's so 100% boring, it could even be filled in using C-macro-style via search-replace.

Coming from a naive developer-UX-perspective, it's really not understandable why that's a problem at all. Now I'm sitting here without any reasonable workaround* (except free-floating constants or magic values), and that was so unsatisfying, I wrote this comment here 😅
IMO, this warning should either be dropped until there's a reasonable workaround; or the lint should link to/propose a reasonable workaround.

*If I missed something, please tell me and I'll happily heat my own words 😅

@RalfJung
Copy link
Member

RalfJung commented May 7, 2024

I think the intended work-around is to move the const FIFO_SIZE out of the impl block. By declaring it inside the impl block, your const implicitly becomes dependent on all generic parameters of the type, and that's what triggers the warning.

@KizzyCode
Copy link

KizzyCode commented May 11, 2024

By declaring it inside the impl block, your const implicitly becomes dependent on all generic parameters of the type, and that's what triggers the warning.

I don't really understand this part. As far as I see it, in most cases I can declare and use constants within generic structs pretty fine. I don't get a warning if I use FIFO_SIZE in conditional checks or REGISTER_FIFO_ADDRESS_POINTER; the only problem where this doesn't work is for array sizes and stuff.

And that feels inconsistent.

I think the intended work-around is to move the const FIFO_SIZE out of the impl block

Yup, it's the obvious fix, but that also feels more like a hack. Now I have to move this single const out of its associated context, while all other consts can stay in there and be used for constant operations (even compile-time assertions). At that point, I might as well hardcode the constant 😅

EDIT:

It appears even more inconsistent once you realize that you can easily do stuff like

impl<Peripheral, Tx, Rx, Sck, Select, Reset> Rfm95<Peripheral, Tx, Rx, Sck, Select, Reset>
where
    Peripheral: SpiDevice,
    Tx: PinId + ValidPinIdTx<Peripheral>,
    Rx: PinId + ValidPinIdRx<Peripheral>,
    Sck: PinId + ValidPinIdSck<Peripheral>,
    Select: PinId,
    Reset: PinId,
{
    /// A default FIFO buffer "seed"
    const FIFO_BUF_DEFAULT: [u8; 0xFF] = [0x00; 0xFF];
}

and use that instead, which works again.


Please note that I don't want to flame here; it's just that Rust makes great effort to be intuitive and developer-friendly, from error messages to 0-compromise correct APIs (e.g. Path). This one however feels out of place ("unrusty"), and reminds me more of C++-template magic and unexpected errors. So I thought this might be worth giving some feedback.

@RalfJung
Copy link
Member

RalfJung commented May 12, 2024

I don't really understand this part. As far as I see it, in most cases I can declare and use constants within generic structs pretty fine. I don't get a warning if I use FIFO_SIZE in conditional checks or REGISTER_FIFO_ADDRESS_POINTER; the only problem where this doesn't work is for array sizes and stuff.

Correct; using them as values is fine, but using them in types (like in an array size) is problematic. There's a big difference between these two ways of using something. That should hopefully be less surprising if you consider that runtime values are entirely forbidden as array sizes! Just because consts are allowed in both places, doesn't mean that suddenly these fundamental differences disappear. Consts are allowed in both places but they behave quite differently.

Now, this is all preliminary, and I don't know the long-term plans for generic consts. But there is an underlying consistency here. The confusing stems from conflating runtime expressions and compile-time expressions even though those are completely different beasts.

@fmease fmease changed the title Tracking Issue for forbidding unchecked generic expressions in constants Tracking issue for future-incompatibility lint const_evaluatable_unchecked Sep 14, 2024
@fmease fmease added T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. C-tracking-issue Category: An issue tracking the progress of sth. like the implementation of an RFC A-lint Area: Lints (warnings about flaws in source code) such as unused_mut. labels Sep 14, 2024
@Manishearth
Copy link
Member

ICU4X hits this here:

https://github.com/unicode-org/icu4x/blob/fcef14a31dbe5e2d0ee5f2a489872df373c2d0de/utils/zerotrie/src/options.rs#L135-L138

We use Self::FLAGS in a bunch of match statements, so it must be a constant.

This const is not dependent on the generic parameter.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-const-eval Area: Constant evaluation, covers all const contexts (static, const fn, ...) A-const-generics Area: const generics (parameters and arguments) A-lint Area: Lints (warnings about flaws in source code) such as unused_mut. C-future-incompatibility Category: Future-incompatibility lints C-tracking-issue Category: An issue tracking the progress of sth. like the implementation of an RFC T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-lang Relevant to the language team, which will review and decide on the PR/issue.
Projects
Archived in project
Development

No branches or pull requests