Repository navigation
Possible back-compat hazard from empty structs in std #22949
Description
$ git grep 'pub struct [^(]*;' -- src/libcore src/libcollections/ src/libstd/ src/libunicode/
src/libcore/fmt/mod.rs:55:pub struct Error;
src/libcore/fmt/num.rs:87:pub struct UpperHex;
src/libcore/marker.rs:222:pub struct NoCopy;
src/libcore/marker.rs:230:pub struct Managed;
src/libcore/marker.rs:368:pub struct PhantomData<T:?Sized>;
src/libcore/marker.rs:383:pub struct ContravariantLifetime<'a>;
src/libcore/marker.rs:389:pub struct CovariantLifetime<'a>;
src/libcore/marker.rs:395:pub struct InvariantLifetime<'a>;
src/libcore/marker.rs:401:pub struct ContravariantType<T>;
src/libcore/marker.rs:407:pub struct CovariantType<T>;
src/libcore/marker.rs:413:pub struct InvariantType<T>;
src/libcore/ops.rs:970:pub struct RangeFull;
src/libstd/net/parser.rs:330:pub struct ParseError;
src/libstd/old_io/buffered.rs:437: pub struct NullStream;
src/libstd/old_io/net/ip.rs:373:pub struct ParseError;
src/libstd/old_io/util.rs:82:pub struct NullWriter;
src/libstd/old_io/util.rs:91:pub struct ZeroReader;
src/libstd/old_io/util.rs:112:pub struct NullReader;
src/libstd/old_path/posix.rs:101:pub struct ParsePathError;
src/libstd/old_path/windows.rs:128:pub struct ParsePathError;
src/libstd/rt/unwind.rs:394: pub struct EXCEPTION_RECORD;
src/libstd/rt/unwind.rs:396: pub struct CONTEXT;
src/libstd/rt/unwind.rs:398: pub struct DISPATCHER_CONTEXT;
src/libstd/sync/mpsc/mod.rs:395:pub struct RecvError;
src/libstd/sys/unix/os.rs:156:pub struct JoinPathsError;
src/libstd/sys/windows/os.rs:197:pub struct JoinPathsError;
These are the pub structs that have no data fields at all. Once stabilised these cannot ever have anything added to them, since they can currently be publicly constructed by just writing their name. We should make sure that this is the intended behaviour for each. (I'm sure many of those are not intending to be #[stable] for 1.0, so this is probably a fairly quick audit.)
cc @aturon
Metadata
Metadata
Assignees
Labels
No labels
Activity
alexcrichton commentedon Mar 2, 2015
Of those the only publicly exposed variants on the path to stabilization are:
Of these, most are actually intended to be empty structs, and there are only two which I would consider for modification:
I personally think it's ok for
fmt::Errorto remain the same, but I do think thatnet::ParseErrorneeds to be made opaque (it also should be reexported!)aturon commentedon Mar 2, 2015
Thanks @huonw!
markerandopscases are intendedold_foocases don't matterrtcases don't matterWhateverErrorcases do matter and should be fixed.aturon commentedon Mar 2, 2015
triage: P-backcompat-libs (1.0 beta)
huonw commentedon Mar 2, 2015
Hm, if data-less
Errorstructs matter, doesn't the same apply to the simpleErrorenums, since they can also be constructed externally? (std::sync::mpsc::TryRecvErroris one such example, but maybe this specific example doesn't matter.)alexcrichton commentedon Mar 2, 2015
I've personally tried to ensure that all errors which may contain contextual information to always have a private field to prevent construction, but in the case of errors like
TryRecvErrorI don't believe that we'll ever have any error variants other than those listed, so I opted to make them public (to allow for easy comparison). It is something to worry about, however.aturon commentedon Mar 3, 2015
@huonw
Hm, I just realized this was the case, from #22122. I wish this was less painful to fix :-/
pnkfelix commentedon Mar 3, 2015
I suppose both this and #22045 are both particular instances of a more general principle that structs with all pub fields (and all enums that contain solely such structs) are backward compatibility hazards, no?
aturon commentedon Mar 5, 2015
@pnkfelix Yes, I agree. These definitely need to be addressed before beta.
Auto merge of #23504 - alexcrichton:parse-error-not-unit, r=aturon
Rollup merge of rust-lang#23504 - alexcrichton:parse-error-not-unit, …
alexcrichton commentedon Mar 24, 2015
Ok,
ParseErrorhas been dealt with and I think it's fine to leavefmt::Erroras-is, so I'm going to close this as "audit completed". Thanks again for opening the issue @huonw!