Unused trait bounds

I searched for a bit but couldn't find anything except a small issue on clippy that got updated in 2016 the last time.

The compiler already tells us when a functions/field/structs/etc.. is never used. Expanding on this idea I think it would be valuable if the compiler told us if there are redundancies and or unused trait bounds in functions/structs/etc.. since over time while refactoring code often I end up with many trait bounds I don't actually need, but its not obvious from looking at the code (first you have to look at all function signatures the function calls, map all generic parameters to the concrete type, merge the trait bounds, and so on).

Is there any tool I missed that helps with this?
Or should I create an issue for rustc, or are there known reasons for this to not work/be implemented?

2 Likes

The compiler may not be able to determine if a trait bond is necessary. For example you may need a type to implement Eq (and not just PartialEq) for algorithmic reasons, without actually using the Eq trait directly anywhere. (it has no methods on it)

2 Likes

Ah I see. But I'm not willing to give up on this yet :slight_smile:

So you see a problem when considering marker traits? (Or more specifically traits that declare a contract beyond of functions it contains)

Similar to [must_use], maybe it would be possible to create a tag like [contract] (just an example) to tell the compiler to always assume a trait is required?

This would still help in a lot of situations, given my use of traits, which is mostly numeric traits like Add and alike that don't have a contract beyond providing an + operation.

Out of curiosity: where do you see something like that being used? I mean, even on a trait like Eq, it is not only undesirable to always require it, it is also infeasible for eg floating point types.

1 Like

I didn't mean that it should always be required for every type, but as a hint to the compiler that even if the trait is not actively used in a function it should not tell the user that it is not required, since (as Alice said) it represents a contract the compiler cannot reason about.

I believe such an attribute (macro) would essentially require the trait as an argument, which means that when it is specified, the trait is named doubly at definition site.

So I believe maybe I'm missing something. Would you please provide an example for us to work with?

Let's assume we have the following code (obviously very reduced):

trait Foo{
    fn foo();
}

fn bar<T>(t: T) where
    T: Foo,
{
    t.foo();
}

Now for some reason, the implementation for bar changes and does not require T to implement Foo:

trait Foo{
    fn foo();
}

//ideally: compiler tells us we don't require T: Too anymore
fn bar<T>(t: T) where 
    T: Too,
{
    //do something else with t, but don't call foo
}

But now, we still have the trait bound. And it would be nice if the compiler told us that we don't require that bound anymore.

Now for Alice's concern, I would suggest something like:

//Tell the compiler to not assume this trait is not needed, just because we don't use it.
[contract]
trait Baz{
}

//the compiler does not tell us that we don't require T: Baz, since we told the compiler Baz represents something it cannot reason about in these terms.
fn bar2<T>(t:T) where
    t: Baz,
{
    //do something
}

Written on my phone so sorry for any typos or autocorrect

3 Likes

That clarifies things for me, thank you for typing it up.

In a vacuum it looks like a great idea to me.
My concern is that if accepted, this proposal will trigger a tsunami of compiler warnings all across the Rust ecosystem because no existing traits (in stdlib and the broader ecosystem alike) have a #[contract] attribute at definition site.
Basically, to make this feasible it would require a lot of maintenance work from Rust trait authors everywhere.

3 Likes

Well, when implemented in the compiler, I guess the std would provide the #[contract] attributes for its own traits.
As for the tsunami, either those are mostly traits not required or marker traits of which as far as I know there aren't that many (at least i cant recall having seen any, except once) outside of the std (as far as I know, might be wrong)

1 Like

Since I have some free time due to current circumstances I would like to bump this thread once, especially what others think about this.

I still think this would be a valuable feature to have, even if it generates a lot of warnings for current crates (I don't think warnings are considered a problem for backwards comparability?).

I would be fine with this as a Clippy lint, perhaps even warn-by-default. I don't think it belongs in the compiler.

No, but it's still a problem, because unlike in other languages, in Rust, people care about warnings. It is customary and recommended to resolve warnings, because rustc is way smarter in its warnings than e.g. C compilers (and warnings are turned on by default). So generating a lot of warnings generally correlates pretty well with "you've got some low quality code right there". So, this proposal would essentially entail marking a lot of existing code as suspiciously low-quality, for no good reason.

This should at most be a Clippy lint.

Besides from the marker traits (or other traits that act as a sort of contract), I would indeed say, that these warnings indicate "low-quality" code (at least as much as "unused code" would indicate "low-quality" code).

The problem is that the compiler doesn't know which case it is, and once the programmer has determined the warning is wrong, there should be a straight-forward code change that hides the warning. If you don't provide an easy way to remove it once I've checked it's ok, you can bet I'm going to turn it off for the project. (this issue is why I don't use clippy)

Note that there's another case besides marker/contract traits: Often constructors don't require the trait themselves, but it provides better error messages if it still requires the trait: The user will get the error on the constructor call rather than when they try to use it.

2 Likes

This is the part that catches me up. Isn't that every trait? When would I not want to consider a trait a contract?

The Add trait imposes no additional constraints on the implementation beyond those enforced by the compiler. These additional constraints are what makes something a contract trait.

But Add itself is still a contract just as good as any other, right?

fn can_add<T: Add>(_x: T) {}

fn will_add<T: Add<Output=T>>(a: T, b: T) -> T {
    todo!()
}

I certainly don't want the compiler second-guessing the Add bounds in those functions.

2 Likes

The difference is, that for example when implementing the Add trait it would be fine to implement it as 1+1=5.
But the definition of the Eq trait says that any type implementing it must behave like a=b && b=c => b=c.
This last part is the contract the programmer has to take care of (as the compiler can't check it).

Would something like [allow(unused)] solve that problem for you?

Maybe it would be worth it starting with something simpler:
What if the compiler would only warn if a super and a subtrait is present in a trait bound? Not checking at all if they are needed but just checking if one of them is certainly not required (I can't think of a situation where a trait bound like Eq+PartialEq would be helpful).

Definitely not. Idiomatic code should not have to turn warnings off. It infuriates me every time I have to do this in JS to make my IDE happy.

Meh, I don't care either way for this warning. I don't think this really catches any of the cases that have been discussed in this thread, but I also would not get mad at it.

1 Like