Skip to content
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

Deprecate Error::description for real #66919

Open
wants to merge 1 commit into
base: master
from

Conversation

@dtolnay
Copy link
Member

dtolnay commented Dec 1, 2019

description has been documented as soft-deprecated since 1.27.0 (17 months ago). There is no longer any reason to call it or implement it.

This PR:

  • adds #[rustc_deprecated(since = "1.41.0")] to Error::description;
  • moves description (and cause, which is also deprecated) below the source and backtrace methods in the Error trait;
  • reduces documentation of description and cause to take up much less vertical real estate in rustdocs, while preserving the example that shows how to render errors without needing to call description;
  • removes the description function of all currently unstable Error impls in the standard library;
  • marks #[allow(deprecated)] the description function of all stable Error impls in the standard library;
  • replaces miscellaneous uses of description in example code and the compiler.

description

`description` has been documented as soft-deprecated since 1.27.0 (17
months ago). There is no longer any reason to call it or implement it.

This commit:

- adds #[rustc_deprecated(since = "1.41.0")] to Error::description;

- moves description (and cause, which is also deprecated) below the
  source and backtrace methods in the Error trait;

- reduces documentation of description and cause to take up much less
  vertical real estate in rustdocs, while preserving the example that
  shows how to render errors without needing to call description;

- removes the description function of all *currently unstable* Error
  impls in the standard library;

- marks #[allow(deprecated)] the description function of all *stable*
  Error impls in the standard library;

- replaces miscellaneous uses of description in example code and the
  compiler.
@dtolnay

This comment has been minimized.

Copy link
Member Author

dtolnay commented Dec 1, 2019

@rfcbot fcp merge

@rfcbot

This comment has been minimized.

Copy link

rfcbot commented Dec 1, 2019

Team member @dtolnay has proposed to merge this. The next step is review by the rest of the tagged team members:

No concerns currently listed.

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

@SimonSapin

This comment has been minimized.

Copy link
Contributor

SimonSapin commented Dec 1, 2019

@nox Any thoughts on this? If I remember correctly you recently argued for defining description in a new impl Error for Foo in a library, despite soft-deprecation.

@nox

This comment has been minimized.

Copy link
Contributor

nox commented Dec 1, 2019

I don't care enough to argue about that stuff, but I certainly hope this won't land without a crater run, and without an adequate amount of certainty that this won't trigger an important amount of deprecation warnings.

All things said, I don't think this hard-deprecation is useful.

@Mark-Simulacrum

This comment has been minimized.

Copy link
Member

Mark-Simulacrum commented Dec 1, 2019

@bors try for a crater run

@bors

This comment has been minimized.

Copy link
Contributor

bors commented Dec 1, 2019

⌛️ Trying commit b6194ab with merge bcff76b...

bors added a commit that referenced this pull request Dec 1, 2019
Deprecate Error::description for real

`description` has been documented as soft-deprecated since 1.27.0 (17 months ago). There is no longer any reason to call it or implement it.

This PR:

- adds `#[rustc_deprecated(since = "1.41.0")]` to `Error::description`;
- moves `description` (and `cause`, which is also deprecated) below the `source` and `backtrace` methods in the Error trait;
- reduces documentation of `description` and `cause` to take up much less vertical real estate in rustdocs, while preserving the example that shows how to render errors without needing to call `description`;
- removes the description function of all *currently unstable* Error impls in the standard library;
- marks `#[allow(deprecated)]` the description function of all *stable* Error impls in the standard library;
- replaces miscellaneous uses of `description` in example code and the compiler.

---

![description](https://user-images.githubusercontent.com/1940490/69910369-3bbaca80-13bf-11ea-94f7-2fe27a7ea333.png)
@bors

This comment has been minimized.

Copy link
Contributor

bors commented Dec 1, 2019

☀️ Try build successful - checks-azure
Build commit: bcff76b (bcff76bad586cfb6874f179a4b66448e6b8a832c)

@Mark-Simulacrum

This comment has been minimized.

Copy link
Member

Mark-Simulacrum commented Dec 1, 2019

@craterbot check

@craterbot

This comment has been minimized.

Copy link
Collaborator

craterbot commented Dec 1, 2019

👌 Experiment pr-66919 created and queued.
🤖 Automatically detected try build bcff76b
🔍 You can check out the queue and this experiment's details.

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

@craterbot

This comment has been minimized.

Copy link
Collaborator

craterbot commented Dec 1, 2019

🚧 Experiment pr-66919 is now running

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

@@ -12,21 +12,23 @@ on it.
```rust,ignore (pseudo-Rust)
use std::error::Error as StdError;

This comment has been minimized.

Copy link
@goodmanjonathan

goodmanjonathan Dec 1, 2019

Contributor
Suggested change
use std::error::Error as StdError;
@dtolnay

This comment has been minimized.

Copy link
Member Author

dtolnay commented Dec 1, 2019

I don't care enough to argue about that stuff, but I certainly hope this won't land without a crater run, and without an adequate amount of certainty that this won't trigger an important amount of deprecation warnings.

All things said, I don't think this hard-deprecation is useful.

I am still seeing description functions pop up in my work codebase. New developers see it in the trait, and obviously errors need a description so they write one. The error idioms in Rust are confusing enough and we don't need to keep around this part of the confusion. As such, I'm set on either real-deprecating it or hiding it from rustdoc (#66859).

@craterbot

This comment has been minimized.

Copy link
Collaborator

craterbot commented Dec 4, 2019

🎉 Experiment pr-66919 is completed!
📊 50 regressed and 0 fixed (81904 total)
📰 Open the full report.

⚠️ If you notice any spurious failure please add them to the blacklist!
ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.