Description
Here are two similar pieces of code.
version 1 (playground):
#[derive(Debug, PartialEq, Eq, PartialOrd)]
struct Xy { x: i32, y: i32 }
#[allow(dead_code)]
impl Xy {
// imagine a slew of other methods here
fn max(&self, other: Xy) -> Xy {
use std::cmp::max;
Xy { x: max(self.x, other.x), y: max(self.y, other.y) }
}
// and imagine a slew of other methods here as well
// (thus the `allow(dead_code)` above).
}
fn main() {
let unit_x = Xy { x: 1, y: 0 };
let unit_y = Xy { x: 0, y: 1 };
let sum = unit_x.max(unit_y);
println!("{sum:?}");
}
version 2 (playground):
#[derive(Debug, PartialEq, Eq, PartialOrd, Ord)]
struct Xy { x: i32, y: i32 }
#[allow(dead_code)]
impl Xy {
// imagine a slew of other methods here
fn max(&self, other: Xy) -> Xy {
use std::cmp::max;
Xy { x: max(self.x, other.x), y: max(self.y, other.y) }
}
// and imagine a slew of other methods here as well
// (thus the `allow(dead_code)` above).
}
fn main() {
let unit_x = Xy { x: 1, y: 0 };
let unit_y = Xy { x: 0, y: 1 };
let sum = unit_x.max(unit_y);
println!("{sum:?}");
}
version 1, when run, prints this:
Xy { x: 1, y: 1 }
version 2, when run, prints this:
Xy { x: 1, y: 0 }
(The only difference between the two blocks of code above is that one included Ord
in the derive; the other did not.)
I expected to see this happen: The compiler should issue a warning at the point where fn max
is defined, saying that the Ord
trait, which is part of the prelude (and thus does not need to be explicitly imported at the call site to take precedence over fn max
), is going to end up being called on any use of this method that doesn't explcitly disambiguate. In other words, the compiler should proactively check for resolution ambiguities based on what prelude-provided traits are implemented and what methods those traits provide.
A lint that would look something like:
You have defined the inherent method `fn max(&self, other: Xy) -> Xy` on the `Xy` type
which also implements the `Ord` trait. Any call of the form `xy.max(other)` will not call
your inherent method. You may want to choose a different name for this method, or
prominently indicate in its documentation that one must use the unambiguous call
syntax `Xy::max(&xy, &other)` to invoke it.
Instead, this happened: The compiler only "warns" via the dead code lint.
I can understand an attitude that says "this is your own fault for ignoring the dead code warning", but I do not consider this to be quite the same as a dead code problem.
The ambiguity in method resolution here is dangerous, because its all too easy to overweight the immediately visible inherent definition of fn max(&self, ...)
, while overlooking the prelude's provision of Ord
with its own definition of fn max(&self, ...)
that will end up taking precedence here.
Basically, I am claiming that while the dead code lint can be correlated with more serious problems (and thus one can argue that users should be treating instances of the dead code lint as seriously as any other diagnostic issue), I think that in practice we can do a better job of identifying cases like this where the choice of method name is very likely to cause a resolution ambiguity for all reasonable uses of the method.
Meta
Rust versions I tested:
1.86.0; 1.88.0