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 AllocErr to AllocError #57

Closed
SimonSapin opened this issue Apr 26, 2020 · 15 comments
Closed

Rename AllocErr to AllocError #57

SimonSapin opened this issue Apr 26, 2020 · 15 comments
Labels

Comments

@SimonSapin
Copy link
Contributor

Almost every error type in the standard library is named SomethingError, not SomethingErr. The one exception is LayoutErr, which is stable and so can’t easily be renamed. Although LayoutErr has more "proximity" to AllocErr (being in the same module) I think we should leave it as a lone inconsistency rather than make it a second naming pattern to follow.

@TimDiekmann
Copy link
Member

TimDiekmann commented Apr 26, 2020

I think we should leave it as a lone inconsistency rather than make it a second naming pattern to follow.

As we are in the module alloc we should then rename it to Error to follow the naming pattern in std (io::Error and fmt::Error, not IoError and FmtError). Search results for Error

@SimonSapin
Copy link
Contributor Author

Maybe, though those modules only have one error type while std::alloc also has LayoutErr. I could still live with this.

@SimonSapin
Copy link
Contributor Author

On a less serious note, we also have an alloc::alloc::alloc function ;)

@TimDiekmann
Copy link
Member

Maybe, though those modules only have one error type while std::alloc also has LayoutErr. I could still live with this.

AllocError is the main error type in alloc. Personally I think Layout(Err) should live in mem instead of alloc, right next to align_of and size_of.

@SimonSapin
Copy link
Contributor Author

That… hadn’t occurred to me but it makes sense. It may even be worth making the move and having re-exports for compat.

@TimDiekmann
Copy link
Member

Actually I was just about posting a proposal... #59

@TimDiekmann
Copy link
Member

TimDiekmann commented Sep 24, 2020

We should decide, if we want AllocErr to be renamed to Error or AllocError. Other modules like core::fmt also have the Error type and it's known pattern to rust users, that different Errors occurs in core/std. Anyway, we also have core::alloc::LayoutErr (stable) and this will not be moved until we use Layout outside of std::alloc (see #59 and rust-lang/rust#71856). However, AllocErr is the "main-error" type in core::alloc, which is an argument for renaming it to Error.

One other thing to consider is, that AllocError may occure alongside other errors more often as soon as a user decides to use a fallible allocation API.

We basically have to decide, if we want to go the old std-way or the explicit way here.

@Amanieu
Copy link
Member

Amanieu commented Sep 24, 2020

I don't mind the way it is right now, especially considering LayoutErr and AllocErr are likely to be used together a lot.

@exrook
Copy link

exrook commented Sep 24, 2020

Doing a quick survey of all the error types in std, we have:

Error

std::io::Error
std::fmt::Error

_ Error

std::array::TryFromSliceError
std::cell::BorrowError
std::cell::BorrowMutError
std::char::CharTryFromError
std::char::DecodeUtf16Error
std::char::ParseCharError
std::collections::TryReserveError [unstable]
std::env::JoinPathsError
std::env::VarError
std::ffi::FromBytesWithNulError
std::ffi::FromVecWithNulError [unstable]
std::ffi::IntoStringError
std::ffi::NulError
std::io::IntoInnerError
std::net::AddrParseError
std::num::ParseFloatError
std::num::ParseIntError
std::num::TryFromIntError
std::option::NoneError
std::path::StripPrefixError
std::str::Utf8Error
std::string::FromUtf16Error
std::string::FromUtf8Error
std::string::ParseError
std::sync::PoisonError
std::sync::TryLockError 
std::sync::mpsc::RecvError
std::sync::mpsc::RecvTimeoutError 
std::sync::mpsc::SendError
std::sync::mpsc::TryRecvError
std::sync::mpsc::TrySendError
std::thread::AccessError
std::time::SystemTimeError

_ Err

std::alloc::LayoutErr

I think it makes the most sense to go with AllocError to be consistent with the majority of the other error types. Also, as Tim mentioned, AllocError will likely be used in code with other application specific error types so I don't think it makes sense to go down the std::alloc::Error path.

Also I think it would be nice if we could deprecate LayoutErr with LayoutError just for consistency.

@Lokathor
Copy link

pub type LayoutError = LayoutErr; would solve a lot

@TimDiekmann
Copy link
Member

Is it possible to write

#[stable]
pub type LayoutError = LayoutErr;

and deprecate LayoutErr without getting a warning when using LayoutError?

Is a type alias 100% backwards compatible when changing it to the actual struct? Or vice versa?

@TimDiekmann
Copy link
Member

Iff we change LayoutErr to LayoutError, do we still want it in core::alloc or should we move it to core::mem? Related: #59 and rust-lang/rust#71856

@JelteF
Copy link

JelteF commented Sep 24, 2020

and deprecate LayoutErr without getting a warning when using LayoutError?

You could also "soft deprecate" it, by simply writing a note in the docs about it.

Is a type alias 100% backwards compatible when changing it to the actual struct? Or vice versa?

I believe so

@TimDiekmann
Copy link
Member

I believe so

Assuming this works, we should rename LayoutErr to LayoutError, add pub type LayoutErr = LayoutError, and do a crater-run just to be sure.

However, if LayoutErr will be renamed should be discussed in a separate issue, as this is way more complicated than renaming AllocErr (as LayoutErr is stable).


I'd go with @exrook suggestion and just rename AllocErr to AllocError

to be consistent with the majority of the other error types.

AllocError is by far not common enough to name it alloc::Error like fmt::Error or io::Error.

@Lokathor
Copy link

io::Error and fmt::Error are actually the ones that don't fit the style of the rest of the standard library. (In a world where we could rename everything, they'd probably get a rename too.)

Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this issue Sep 30, 2020
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this issue Oct 1, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

6 participants