/// Very useful trait ...
pub trait Foo {}
impl Foo for i32 {}
/// Return type of combine() -- combination of two Foos
// This is intended to implement Foo and be useless for anything else
pub struct Combined<A, B>(A, B); // where A: Foo, B: Foo // these are intended usage, but not necessary to define the struct
impl<A, B> Foo for Combined<A, B> where A: Foo, B: Foo {}
/// Combine two Foos into a big one
pub fn combine<A, B>(a: A, b: B) -> Combined<A, B>
// These are intended usage, but not necessary to define the function
// where A: Foo, B: Foo // Uncomment to improve error messages
{
Combined(a, b)
}
fn test<F: Foo>(_: F) {}
fn main() {
let x = combine(combine(1, 2), combine(3, 4.5)); // there is a bug on this line
// more code
test(x);
}
As you can see the compiler reports an error that:
points at the use of x, not at the construction of it (where the bug actually is)
mentions its type which can be very complex and ugly and does not help much to find the bug
However it you uncomment the where bounds on the combine function, the error message is much better:
it points directly to the place of the bug (where we try to combine incompatible components)
So my question is: Is it considered good practice to add such unnecessary bounds? (i.e. they are not needed to implement the function, but its return type/value is useless if they are not satisfied)
Or is there other way to improve error messages?
I have noticed that e.g. Iterator::map() does the same (with F: FnMut bound), so it is ok?
I'd make this dependent on whether Combine is a sensible type, even if it doesn't implement Foo. In the case of Iterator::map, any F that doesn't implement FnMut is not a sensible parameter, hence it requires the bound.
It is generally considered good practice to put bounds on impl blocks of a struct and not the struct itself.
Note that you can also use where Combined<A, B>: Foo as a bound, you aren't limited to applying it to each generic parameter. Whether or not that results in clearer errors also depends on context.
semantically, the bounds belong to both the struct and the constructor function, the type/value does not make much sense without them.
However the (current) implementation does not require them.
Now that I think of it, adding the bounds would allow me to (non-breaking) change the implementation to something that actually depends on them. That is unlikely, but still, it seems omitting bounds would expose implementation details.
Despite how I originally formulated my question I do not think it is a good approach to manipulate the code to adjust error messages, rather the code should express the intent and quality of error messages should follow from that. So another way to think about it is:
putting bounds on the struct (and everything else) would mean "The type is invalid unless the bounds are met".
putting them on constructor function would mean "The type is valid, but the value cannot be constructed this way unless the bounds are met".
putting them only on the impl block would mean "The type is valid and the value can be constructed this way, but it cannot be used as Foo unless bounds are met".
For me #1 seems most natural, but:
I wonder why that is so, since in my case #2 is weird and #3 is possible to implement, but it does not capture my intent and leads to bad error messages (and it feels duck-type-y to me).
So I guess, without more input I will go for #2 as a compromise, the structs are not that important anyway, as they exist mostly to be return types (I decided against impl Trait in return position).
The main reason is “it doesn’t help” — bounds on the struct don't imply bounds on the impl block. But that’s just saying they're mostly redundant.
The subtler reason to prefer it is that it lets your type’s users be more flexible in cases where your type’s functionality is not always required, but it has to be mentioned anyway. For example:
trait Foo {}
struct FooContainer<F> {
foos: Vec<F>,
}
impl<F: Foo> FooContainer<F> {
fn new() -> Self {
Self { foos: vec![] }
}
fn do_foo_thing(&self) { /* ... */ }
}
struct SometimesUsesFoo<T> {
fc: Option<FooContainer<T>>,
other_case: T,
}
fn main() {
// This is not an error even though i32 does not implement Foo
// and it is impossible to have fc: Some
let s: SometimesUsesFoo<i32> = SometimesUsesFoo {
fc: None,
other_case: 0,
};
}
If FooContainer had a F: Foo bound itself, then the user of SometimesUsesFoo would be forced to construct it only with types that implement Foo even if there is not actually any FooContainer in use at all.
This example is unfortunately vague and abstract, because this is the sort of thing that doesn’t come up until you have very complex situations. But it sometimes matters a lot. It’s like the question “why is Vec::new() const when you can’t ever have a non-empty vector in const?”, but with a less obvious use for the empty case.
Ok, this makes sense. But it seems to me like a workaround. Better way might be to make the type always valid and uninhabited unless the bounds are satisfied (but I did not give it that much thought, there might be another problems). Anyway, we do not have that and in the world we have, valid but unconstructible looks like a close enough approximation.
I'm unclear what the difference is - uninhabited types are valid types but unconstructable? I guess the difference is the implicit structure constructor that you can only "remove" with module encapsulation and private members.