Possible back-compat hazard from empty structs in std

[huonw]

$ 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

[alexcrichton]

Of those the only publicly exposed variants on the path to stabilization are:

src/libcore/fmt/mod.rs:55:pub struct Error;
src/libcore/marker.rs:222:pub struct NoCopy;
src/libcore/marker.rs:368:pub struct PhantomData<T:?Sized>;
src/libcore/ops.rs:970:pub struct RangeFull;
src/libstd/net/parser.rs:330:pub struct ParseError;
src/libstd/sync/mpsc/mod.rs:395:pub struct RecvError;

Of these, most are actually intended to be empty structs, and there are only two which I would consider for modification:

src/libcore/fmt/mod.rs:55:pub struct Error;
src/libstd/net/parser.rs:330:pub struct ParseError;

I personally think it's ok for fmt::Error to remain the same, but I do think that net::ParseError needs to be made opaque (it also should be reexported!)

[aturon]

Thanks @huonw!

• The marker and ops cases are intended
• The old_foo cases don't matter
• The rt cases don't matter
• The WhateverError cases do matter and should be fixed.

[aturon]

triage: P-backcompat-libs (1.0 beta)

[huonw]

Hm, if data-less Error structs matter, doesn't the same apply to the simple Error enums, since they can also be constructed externally? (std::sync::mpsc::TryRecvError is one such example, but maybe this specific example doesn't matter.)

[alexcrichton]

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 TryRecvError I 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]

@huonw

Hm, if data-less Error structs matter, doesn't the same apply to the simple Error enums, since they can also be constructed externally? (std::sync::mpsc::TryRecvError is one such example, but maybe this specific example doesn't matter.)

Hm, I just realized this was the case, from #22122. I wish this was less painful to fix :-/

[pnkfelix]

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]

@pnkfelix Yes, I agree. These definitely need to be addressed before beta.

[alexcrichton]

Ok, ParseError has been dealt with and I think it's fine to leave fmt::Error as-is, so I'm going to close this as "audit completed". Thanks again for opening the issue @huonw!