bsd: Annotate ELAST constants as unstable

[dybucc]

Description

This set of constants had already been discussed to cause some issues in #3131. This patch deprecates such bug-prone and latest-error symbols such that the interface to the libc crate is not prone to SemVer-breakage as often as the values change upstream.

Note the NetBSD constant was left unmodified as it already had been annotated with a deprecation attribute.

If you yourself want to help out in the deprecation effort for this type of constants across the libc crate, maybe try out the libc-constant-deprecator crate. It's been the tool used to perform the changes that have been submitted in this PR, and it very much welcomes bug reports.

Sources

• Upstream source code for the FreeBSD ELAST macro declaration, mentioning its use as a "limit" value for other errno values.

  https://github.com/freebsd/freebsd-src/blob/bd15d6ef126ee4c0eac931117f6bbbf6f9a3fc72/sys/sys/errno.h#L184

• Upstream source code for the OpenBSD equivalent macro declaration.

  https://github.com/openbsd/src/blob/58ff4898d99200dea776fa757004330318255d45/sys/sys/errno.h#L175

• Upstream source code for the XNU kernel equivalent macro declaration.

  https://github.com/apple-oss-distributions/xnu/blob/f6217f891ac0bb64f3d375211650a4c1ff8ca1ea/bsd/sys/errno.h#L270

• Upstream source code for the DragonFlyBSD equivalent macro declaration.

  https://github.com/DragonFlyBSD/DragonFlyBSD/blob/7b7fb99bddde8ded0bee8f43df4026ca02be0938/sys/sys/errno.h#L206

Checklist

• Relevant tests in libc-test/semver have been updated
• No placeholder or unstable values like *LAST or *MAX are included (see #3131)
• Tested locally (cd libc-test && cargo test --target mytarget); especially relevant for platforms that may not be checked in CI

[rustbot]

Some changes occurred in a NetBSD-like module

cc @semarie

Some changes occurred in an OpenBSD module

cc @semarie

[JohnTitor]

I think you should deprecate them for 0.2 and remove them for 1.0. Same for similar your PRs.

View changes since this review

[rustbot]

Reminder, once the PR becomes ready for a review, use @rustbot ready.

[dybucc]

I think you should deprecate them for 0.2 and remove them for 1.0. Same for similar your PRs.

View changes since this review

I don't quite understand what you mean. You mention deprecating for the 0.2 release, and removal for
the 1.0 release, but literally doing that means opening up separate PRs where I target these same
changes for the libc-0.2 branch, and removal (alongside modification of the corresponding *.txt
SemVer files) for the main branch. Just so I don't get things wrong, is that what you're implying?

Also, I left some comments, especially in #5122, that I would like to get external feedback on, if
you don't mind.

@rustbot ready

[JohnTitor]

Adding a deprecation attr for 1.0 doesn't make sense at all. You need to deprecate them for 0.2, see #4343 as an example.

@rustbot author

[dybucc]

Adding a deprecation attr for 1.0 doesn't make sense at all. You need to deprecate them for 0.2,
see #4343 as an example.

@rustbot author

Thanks. Done.

Should I also remove them from the SemVer plain text files?

@rustbot ready

[JohnTitor]

Should I also remove them from the SemVer plain text files?

No in this PR, let's do that in a PR actually removing them.

[JohnTitor]

LGTM, but this would make a lot of warnings in the world so cc @tgross35 to be seconded if we're ready to go ahead or not.

View changes since this review

[dybucc]

CI actually passes. There seems to be an issue with a glob import that is not used, but this has not
been changed in the patch (it's not even part of it, for that matter.) For some reason, rebasing
onto main with dependabot updates has ended up with a warning across all of my open PRs due to
that one (now apparently unused) import.

[tgross35]

I've been mulling this over a lot the past couple of weeks and have been tending to agree with Amanieu's comment in #3131 (comment). That is, I don't think deprecating to remove is the right path here: there are completely valid cases for these constants, e.g. allocating an array that can then be indexed by all possible values of an enum or looping through possible values of a flag. (I've actually needed both of these in the past couple of weeks.)

Would it be possible to find+replace the deprecation notices with a doc comment indicating that these constant values are subject to change? It can just be a terse comment linking to usage guidelines, which I'm adding in #5179 to help address this case.

@JohnTitor I'd like your thoughts here of course, given your concern in #3040 (comment). My thinking is that the failure modes here likely aren't any worse than any other constant like unsupported syscalls or flags - ELAST is just a pointer to some specific errno, you could use the errno regardless of whether or not ELAST tells you which one it is.

---

If you yourself want to help out in the deprecation effort for this type of constants across the libc crate, maybe try out the libc-constant-deprecator crate. It's been the tool used to perform the changes that have been submitted in this PR, and it very much welcomes bug reports.

That is quite a cool tool! You may want to rename/reframe it as something like "API reviewer" because I assume it is useful beyond libc and could pretty easily do more than deprecate. E.g. add doc comments :) or add #[non_exhaustive] to structs that are already non_exhaustive via a field (which we will eventually want once the rustc bug is fixed).

[dybucc]

Done. From reading what may become the new usage guidelines in #5179, I decided to instead add a new macro for declaring these types of constants that automatically adds the doc comment. I've opened another PR that solely defines the macro.

Should I close the analogous PRs that remove the symbols?

[tgross35]

If we had rust-lang/rust#143549 then that would be one thing, but I'd like to avoid more functionlike macros then necessary because they tend to make things pretty messy. I think the comment can be made to fit on 1-2 lines so it's easier to copy around to all relevant values, something like:

/// This is an end marker constant, its value is likely to change across releases. See
/// [Best Practices](crate::#best-practices) for details.

View changes since the review

[dybucc]

Done on this PR. Will be updating the rest soon.

[tgross35]

Should I close the analogous PRs that remove the symbols?

I think so. My concern is that removing them will break valid code where the only alternatives are going to be strictly worse.

[rustbot]

This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

[tgross35]

LGTM, thank you!

View changes since this review