rust-lang / wg-allocators

Home of the Allocators working group: Paving a path for a standard set of allocator traits to be used in collections!
http://bit.ly/hello-wg-allocators
209 stars 9 forks source link

Rename AllocErr to AllocError #57

Closed SimonSapin closed 4 years ago

SimonSapin commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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

SimonSapin commented 4 years ago

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

TimDiekmann commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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

TimDiekmann commented 4 years ago

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 https://github.com/rust-lang/wg-allocators/issues/59 and https://github.com/rust-lang/rust/pull/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 commented 4 years ago

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

exrook commented 4 years ago

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 commented 4 years ago

pub type LayoutError = LayoutErr; would solve a lot

TimDiekmann commented 4 years ago

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 commented 4 years ago

Iff we change LayoutErr to LayoutError, do we still want it in core::alloc or should we move it to core::mem? Related: https://github.com/rust-lang/wg-allocators/issues/59 and https://github.com/rust-lang/rust/pull/71856

JelteF commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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.)