-
Notifications
You must be signed in to change notification settings - Fork 13.1k
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
Foo<T>: Trait
bounds (T
is a type param) are allowed in const fn
s
#83452
Comments
It's not clear to me why this would be expected not to work. The bounds are upheld correctly by the compiler in precisely the same way as they are for non- Same thing if you call |
@slightlyoutofphase That's an argument for allowing all bounds in If you look at what's proposed in https://github.com/oli-obk/rfcs/blob/const_generic_const_fn_bounds/text/0000-const-generic-const-fn-bounds.md#guide-level-explanation . The |
I guess my overall point is that I'm not personally aware of anywhere it's been specifically stated that this shouldn't work, and why it shouldn't work.
Your |
It doesn't matter, you're allowed to add defaulted methods and that shouldn't break downstream users, which is why This is not the issue to figure out how to solve problems with const trait impls and bounds though, it's about making sure that there's no forward compatibility bugs. This isn't allowed now: const fn foo<T: Const>() -> u8 {
T::C
} and I see no reason why it should be any different with |
It doesn't need to be "required". The only limitation currently is that a user cannot directly write:
meaning
Yeah it is, if you use the |
The problem is that the code posted above is accepted on stable, but the intention of the |
This makes much more sense if they're specifically talking about stable, where literally no feature flags can be used (meaning Edit: Actually, I'm not sure the "blanket requirement" interpretation of what is meant in the RFC is entirely correct. The way it works now, if actually using #![allow(incomplete_features, dead_code)]
#![feature(const_fn, const_trait_impl)]
pub trait Const {
const ONE: usize = 1usize;
fn one() -> Self;
}
impl const Const for u8 {
fn one() -> u8 {
1u8
}
}
impl Const for usize {
fn one() -> usize {
1usize
}
}
const fn foo1<T: Const>() -> usize {
T::ONE
}
const fn foo2<T: Const>() -> T {
T::one()
}
fn main() {
// `foo1` explicitly returns `1usize` no matter what, so the `constness` of T's
// `Const` trait impl is completely irrelevant.
const X: usize = foo1::<u8>();
const Y: usize = foo1::<usize>();
// `u8` did `impl const Const`, so the next line works with `foo2`.
const Z: u8 = foo2::<u8>();
// `usize` just did `impl Const`, so the next line does not work with `foo2`.
const W: usize = foo2::<usize>();
} Which is to say, in the context of a The only requirements for associated anything the RFC mentions is the requirement for associated types to also have const impls while actually implementing a given trait as const. |
@slightlyoutofphase The code isn't using any features at all(so it shouldn't matter that my initial comment used nightly), and yes this works on stable in every version I tried back to Rust 1.31.0 https://rust.godbolt.org/z/7TnxfPP3W The only reason I mentioned that I'm using nightly is to make it obvious that this wasn't fixed by a recent PR. Please don't assume that using nightly means that I must want to use any or all nightly features. |
I still don't understand how there's any kind of "forward compatibility" concern. You cannot write Your original example could only be broken if the maintainer of the |
SImple, the code compiles now, and it'll break whenever const impls and bounds are fully implemented. It''ll be required to be a I repeat, no code at all in #83452 (comment) needs to change for that to break. Also, there is no code around that, it's the only code you need in a |
Again, my point is that it's not clear to me that this is actually the case for something that strictly involves nameable integral constants with known primitive types. It doesn't make any sense, and has none of the same actual stability concerns that calling const trait methods (which you cannot do currently on stable) does. Consider something like the following, also, that does work on stable today as well: trait Trait {
const F: usize = 99;
}
struct X;
impl Trait for X {}
const fn foo() -> usize {
X::F
} What it actually does is ultimately the same thing as your example. It's just accessing the constant indirectly. Which is fine, because if someone changes Look at the playground link I posted earlier, also. The way it's been made to work so far is logical and provides really good error reporting. Intentionally "dumbing it down" in the particular way you're suggesting would be very strange IMO. |
No, it'd make the most sense, because you don't have to look into the function or the trait definitions to figure out whether a const impl is requried. What you're asking is that const bounds are inferred from the function definition. |
Who doesn't have to "look into" what, exactly, in what specific context? I don't understand what you mean. |
Imagine that function definitions are opaque(like they are WRT trait bounds, you can't add trait bounds inside the function body), what is the constness of the bounds of the function? |
Can you give an example of what you're referring to? |
This gives you an error, it doesn't implicitly add a Clone bound to the function fn clone_something<T>(x: &T) -> T {
<T as std::clone::Clone>::clone(x)
} what you're proposing is analogous to inferring the fn clone_something<T>(x: &T) -> T {
<T as std::clone::Clone>::clone(x)
}
{
struct Unclonable;
clone_something(&Unclonable); // Erroring at this line. If it was removed, there'd be no error
} |
I'm not "proposing" anything at all. I've continuously just tried to make the point that the only possible way to use traits in Also, I don't think your example is all that similar to something to something like: #![allow(incomplete_features, unused_variables, dead_code)]
#![feature(const_fn, const_trait_impl)]
use std::ops::Add;
#[derive(Copy, Clone)]
struct NotConstAdd {}
impl Add for NotConstAdd {
type Output = Self;
fn add(self, _other: Self) -> Self::Output {
self
}
}
const fn do_add<T: Copy + Add<Output = T>>(a: T) -> T {
// Don't use Add
a
}
const fn really_do_add<T: Copy + Add<Output = T>>(a: T) -> T {
// Do use Add
a + a
}
// Works.
const A: NotConstAdd = do_add(NotConstAdd {});
// Does not work.
const B: NotConstAdd = really_do_add(NotConstAdd {});
fn main() {
// Both of these work, because they're not being called in a const context.
do_add(NotConstAdd {});
really_do_add(NotConstAdd {});
} Again, the only thing that's missing in the above is the ability to "opt out" / "opt in" to the constness of the impl, which just isn't implemented yet. The error given for Edit: You also have to consider that #![allow(incomplete_features)]
#![feature(const_fn, const_trait_impl)]
use std::ops::Deref;
struct Strange {}
impl const Deref for Strange {
type Target = str;
fn deref(&self) -> &Self::Target {
"What a weird way to use deref!"
}
}
const STR: &str = &*Strange {};
fn main() {
println!("{}", STR);
} |
@slightlyoutofphase
I agree with that, so long as we're talking about cases that don't require trait bounds being added to the function. The example in my first comment is wrong because of the trait bound. Here is a case that isn't problematic: trait Foo {
const X: u8;
}
struct Generic<T>(T);
impl<T> Foo for Generic<T> {
const X: u8 = 10;
}
const fn hello<T>() -> u8 {
Generic::<T>::X
} The reason this is fine is because no bound needs to be added to |
I understand that perfectly, just as I understand these are experimental features and everything else. I don't agree it's an accurate description of what we're talking about here though, particularly in the presence of stuff like the language-level syntactic use cases for Your |
@slightlyoutofphase Your const Deref example is not incompatible with anything I said. Again, let's not use examples of language limitations (eg: not being able to prove that the code that initializes associated constants terminates), as a reason to make bounds implicitly const depending on code inside the function. |
I meant it as an example of something that technically presents the same scenario you're describing as being the overall issue here, where any struct at all may or may not be validly dereferenceable with
That certainly wasn't my point. And I still disagree that any bounds are being "changed" overall even currently. Everything we're talking about here is a matter of "is or is not an explicit compile-time error". |
What I'm talking about is exclusively about whether a I'm saying that this is how it should work in generic contexts: use std::ops::Add;
const fn foo<T: Add>() {
// The function definition doesn't matter WRT its bounds, so I'm not writing it.
}
const fn bar<T: ?const Add> (l: T, r: T) {
foo::<T>()
} which should eventually error at the
That code above is analogous to this: fn foo<T: std::ops::Add>() {
// The function definition doesn't matter
}
fn bar<T> (l: T, r: T) {
foo::<T>()
} which errors at the
|
Yeah, that makes sense as far as how the opt-out should interact with everything else. I wasn't arguing that. |
How, this clearly looks like you're arguing that the required constness of bounds should be essentially compile-time duck typing, where the contents of the function determines what constness is required of the caller: const fn do_add<T: Copy + Add<Output = T>>(a: T) -> T {
// Don't use Add
a
}
const fn really_do_add<T: Copy + Add<Output = T>>(a: T) -> T {
// Do use Add
a + a
}
// Works.
const A: NotConstAdd = do_add(NotConstAdd {});
// Does not work.
const B: NotConstAdd = really_do_add(NotConstAdd {}); My argument is that neither should work, because it makes is a breaking change to use If you think that dynamically unreached code inside a function shouldn't trigger this error, it's even worse of an idea, and that's how it works now(because it's not enforced at the type level yet): const fn do_add<T: Copy + Add<Output = T>>(a: T) -> T {
// Don't use Add
a
}
const fn other_fn() -> bool {
false
}
const fn really_do_add<T: Copy + Add<Output = T>>(a: T) -> T {
let cond = other_fn();
// Do use Add, just in non-obviously dead code.
if cond { a + a; }
a
}
// Works.
const A: NotConstAdd = do_add(NotConstAdd {});
// Also works.
const B: NotConstAdd = really_do_add(NotConstAdd {}); |
What was your point then, the only thing that could break in
You're not getting that this is about what you can determine about a function without even running it or peeking inside, there would be no absolute requirement for trait bounds if we only just cared that things eventually error at compile-time, they'd just be a way to document what the function does. Identical to that, I just don't get why you think that expressions inside of functions should determine what impls are required of callers, that's not how it works today (without using incomplete experimental features), and it's not how it should work in the future. |
@jonas-schievink: thanks for the link, that's helpful info to have. I guess I'm still a bit unclear about how something that is not itself a trait impl, but rather a free function (or non-trait inherent method) is ultimately intended to behave, though. E.G: // Say this is just a regular inherent struct method
pub const fn get_usize(&self) -> usize {
NonSelfConcreteImplementerOfSomeTrait::USIZE
} or for a more complex example: trait Trait {
const VALUE: usize = 999;
}
struct Struct {}
impl Trait for Struct {}
impl Struct {
const fn get_usize(&self) -> usize {
1
}
}
// Does not refer to `Trait` externally at all.
const fn get_usize(s: &Struct) -> usize {
// Technically, either side of the addition operation below could become
// invalid code if the person who maintains `Struct` changes it, assuming
// that's not the same person who maintains this `get_usize` free function.
s.get_usize() + <Struct as Trait>::VALUE
}
const STRUCT: Struct = Struct {};
const U: usize = get_usize(&STRUCT);
fn main() {
println!("{}", U);
} @rodrimati1992: Why would I be talking about runtime errors as far as your |
That's missing my point entirely, it doesn't matter if it eventually errors, read my point about guarantees.
Also, bringing up examples of panicking const fns is completely irrelevant to my point about const bounds (and therefore impls required inside functions), because bounds are enforced at the call site whenever the call site is defined, not when the call site is eventually called itself, even if the functions are all generic.
Obviously, that's already a breaking change for I don't get this oneWhy do you keep bringing this example up as though it proves a point about const bounds?. If it's not about const bounds stop bringing it up, we're talking code that I argue should error because of const bounds, not about panicking const code. // Say this is just a regular inherent struct method
pub const fn get_usize(self) -> usize {
NonSelfConcreteImplementerOfSomeTrait::USIZE
} This is the same as putting the associated constant in a local And if you're using associated constants from a type parameter, struct X<T>(T);
impl<T> X<T> {
const GET_X: usize = <T as Trait>::FOO;
}
pub const fn get_usize(self) -> usize {
X::<T>::GET_X
} which clearly isn't problematic, no non-const trait impl is required inside the function. |
If you keep saying that it (the enforcement of const impls) happens to work now, you are making the mistake of assuming that how it works now is likely what will go to stable, so obviously it's not a bug that the code in my original comment is accepted(it is a bug). |
You might think you're making a point by showing const fns that panic, you're not, all you're doing is showing limitations in Rust that should be avoided (in the design of the language) whenever there's a good alternative. |
This is intentionally allowed now, so no point in leaving this issue open. |
I tried this code:
I expected to see this code not compile, because it bounds types by non-auto traits in
const fn
s.Instead, this happened: The code compiles without errors, and prints
3
This does error with
T: Const
bounds as expected.Meta
rustc --version --verbose
:The text was updated successfully, but these errors were encountered: