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

Rename LayoutErr to LayoutError #73

Open
exrook opened this issue Oct 1, 2020 · 2 comments
Open

Rename LayoutErr to LayoutError #73

exrook opened this issue Oct 1, 2020 · 2 comments

Comments

@exrook
Copy link

exrook commented Oct 1, 2020

We should rename LayoutErr to LayoutError in core::alloc. See prior discussion here: #57 (comment)

Since LayoutErr is stable, we can provide a type alias

pub type LayoutErr = LayoutError

I've implemented this change here, rust-lang/rust@master...exrook:rename-layouterr. Some things that are still unclear to me are:

  1. Should LayoutErr be deprecated
  2. Should LayoutError be behind a feature flag at first

If LayoutErr is deprecated or marked for deprecation in a later version, it can no longer be used in std because of #[deny(deprecated,deprecated_in_future)] being in effect. I don't think it makes sense to have LayoutError replace LayoutErr in std, but then be behind a feature flag, only usable on nightly. This is mainly a usability issue because crates can of course continue to use LayoutErr despite the stdlib rustdocs showing LayoutError.

Option 1

Is it possible to make LayoutError immediately stable, then deprecate LayoutErr in the same or a later release? The following would then be possible:

#[stable(feature = "alloc_layout", since = "1.28.0")]
#[rustc_deprecated(since = "1.48.0", reason = "use LayoutError instead", suggestion = "LayoutError")]
pub type LayoutErr = LayoutError;

#[stable(feature = "alloc_layout_error", since = "1.48.0")]
#[derive(Clone, PartialEq, Eq, Debug)]
pub struct LayoutError {
    private: (),
}

Option 2

I think this would make more sense than trying to do a staged approach of adding LayoutError as unstable, then marking it stable, then deprecating the LayoutErr type alias.

Stage 1 - release 1.N

#[stable(feature = "alloc_layout", since = "1.28.0")]
pub type LayoutErr = LayoutError;

#[unstable(feature = "alloc_layout_error", issue = "32838")]
#[derive(Clone, PartialEq, Eq, Debug)]
pub struct LayoutError {
    private: (),
}

Stage 2 - release 1.N+1

#[stable(feature = "alloc_layout", since = "1.28.0")]
pub type LayoutErr = LayoutError;

#[stable(feature = "alloc_layout_error", since = "1.49.0")]
#[derive(Clone, PartialEq, Eq, Debug)]
pub struct LayoutError {
    private: (),
}

Stage 3 - release 1.N+2

#[stable(feature = "alloc_layout", since = "1.28.0")]
#[rustc_deprecated(since = "1.51.0", reason = "use LayoutError instead", suggestion = "LayoutError")]
pub type LayoutErr = LayoutError;

#[stable(feature = "alloc_layout_error", since = "1.49.0")]
#[derive(Clone, PartialEq, Eq, Debug)]
pub struct LayoutError {
    private: (),
}
@SimonSapin
Copy link
Contributor

SimonSapin commented Oct 1, 2020

If and when this WG and the Libs team agree on a new name, I think there is no need to mess with #[unstable] items and the new name can be stable as soon as it’s added. It’s not like there are more design API details that are worth reserving the right to change later, as LayoutErr is already stable.

As to deprecation warnings, if we want them emitted we should use the since attribute to make it start two release cycles or more after the release that adds the new name (the current version of Nightly at the time of landing). That way the new name has reached the Stable channel when the Nightly channel starts warning. There is no need to wait for that cycle to add the attribute. That’s what since in rustc_deprecated is for, we can set it to start in the future.

However another option is to not emit any deprecation warning, and only soft-deprecate the old name in docs. What do we really gain by nudging existing code bases to actively migrate to the new name?

Additionally I personally don’t feel strongly that the naming consistency is worth bothering in the first place. Not renaming at all is also an option.

@exrook
Copy link
Author

exrook commented Oct 1, 2020

Thanks for the feedback Simon. It's good to hear that there is no need to use #[unstable] for a change like this, I would definitely prefer that route. I've removed the #[unstable] attributes and updated the deprecation attributes to be since = "1.50.0" in my branch.

As for deprecating the old name, given that the deprecated lint is warn by default, If we do decide to change the name I think it makes sense to let users know about the change through this mechanism.

As for whether this change is worth it, since we have also renamed AllocErr to AllocError, LayoutErr is the only error type in stdlib that does not follow the Error pattern. If we do make this change, the stdlib will be completely standardized on all error types ending with Error.

Additionally, doing some searches on github, there are approximately 370 uses of LayoutErr, of which 260 uses are of LayoutErr with AllocErr which will have to change anyways since AllocErr has now been renamed.

I think it makes sense to make this change for the sake of polish and consistency, this feature is not yet highly utilized, and the impact to existing users should be very low (they can continue to use LayoutErr if they wish, or even use LayoutError as LayoutErr),

m-ou-se added a commit to m-ou-se/rust that referenced this issue Nov 16, 2020
Rename/Deprecate LayoutErr in favor of LayoutError

Implements rust-lang/wg-allocators#73.

This patch renames LayoutErr to LayoutError, and uses a type alias to support users using the old name.

The new name will be instantly stable in release 1.49 (current nightly), the type alias will become deprecated in release 1.51 (so that when the current nightly is 1.51, 1.49 will be stable).

This is the only error type in `std` that ends in `Err` rather than `Error`, if this PR lands all stdlib error types will end in `Error` 🥰
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants