libs: clean up enums now that namespacing has arrived #19253

aturon · 2014-11-23T21:26:58Z

Now that enums are namespaced, there's a fair amount of cleanup work to do in libstd.

In particular:

During the rollout of the feature, variants were re-exported at the top level to minimize churn. But the new idiom is to keep these items namespaced under the enum. In most cases, these re-exports should be removed.
Some variants included prefixes or suffixes in their names as a way to work around the lack of namespacing. These should be renamed.

A lot of this work will happen naturally during API stabilization, but it's also something that can tackled in parallel to help speed things up.

The text was updated successfully, but these errors were encountered:

aturon · 2014-11-23T21:27:14Z

cc @sfackler @japaric

aturon · 2014-11-24T23:33:09Z

Does someone want to make a list of the various enums this should apply to, so we can triage?

fhartwig · 2014-11-25T20:55:53Z

Here's a list of enums whose variants are re-exported, by file:

src/libstd/collections/hash/map.rs: Entry
src/libstd/collections/hash/table.rs: BucketState
src/libstd/dynamic_lib.rs: Rtld
src/libstd/io/mod.rs: SeekStyle, FileMode, FileAccess, FileType, IoErrorKind
src/libstd/io/net/addrinfo.rs: SocketType, Flag, Protocol
src/libstd/io/net/ip.rs: IpAddr
src/libstd/io/process.rs: StdioContainer, ProcessExit
src/libstd/num/strconv.rs: ExponentFormat, SignificantDigits, SignFormat
src/libstd/os.rs: MemoryMapKind, MapOption, MapError
src/libstd/path/windows.rs: PathPrefix
src/libstd/sys/common/net.rs: SocketStatus, InAddr
src/libstd/sys/unix/timer.rs: Req
src/libstd/sys/windows/timer.rs: Req
src/libcore/cmp.rs Ordering
src/libcore/result.rs: Result
src/libcore/option.rs: Option
src/libcollections/str.rs: MaybeOwned

I hope I haven't missed any.
Apart from Result, Option and (maybe) Ordering, I don't think any of these need to have their variants re-exported.

sfackler · 2014-11-25T21:02:14Z

We don't really need to reexport Result's, Option's and Ordering's either since everyone will be pulling them from the prelude anyway.

The type aliases json::JsonString and json::JsonObject were originally prefixed with 'json' to prevent collisions with (at the time) the enums json::String and json::Object respectively. Now that enum namespacing has landed, this 'json' prefix is redundant and can be removed: json::JsonArray -> json::Array json::JsonObject -> json::Object In addition, this commit also unpublicizes all of the re-exports in this JSON module, as a part of rust-lang#19253 [breaking-change]

All of the enum components had a redundant 'Type' specifier: TypeSymlink, TypeDirectory, TypeFile. This change removes them, replacing them with a namespace: FileType::Symlink, FileType::Directory, and FileType::RegularFile. RegularFile is used instead of just File, as File by itself could be mistakenly thought of as referring to the struct. Part of rust-lang#19253.

* Remove public reexports, as a part of rust-lang#19253 * Rename getopts::Fail_ to getopts::Fail * Didn't see a reason for the suffixed '_' * Removed getopts::FailType * Looked like it was only beings used for tests; refactored the tests to stop requiring it * A few other non-breaking trivial refactoring changes [breaking-change]

frewsxcv · 2014-11-27T23:11:34Z

@sfackler I'll open a PR that gets rid of those reexports. If anyone has other suggestions for ones to get rid of, I can do them as well

In regards to: rust-lang#19253 (comment) This commit: * Changes the #deriving code so that it generates code that utilizes fewer reexports (in particur Option::* and Result::*), which is necessary to remove those reexports in the future * Changes other areas of the codebase so that fewer reexports are utilized

In regards to: #19253 (comment) This commit: * Changes the #deriving code so that it generates code that utilizes fewer reexports (in particur Option::\*, Result::\*, and Ordering::\*), which is necessary to remove those reexports in the future * Changes other areas of the codebase so that fewer reexports are utilized

* Remove public reexports, as a part of #19253 * Rename getopts::Fail_ to getopts::Fail * Didn't see a reason for the suffixed '_' * Removed getopts::FailType * Looked like it was only beings used for tests; refactored the tests to stop requiring it * A few other non-breaking trivial refactoring changes [breaking-change]

In regards to: rust-lang#19253 (comment) This commit: * Changes the #deriving code so that it generates code that utilizes fewer reexports (in particur Option::* and Result::*), which is necessary to remove those reexports in the future * Changes other areas of the codebase so that fewer reexports are utilized

frewsxcv · 2014-12-06T22:31:30Z

Might as well mention that rust-lang/rfcs#497 will remove Ordering from the prelude, so I'm not going to remove that reexport yet

Remove most of the public reexports mentioned in rust-lang#19253 These are all leftovers from the enum namespacing transition In particular: * src/libstd/num/strconv.rs * ExponentFormat * SignificantDigits * SignFormat * src/libstd/path/windows.rs * PathPrefix * src/libstd/sys/windows/timer.rs * Req * src/libcollections/str.rs * MaybeOwned * src/libstd/collections/hash/map.rs * Entry * src/libstd/collections/hash/table.rs * BucketState * src/libstd/dynamic_lib.rs * Rtld * src/libstd/io/net/ip.rs * IpAddr * src/libstd/os.rs * MemoryMapKind * MapOption * MapError * src/libstd/sys/common/net.rs * SocketStatus * InAddr * src/libstd/sys/unix/timer.rs * Req [breaking-change]

Remove most of the public reexports mentioned in rust-lang#19253 These are all leftovers from the enum namespacing transition In particular: * src/libstd/num/strconv.rs * ExponentFormat * SignificantDigits * SignFormat * src/libstd/path/windows.rs * PathPrefix * src/libstd/sys/windows/timer.rs * Req * src/libcollections/str.rs * MaybeOwned * src/libstd/collections/hash/map.rs * Entry * src/libstd/collections/hash/table.rs * BucketState * src/libstd/dynamic_lib.rs * Rtld * src/libstd/os.rs * MemoryMapKind * MapOption * MapError * src/libstd/sys/common/net.rs * SocketStatus * InAddr * src/libstd/sys/unix/timer.rs * Req [breaking-change]

Part of rust-lang#19253 I would have removed this public reexport in rust-lang#19842, but rust-lang#19812 hadn't merged (and snapshotted) at the time In rust-lang#19407, I changed the codebase to stop utilizing this reexport [breaking-change]

rust-lang/rust#19253 and rust-lang/rust@25f8051 have introduced changes to the namespacing within the std::collections::hash_map, breaking some of Cargo code which imported these. rust-lang/rust@cf350ea, implementing changes proposed by RFC rust-lang#344, have also broken some code which relies on hash_set::SetItems (now renamed to hash_set::Iter). This commit fixes the incompatibilities: imports of std::collections::hash_map::{Occupied, Vacant} have been replaced by imports of std::collections::hash_map::Entry::{Occupied, Vacant} and one instance where the SetItems has been used was replaced by the proper usage of Iter.

frewsxcv · 2014-12-23T07:40:45Z

Status update with this issue: All of the public reexports that I mentioned in my previous comment have been removed (with the exceptions of the io stuff which will get removed soon enough). I just did a quick grep and potentially found some more that need to be removed. If you see any from those results that I should remove, please let me know.

rust-lang/rust#19253 and rust-lang/rust@25f8051 have introduced changes to the namespacing within the std::collections::hash_map, breaking some of Cargo code which imported these. rust-lang/rust@cf350ea, implementing changes proposed by RFC #344, have also broken some code which relies on hash_set::SetItems (now renamed to hash_set::Iter). This PR fixes the incompatibilities: imports of std::collections::hash_map::{Occupied, Vacant} have been replaced by imports of std::collections::hash_map::Entry::{Occupied, Vacant} and one instance where the SetItems has been used was replaced by the proper usage of Iter.

Part of rust-lang#19253 [breaking-change]

steveklabnik · 2015-02-15T18:18:06Z

