Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Consider: requiring --cfg allow_crate_unstable #5

Open
CAD97 opened this issue Jun 30, 2022 · 2 comments
Open

Consider: requiring --cfg allow_crate_unstable #5

CAD97 opened this issue Jun 30, 2022 · 2 comments

Comments

@CAD97
Copy link

CAD97 commented Jun 30, 2022

I believe that by requiring a --cfg opt in (i.e. in RUSTFLAGS) it is possible to support "layered unstablility" while preventing "unstability hiding".

Specifically: if semver-exempt features are available under (just) a normal cargo feature flag, it is possible that you use some crate awsm-sauce under its default features and assume everything is stable, but in actuality awsm-sauce enables some unstable-risky-feature in the crate upstream, exposing you to semver breakage if upstream takes advantage of the fact the functionality is marked unstable to change it in an API breaking way. Even though your application did not acknowledge the use of unstable features at all.

This is the reason that proc-macro2 requires setting --cfg procmacro2_semver_exempt to get access to unstable features.

Note that this must not only be done for your crate, but for any crate that depends on your crate. This infectious nature is intentional, as it serves as a reminder that you are outside of the normal semver guarantees.

However, extending this pattern to more than one crate is problematic, because while the lack of being able to encapsulate this is desired, it makes writing libraries utilizing upstream unstable features impractical. Say I'm the author awsm-sauce: now upstream is requiring --cfg upstream_semver_exempt, and I've realized the error of my ways, so gate the use of unstable parts of upstream behind an unstable-* flag. But now, even though my consumers are reasonably hidden from my use of upstream, and they're explicitly opting into my unstable API surface, they have to also opt into upstream's unstable surface.

My proposed solution: form a consensus that every crate should use the same --cfg flag, e.g. allow_crate_unstable (that's a literal crate, not a placeholder for the crate name) or allow_3rd_party_unstable or crates_are_semver_exempt; something illustrative. The point is to have a singular opt-in for builds which unlocks the ability to enable any crate's unstable APIs, just like using the nightly Rust toolchain gives all crates being compiled the ability to enable #![feature]s.

Suggested implementation

Normally annotated functions:

/// This function does something really risky!
///
/// Don't use it yet!
#[stability::unstable(feature = "risky-function")]
pub fn risky_function() {
    impl_body!()
}

// becomes

/// This function does something really risky!
///
/// Don't use it yet!
///
/// # Availability
///
/// **This API is marked as unstable** and is only available when the
/// `unstable-risky-function` crate feature is enabled. This comes with no
/// stability guarantees, and could be changed or removed at any time.
#[cfg(all(allow_crate_unstable, feature = "unstable-risky-function"))]
pub fn risky_function() {
    impl_body!()
}

/// This function does something really risky!
///
/// Don't use it yet!
#[cfg(not(all(allow_crate_unstable, feature = "unstable-risky-function")))]
pub(crate) fn risky_function() {
    impl_body!()
}

However, for the usage of upstream unstable, we need additional annotation, because the function cannot even be compiled if the upstream does not expose the used unstable item(s).

/// This function does something really risky!
///
/// Don't use it yet!
#[stability::unstable(feature = "risky-function")]
#[stability::upstream(crate = "proc-macro2", feature = "proc_macro_diagnostic")]
pub fn risky_function() {
    impl_body!()
}

// becomes

/// This function does something really risky!
///
/// Don't use it yet!
///
/// # Availability
///
/// **This API is marked as unstable** and is only available when the
/// `unstable-risky-function` crate feature is enabled. This comes with no
/// stability guarantees, and could be changed or removed at any time.
///
/// **This API uses upstream unstable features** from the following crates:
///
/// - `proc-macro2`: `unstable-proc_macro_diagnostic`
///
/// Using this API exposes you to the unstability of these crates as well.
#[cfg(all(allow_crate_unstable, feature = "unstable-risky-function"))]
pub fn risky_function() {
    impl_body!()
}

#[stability::upstream] is a pseudo-attribute recognized, parsed, and stripped by #[stability::unstable].

If always requiring RUSTFLAGS=--cfg allow_crate_unstable is deemed excessive, we could allow the use of some e.g. this-software-is-provided-as-is-with-no-warranty-of-any-kind feature alternative to --cfg allow_crate_unstable, with an attached disclaimer that this disables the protection against hiding instability.

@sagebind
Copy link
Owner

sagebind commented Jul 8, 2022

This is an interesting proposal, thanks for writing it up. I'll have to think on this.

@CAD97
Copy link
Author

CAD97 commented Jul 8, 2022

A small revision to the suggested expansion:

#[cfg(feature = "unstable-risky-function")]
pub fn risky_function() {
    impl_body!()
}

#[cfg(not(feature = "unstable-risky-function"))]
pub(crate) fn risky_function() {
    impl_body!()
}

#[cfg(all(feature = "unstable-risky-function", not(allow_crate_unstable))]
compile_error!("use of feature(unstable-risky-function) requires --cfg allow_crate_unstable");

This has the advantage of providing a targeted error explaining that use of the feature requires a --cfg opt-in (instead of just saying the function is private). A secondary advantage is being a clearly distinct unit able to be cleanly emitted separately, e.g. potentially on an option flag.

It has the downside of duplicating this message for every API gated on the same feature flag. This could be mitigated by the proc macro maintaining a global memo of what feature flags have already had a guard emitted... though I worry both that this mitigation breaks build determinism (e.g. if the macros are handled in a different unspecified order) and might sometimes maintain state between crates (e.g. making whether a feature is gated depend on whether state from a previous compilation is still in cache).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants