Remove overriding impls of deprecated `Error::description`.

[hkBst]

I propose to remove overriding impls of deprecated Error::description.

I naively proposed removing one such here as I was under the mistaken impression that it would delegate to Display::fmt, but it actually defaults to "description() is deprecated; use Display".

Deprecation happened 46 stable versions ago (in 1.42), or almost 6 years ago.

git grep 'fn description' | wc -l currently gives 72 results, which would need to be changed.

[cuviper]

Nominating for libs-api in particular, since this message is user-facing (if anyone is still using the deprecated method).

[programmerjake]

i didn't see any that looked like they were being used, but there is quite a lot of rust code that calls a description() method on github the first 2 pages have several uses of Error::description() afaict. at least two of them are published on crates.io.

that makes me think rust shouldn't make this change because it almost certainly breaks a lot of old code which we promised we'd keep working.

[hkBst]

First of all, the proposed change is (technically?) not breaking; Error::description just becomes a static deprecation message. Second, I examined the first 3 hits and the first and third are unrelated uses of description, while the second is already broken for an unrelated reason.

The first hit Dyon calls an unrelated description:

    /// Returns description of the type.
    pub fn description(&self) -> String {
        use Type::*;

        match *self {
            Unreachable => "unreachable".into(),
            Void => "void".into(),
            Any => "any".into(),
            Bool => "bool".into(),
            F64 => "f64".into(),
            Vec4 => "vec4".into(),
            Mat4 => "mat4".into(),
            Str => "str".into(),
            Link => "link".into(),
            Array(ref ty) => {
                if let Any = **ty {
                    "[]".into()
                } else {
                    let mut res = String::from("[");
                    res.push_str(&ty.description());
                    res.push(']');
                    res
                }
            }

Second hit, rust-throw, is indeed still using description, has not seen any updates in 7 years, and is already broken because of:

error: missing documentation for a macro
   --> src/lib.rs:606:1
    |
606 | macro_rules! up {
    | ^^^^^^^^^^^^^^^

+2 more (tested on 1.88) because it sets: src/lib.rs:#![deny(missing_docs)]

Third hit, rust-keyutils, uses again seem unrelated:

    pub fn search_for_keyring<'a, D, DK>(&self, description: D, destination: DK) -> Result<Self>
    where
        D: Borrow<<keytypes::Keyring as KeyType>::Description>,
        DK: Into<Option<&'a mut Keyring>>,
    {
        self.search_impl::<keytypes::Keyring>(
            &description.borrow().description(),
            destination.into(),
        )
        .map(Self::new_impl)
    }

(https://github.com/mathstuf/rust-keyutils/blob/5338c420eed17c2bd9d365cc20a240559b1a5573/src/api.rs#L461)

    pub fn description(&self) -> Result<Description> {
        self.description_raw()
            .and_then(|desc| Description::parse(&desc).ok_or(errno::Errno(libc::EINVAL)))
    }

(https://github.com/mathstuf/rust-keyutils/blob/5338c420eed17c2bd9d365cc20a240559b1a5573/src/api.rs#L461)

[hanna-kruppe]

Second hit, rust-throw, is indeed still using description, has not seen any updates in 7 years, and is already broken because of [deny(missing_docs)]

That doesn't mean it's broken in the sense that it can be automatically disregarded. Anything using on this crate continues to work because lints are capped in dependencies. (In this specific case there don't seem to be any public dependents and very few recent downloads, though.)

---

This ACP is missing a motivation. Why do you want to remove those methods? Are they causing any problems or burden?

[hkBst]

Ah, I didn't know about lints being capped in deps.

I left the motivation for this ACP implicit, but it comes down to the cost of continuing to carry these implementations versus the benefit of having them. Both are small, but presumably at some point after deprecation the former will be bigger than the latter. This ACP is essentially asking whether that time is now.

[Amanieu]

We discussed this in the @rust-lang/libs-api meeting today. Considering that description has been deprecated for over 5 years, it's reasonable to assume it is no longer used much and it's therefore acceptable to return a lower-quality error message. If anyone actually encounters this in practice then it would be additional motivation to update their outdated dependencies which still use description.

[shepmaster]

it's therefore acceptable to return a lower-quality error message

Does the same logic extend to Error::cause?

[hkBst]

@shepmaster I'm not sure how you want to extend the logic, since there are some differences:

• Error::cause has a default impl that does already call its replacement Error::source:

  fn cause(&self) -> Option<&dyn Error> {
      self.source()
  }

• the only difference with source is that source returns Option<&(dyn Error + 'static)>.

So it seems like you don't even lose anything by using the default impl, unless there are cases that have an impl that cannot fit the more restrictive return type of source. Do such impl's exist in the compiler, or did I completely miss your point?

[cuviper]

So it seems like you don't even lose anything by using the default impl, unless there are cases that have an impl that cannot fit the more restrictive return type of source. Do such impl's exist in the compiler, or did I completely miss your point?

I found one: TryLockError<T>::cause may return &PoisonError<T> where that T is not known to be 'static -- and it's generally not 'static because it's a lock guard of some kind. So this couldn't be returned by source.

https://github.com/rust-lang/rust/blob/5795086bdfe7ed988aa53a110bd0692c33d8755b/library/std/src/sync/poison.rs#L392

Every other fn cause that I see is forwarding to some inner Error::cause, like Box<E>, io::Error's Custom, etc.