I think this is done now, please let me know if there's anything left oustanding.

In regards to: rust-lang/rust#19253 (comment) This commit: * Changes the #deriving code so that it generates code that utilizes fewer reexports (in particur Option::* and Result::*), which is necessary to remove those reexports in the future * Changes other areas of the codebase so that fewer reexports are utilized

sfackler added the A-libs label Nov 23, 2014

alexcrichton mentioned this issue Nov 24, 2014

Unpublicize JSON reexports, unprefix JSON type aliases #19224

Merged

ogham mentioned this issue Nov 24, 2014

Clean up FileType enum following enum namespacing #19273

Merged

frewsxcv mentioned this issue Nov 27, 2014

getopts: cleanup, renames, remove reexports #19365

Merged

frewsxcv mentioned this issue Nov 29, 2014

Utilize fewer reexports #19407

Merged

frewsxcv mentioned this issue Dec 22, 2014

Remove cmp::Ordering::* public reexport #20140

Merged

geomaster mentioned this issue Dec 23, 2014

Update to rust master rust-lang/cargo#1085

Merged

frewsxcv added a commit to frewsxcv/rust that referenced this issue Dec 30, 2014

Remove core::atomic::Ordering::* public reexport

1d26fb9

Part of rust-lang#19253 [breaking-change]

frewsxcv mentioned this issue Dec 30, 2014

Remove core::atomic::Ordering::* public reexport #20348

Merged

alexcrichton added a commit to alexcrichton/rust that referenced this issue Dec 31, 2014

rollup merge of rust-lang#20348: frewsxcv/rm-reexports

38f9805

Part of rust-lang#19253 [breaking-change]

frewsxcv mentioned this issue Jan 1, 2015

Remove core::iter::MinMaxResult::* public reexport #20386

Merged

frewsxcv added a commit to frewsxcv/rust that referenced this issue Jan 1, 2015

Remove core::iter::MinMaxResult::* public reexport

f9ce6f5

Part of rust-lang#19253 [breaking-change]

alexcrichton added a commit to alexcrichton/rust that referenced this issue Jan 2, 2015

rollup merge of rust-lang#20386: frewsxcv/rm-reexports

9ca8076

Part of rust-lang#19253 [breaking-change]

frewsxcv added a commit to frewsxcv/rust that referenced this issue Jan 3, 2015

Remove graphviz::LabelText::* public reexport

f031285

Part of rust-lang#19253 [breaking-change]

frewsxcv mentioned this issue Jan 3, 2015

Remove graphviz::LabelText::* public reexport #20457

Merged

nikomatsakis pushed a commit to nikomatsakis/rust that referenced this issue Jan 3, 2015

Remove graphviz::LabelText::* public reexport

02df7df

Part of rust-lang#19253 [breaking-change]

nikomatsakis pushed a commit to nikomatsakis/rust that referenced this issue Jan 3, 2015

Remove graphviz::LabelText::* public reexport

115aa3d

Part of rust-lang#19253 [breaking-change]

steveklabnik closed this as completed Feb 15, 2015

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

libs: clean up enums now that namespacing has arrived #19253

libs: clean up enums now that namespacing has arrived #19253

aturon commented Nov 23, 2014

aturon commented Nov 23, 2014

aturon commented Nov 24, 2014

fhartwig commented Nov 25, 2014

sfackler commented Nov 25, 2014

frewsxcv commented Nov 27, 2014

frewsxcv commented Dec 6, 2014

frewsxcv commented Dec 23, 2014

steveklabnik commented Feb 15, 2015

libs: clean up enums now that namespacing has arrived #19253

libs: clean up enums now that namespacing has arrived #19253

Comments

aturon commented Nov 23, 2014

aturon commented Nov 23, 2014

aturon commented Nov 24, 2014

fhartwig commented Nov 25, 2014

sfackler commented Nov 25, 2014

frewsxcv commented Nov 27, 2014

frewsxcv commented Dec 6, 2014

frewsxcv commented Dec 23, 2014

steveklabnik commented Feb 15, 2015