Add impl From<Ordering> for i32

[lopopolo]

This commit adds a first-class From conversion of Ordering into i32.

Ordering is defined to be compatible with the return types of the various ...cmp... routines in the C standard library, but the only way to convert an Ordering to an i32 is with a non-obvious as cast.

This commit adds a proper API for doing this and hides a "scary" as cast inside the core library.

[rust-highfive]

r? @joshtriplett

(rust-highfive has picked a reviewer for you, use r? to override)

[joshtriplett]

Tagging libs-api.

This would be insta-stable, so it needs team review.

@lopopolo Where is Ordering documented to be compatible with the return value of strcmp/memcmp/etc? Converting an Ordering to such a value doesn't seem like a particularly common use case.

[lopopolo]

Where is Ordering documented to be compatible with the return value of strcmp/memcmp/etc?

😅 I didn't say this was documented. I meant to say that Ordering is defined in the source to have the same semantics as the int return value/sign (-1/0/1) of these functions.

I'm implementing several "cmp" style routines as part of an oxidation project and being able to turn a nicely typed Ordering into the int expected at the FFI boundary would be something I'd like to have.

[lopopolo]

Converting an Ordering to such a value doesn't seem like a particularly common use case.

I was of the impression that such a conversion being common was not a requirement for adding From impls to core and std. I think I have a fairly clear use case and such a From impl would remove a little bit of friction here for little cost.

This behavior is already possible with an as cast and is already relied upon for the implementations of Ord and PartialEq for Ordering. This change just adds a discoverable API and makes this conversion less scary than an as cast.

[joshtriplett]

It's not that it needs to be common, it's that the usefulness needs to outweigh the potential for an error-prone conversion. .into() is non-specific, and can sometimes result in an unexpected conversion.

There's a proposal in #81642 to add derive(FromRepr) and automatically derive AsRepr, which would serve the same purpose, and I think that'd be more self-explanatory.

I would propose that we merge #81642 (in some form) and then Ordering will gain an as_repr() method for this.

[lopopolo]

There's a proposal in #81642 to add derive(FromRepr) and automatically derive AsRepr, which would serve the same purpose, and I think that'd be more self-explanatory.

ooooooh I didn't know about this proposal. That looks like it would do the trick. One sticking point is it doesn't look like Ordering has a #[repr(...)] attribute:

rust/library/core/src/cmp.rs

Lines 324 to 336 in e737694

	#[derive(Clone, Copy, PartialEq, Debug, Hash)]
	#[stable(feature = "rust1", since = "1.0.0")]
	pub enum Ordering {
	/// An ordering where a compared value is less than another.
	#[stable(feature = "rust1", since = "1.0.0")]
	Less = -1,
	/// An ordering where a compared value is equal to another.
	#[stable(feature = "rust1", since = "1.0.0")]
	Equal = 0,
	/// An ordering where a compared value is greater than another.
	#[stable(feature = "rust1", since = "1.0.0")]
	Greater = 1,
	}

I would propose that we merge #81642 (in some form) and then Ordering will gain an as_repr() method for this.

This sounds good to me. Do we need to cut another ticket to add a repr to ordering?

[joshtriplett]

@lopopolo Yes, please do submit a PR adding repr(i8) to Ordering, and I'd be happy to FCP it with libs.

Also, I submitted rust-lang/nomicon#315 to clarify that adding such a repr should not cause a problem with the actual size of the type.

[lopopolo]

thanks for the help @joshtriplett. The new PR is here:

• Add #[repr(i8)] to Ordering #89507