Enabling and disabling features

[Anders429]

Second attempt at probing with features. First attempt was #33.

This PR adds two methods to AutoCfg: enable_feature(), which enables a nightly feature to be used in probes, and disable_feature(), which disables a nightly feature in probes. Specifically, it adds #![feature(<feature>)] at the start of subsequent probes.

This API change should be sufficient to allow users to probe whether nightly-only features will work as expected (see requests for this kind of stuff in #24 and #28). The following example will be possible with this PR:

let mut ac = autocfg::new();
ac.enable_feature("step_trait");
// With the feature enabled, probes will include `#![feature(step_trait)]`.
ac.emit_has_trait("std::iter::Step")
ac.disable_feature("step_trait);
// Further probes will no longer set the feature.

Note that when enabling features on stable or beta channels, any subsequent probes will fail until the feature is disabled.

This seems to be the most direct solution. Users who want to probe for the rustc channel or the presence of a nightly feature should ascertain that the nightly feature they wish to enable works correctly. See the discussion in #33 for why probing for rustc channel and probing features directly are not included here.

[unleashed]

Hi @Anders429 @cuviper,

I'm also looking for a way to test for features and perform additional tests with specific features enabled (ie. features for which an associated path was renamed before being stabilized).

But having to remember about disabling the features again looks like a potential footgun to me. I've been playing around a bit with the idea of fixing that and adding a bit of ergonomics. You might want to check out an experimental branch inspired on this one that takes a closure to run probes and then cleans up the enabled features. This plus a couple small changes to return the status of probes when calling emit_*() fns would allow users to write things like:

  let mut ac = autocfg::new();
  // test for reduce
  // this wouldn't actually work because we can't `use trait::method`,
  // but should be ok for illustration purposes
  if !ac.emit_path("core::iter::Iterator::reduce") {
    // test for pre-1.51.0 nightly fold_first or reduce
    ac.emit_with_features(&["iterator_fold_self"], |featured_ac| {
      // both the two below would require `feature(iterator_fold_self)`
      // so return true if any one of them succeeds so that
      // feature_iterator_fold_self is emitted and user code can do:
      // #![cfg_attr(feature_iterator_fold_self, feature(iterator_fold_self)]
      featured_ac.emit_path("core::iter::Iterator::reduce")
        || featured_ac.emit_path("core::iter::Iterator::fold_first")
    });
  }
  // iter_fold_self no longer enabled here

If there is any interest in going in this direction we can take it from there.

[Anders429]

I think that using a closure would be fine as well. I can't think of any issues with it off the top of my head. It just comes down to which is more user-friendly and more readable, I think.

I personally don't think users having to remember to disable features would be a problem, although I can see where issues might arise, since forgetting to disable a feature would result in some cfgs silently not being emitted when they should be. I will note that @cuviper did mention in the previous thread that only a setter is really needed, since you can always clone the AutoCfg before calling the setter, but I guess that's probably the same level of footgun since it still requires two method calls to work correctly.

As a nitpick, I think having a single method called with_features that just enables the features for the span of the closure would be preferable to having separate probe_with_features and emit_with_features methods. It also saves us from having to make the emit_ flavor of methods have to return bools, which would be a breaking change to the current API.

That leaves me comparing these two APIs: the one in this PR, which is

let mut ac = autocfg::new();

ac.enable_feature("step_trait");
ac.emit_has_trait("std::iter::Step")
ac.disable_feature("step_trait);

and something like

let mut ac = autocfg::new();

ac.with_features(&["step_trait], |ac| {
    ac.emit_has_trait("std::iter::Step");
});

The first one looks cleaner to me, IMO, without the indentations. However, the second one doesn't leave any question about which features apply to which probes, and for that reason I kind of lean toward the second one.

[unleashed]

As a nitpick, I think having a single method called with_features that just enables the features for the span of the closure would be preferable to having separate probe_with_features and emit_with_features methods. It also saves us from having to make the emit_ flavor of methods have to return bools, which would be a breaking change to the current API.

Yes, returning bool's is a breaking change - I don't advocate performing that breaking change, specially because it is unnecessary, but it works to illustrate the point that I'd like to have a mechanism to signal when a feature on nightly is available and something is going to use it.

We could track whether we have emitted anything in the closure and if so then emit the feature flag, or just modify emit to check for enabled features and emit flags for any one of them not emitted yet. Does that plus using a single with_features method sound reasonable?

The first one looks cleaner to me, IMO, without the indentations. However, the second one doesn't leave any question about which features apply to which probes, and for that reason I kind of lean toward the second one.

Agreed, that's why I took a stab at it in the first place. To be honest, I've found in my own usage that what I want most of the time, as opposed to using directly something such as with_features, is this:

• Test something without feature enabled.
• If it fails and we're passed an associated feature in, perform the test again with that feature enabled.
• Emit flags for something if available with or without feature, and emit a flag for feature if it is needed to use something.

Perhaps a helper that does this, or a change to with_(maybe)_features to use these semantics, would be beneficial.

[cuviper]

Another way to use it is to simply clone() the AutoCfg, enable the feature and test with it, then drop the clone.

If we did a with_features thing, I would not try to implicitly emit any cfg -- just let the user write what they want.