multiple applicable items in scope on nightly but not stable

[TheBlueMatt]

The following seems to have regressed maybe yesterday or the day before on nightly, but builds on stable. It is real code, sorry its not 100% minimal :)

use core::fmt;
use core::fmt::Debug;

use std::io;

/// An error in decoding a message or struct.
#[derive(Clone, Debug, PartialEq)]
pub enum DecodeError {
	/// A version byte specified something we don't know how to handle.
	/// Includes unknown realm byte in an OnionHopData packet
	UnknownVersion,
	/// Unknown feature mandating we fail to parse message (eg TLV with an even, unknown type)
	UnknownRequiredFeature,
	/// Value was invalid, eg a byte which was supposed to be a bool was something other than a 0
	/// or 1, a public key/private key/signature was invalid, text wasn't UTF-8, TLV was
	/// syntactically incorrect, etc
	InvalidValue,
	/// Buffer too short
	ShortRead,
	/// A length descriptor in the packet didn't describe the later data correctly
	BadLengthDescriptor,
	/// Error from std::io
	Io(/// (C-not exported) as ErrorKind doesn't have a reasonable mapping
        io::ErrorKind),
	/// The message included zlib-compressed values, which we don't support.
	UnsupportedCompression,
}



impl fmt::Display for DecodeError {
	fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
		match *self {
			DecodeError::UnknownVersion => f.write_str("Unknown realm byte in Onion packet"),
			DecodeError::UnknownRequiredFeature => f.write_str("Unknown required feature preventing decode"),
			DecodeError::InvalidValue => f.write_str("Nonsense bytes didn't map to the type they were interpreted as"),
			DecodeError::ShortRead => f.write_str("Packet extended beyond the provided bytes"),
			DecodeError::BadLengthDescriptor => f.write_str("A length descriptor in the packet didn't describe the later data correctly"),
			DecodeError::Io(ref e) => e.fmt(f),
			DecodeError::UnsupportedCompression => f.write_str("We don't support receiving messages with zlib-compressed fields"),
		}
	}
}

Error running this on playground for feb 1's nightly:

 
⣿
Standard Error

   Compiling playground v0.0.1 (/playground)
error[E0034]: multiple applicable items in scope
  --> src/lib.rs:39:32
   |
39 |             DecodeError::Io(ref e) => e.fmt(f),
   |                                         ^^^ multiple `fmt` found
   |
   = note: candidate #1 is defined in an impl of the trait `std::fmt::Display` for the type `ErrorKind`
   = note: candidate #2 is defined in an impl of the trait `Debug` for the type `ErrorKind`
help: disambiguate the associated function for candidate #1
   |
39 |             DecodeError::Io(ref e) => std::fmt::Display::fmt(&e, f),
   |                                       ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
help: disambiguate the associated function for candidate #2
   |
39 |             DecodeError::Io(ref e) => Debug::fmt(&e, f),
   |                                       ~~~~~~~~~~~~~~~~~

For more information about this error, try `rustc --explain E0034`.
error: could not compile `playground` due to previous error

Warnings

No main function was detected, so your code was compiled
but not run. If you’d like to execute your code, please
add a main function.

[ehuss]

This changed as part of #93090 which added a Display impl. This kind of breakage is often considered OK (https://std-dev-guide.rust-lang.org/code-considerations/breaking-changes/new-trait-impls.html), though a crater run may change that decision if sufficiently large number of crates are affected. It doesn't look like one was done, though there will be one done in about 4 weeks to check for beta regressions.

cc @jyn514

[TheBlueMatt]

This kind of breakage is often considered OK

That's.....incredibly surprising? From what I can tell (a) there's no way for users to write code that will be robust to rustc changes, or at least not enforce it in an automated way from rustc and (b) there will be no warning for users until their code suddenly doesn't compile on the latest rustc. Doesn't this very strongly violate the stability guarantees of rustc?

Don't get me wrong, if there's a strong reason for breakage so be it, but adding a trivial trait impl that doesn't significantly improve ergonomics seems like a trivial win compared to breakage? Are there no other ways to go about this - adding the impl in a new edition, warning of future resolution failures for a few versions prior to stabilizing the impl, etc?

[the8472]

That's.....incredibly surprising? From what I can tell (a) there's no way for users to write code that will be robust to rustc changes

Adding trait impls is a minor change per RFC 1105.

Robust code can use partially or fully qualified function call syntax as the error message suggests, but yes, that can be unergonomic.

help: disambiguate the associated function for candidate #2
   |
39 |             DecodeError::Io(ref e) => Debug::fmt(&e, f),
   |                                       ~~~~~~~~~~~~~~~~~

Doesn't this very strongly violate the stability guarantees of rustc?

The API remains stable, it is shorthands that become ambiguous. This situation isn't optimal and there have been some discussions about how to improve it but for now, lacking other solutions, it must be allowed because otherwise new methods cannot be introduced because they could conflict with any random 3rd-party import.

As ehuss already said, the ecosystem-impact will be assessed during the next beta cycle.

(b) there will be no warning for users until their code suddenly doesn't compile on the latest rustc.

Some projects use allowed-fail CI against nightly or beta rustc to get earlier warnings.

[TheBlueMatt]

Adding trait impls is a minor change per RFC 1105. The API remains stable, it is shorthands that become ambiguous..

Yep, totally understand the policy and the changes here and the motivation for it. I guess my point is twofold here - (a) it seems the policy here is at least in some substantial way in major conflict with the publicly stated stability guarantees of rustc and (b) it seems the change to stdlib here is very minor and the inherent tension between code-compiling stability and the really minor win of the feature improvement calls into question whether its worth breaking compilation for downstream projects, given (a).

Some projects use allowed-fail CI against nightly or beta rustc to get earlier warnings.

Yep, that's what led to this issue. Well, and because we have CI fuzzing, and for that we have to use nightly for the LLVM instrumentation stuff, so our CI fuzzing pipeline is currently paused until we figure out if rustc is gonna move forward here or if we have to go change our code.

[ehuss]

Closing per the comment at #94507 (comment).

[TheBlueMatt]

Is there a tracking issue for reconsidering the policy in RFC 1105?