refactor: factor out Config from core::features
#13278
Conversation
To decouple manifest logic from `config.toml` behavior, a trait `UnstableFeatureContext` is created for `Features` to query any necessary information from the outer context. This is a path toward a new package for high-level manifest processing logic. The goal is to break out behavior for `Config` into several small contexts, which can be imported when needed.
|
r? @ehuss (rustbot has picked a reviewer for you, use r? to override) |
rustbot
added
A-configuration
|
Could you lay out your motivation and plan in #4614? I'm a bit concerned that #4614 is a large, invasive project that will affect maintainability with limited gains. Example concerns
|
| @@ -598,7 +598,7 @@ impl Features { | |||
| see_docs() | |||
| ), | |||
| Status::Unstable => { | |||
| if let Some(allow) = &config.cli_unstable().allow_features { | |||
| if let Some(allow) = allow_features { | |||
There was a problem hiding this comment.
imo this change loses some important context of which set of allowed features this is. This is specifically the -Z / CLI features. Maybe cli_allow_features for the trait method and variable??
| use crate::core::features::UnstableFeatureContext; | ||
| use crate::Config; | ||
|
|
||
| impl UnstableFeatureContext for Config { |
There was a problem hiding this comment.
I'm not usually a fan of trait impls being separate from either the trait or the struct definition.
There was a problem hiding this comment.
(annotate-snippets took this to an extreme)
|
@rustbot author |
rustbot
added
S-waiting-on-author
|
Close as this is not relevant at this moment. See #4614 (comment) for possible directions forward. |
What does this PR try to resolve?
To decouple manifest logic from
config.tomlbehavior, a traitUnstableFeatureContextis created forFeaturesto query anynecessary information from the outer context.
This is a path toward a new package for high-level manifest processing
logic. The goal is to break out behavior for
Configinto severalsmall contexts, which can be imported when needed.
How should we test and review this PR?
There is no functional change.
The plan is to add more context impls into
util::config::context.Additional information
Related to #4614