Improve the pretty print of UnstableFeature clause#146753
Improve the pretty print of UnstableFeature clause#146753bors merged 1 commit intorust-lang:masterfrom
Conversation
rustbot
added
S-waiting-on-review
|
I am actually not sure if |
|
"enabled" is supported by "This flag allows limiting the features which can be enabled with #![feature(...)] attributes.". |
BoxyUwU
added
the
S-no-work-capacity-tracking
| | ---- this field does not implement `ConstParamTy_` | ||
| | | ||
| note: the `ConstParamTy_` impl for `[u8]` requires that `unstable feature: `unsized_const_params`` | ||
| note: the `ConstParamTy_` impl for `[u8]` requires that `unstable feature `unsized_const_params` is enabled` |
There was a problem hiding this comment.
this kinda results in something like:
"requires that unstable feature unsized const params is enabled"
note how the feature name isnt actually in the backticks. we could drop the backticks and word it as "feature(unsized_const_params) is enabled" which would avoid this problem and still flow naturally I think.
e.g.
"requires that feature(unsized_const_params) is enabled"
or
fn foo where feature(unsized_const_params) is enabled
There was a problem hiding this comment.
we could also go the entire way and just print ClauseKind::UnstableFeature as #![feature({symbol})] so you'd get stuff like:
"requries that #![feature(unsized_const_params)]"
or
fn foo where #![feature(unsized_const_params)]
which also seems.... Kind of okay?
There was a problem hiding this comment.
or just printing it as feature({symbol}) without the macro stuff 🤔
"requires that feature(unsized_const_params)"
or
fn foo where feature(unsized_const_params)
which does read better in the second example but worse in the first 🤔
There was a problem hiding this comment.
I think I prefer printing as feature({symbol}) is enabled, curious what you think
There was a problem hiding this comment.
I think not adding the macro is better because in the case where we want to say "we have an impl that is annotated with #[unstable_feature_bound]" (which is the case here), having #![feature] in the diagnostic can be confusing 🤔
I also prefer feature({symbol}) is enabled because it does flow better in "requires that feature(unsized_const_params) is enabled". It is slightly bad when we have impl<T> Trait for T where feature(bar) is enabled, but I think it is still acceptable.
BoxyUwU
added
S-waiting-on-author
rust-cloud-vms
bot
force-pushed
the
unsatisfiable-unstable-feature
branch
from
6fb4509 to
f259232
Compare
|
Actually it should be possible to just use one |
rust-cloud-vms
bot
force-pushed
the
unsatisfiable-unstable-feature
branch
from
f259232 to
4e62715
Compare
|
@rustbot ready |
rustbot
added
S-waiting-on-review
|
@bors r+ rollup thanks tiif :3 🎮 :slug: |
bors
added
S-waiting-on-bors
Rollup of 7 pull requests Successful merges: - #146556 (Fix duration_since panic on unix when std is built with integer overflow checks) - #146679 (Clarify Display for error should not include source) - #146753 (Improve the pretty print of UnstableFeature clause) - #146894 (Improve derive suggestion of const param) - #146950 (core: simplify `CStr::default()`) - #146958 (Fix infinite recursion in Path::eq with String) - #146971 (fix ICE in writeback due to bound regions) r? `@ghost` `@rustbot` modify labels: rollup
Rollup of 7 pull requests Successful merges: - #146556 (Fix duration_since panic on unix when std is built with integer overflow checks) - #146679 (Clarify Display for error should not include source) - #146753 (Improve the pretty print of UnstableFeature clause) - #146894 (Improve derive suggestion of const param) - #146950 (core: simplify `CStr::default()`) - #146958 (Fix infinite recursion in Path::eq with String) - #146971 (fix ICE in writeback due to bound regions) r? `@ghost` `@rustbot` modify labels: rollup
Rollup merge of #146753 - tiif:unsatisfiable-unstable-feature, r=BoxyUwU Improve the pretty print of UnstableFeature clause As per #145095 (comment), we could make the diagnostic for unsatisfiable ``UnstableFeature`` clause better. r? `@BoxyUwU`
Rollup of 7 pull requests Successful merges: - rust-lang/rust#146556 (Fix duration_since panic on unix when std is built with integer overflow checks) - rust-lang/rust#146679 (Clarify Display for error should not include source) - rust-lang/rust#146753 (Improve the pretty print of UnstableFeature clause) - rust-lang/rust#146894 (Improve derive suggestion of const param) - rust-lang/rust#146950 (core: simplify `CStr::default()`) - rust-lang/rust#146958 (Fix infinite recursion in Path::eq with String) - rust-lang/rust#146971 (fix ICE in writeback due to bound regions) r? `@ghost` `@rustbot` modify labels: rollup
…iaskrgr Rollup of 7 pull requests Successful merges: - rust-lang#146556 (Fix duration_since panic on unix when std is built with integer overflow checks) - rust-lang#146679 (Clarify Display for error should not include source) - rust-lang#146753 (Improve the pretty print of UnstableFeature clause) - rust-lang#146894 (Improve derive suggestion of const param) - rust-lang#146950 (core: simplify `CStr::default()`) - rust-lang#146958 (Fix infinite recursion in Path::eq with String) - rust-lang#146971 (fix ICE in writeback due to bound regions) r? `@ghost` `@rustbot` modify labels: rollup
5 participants
As per #145095 (comment), we could make the diagnostic for unsatisfiable
UnstableFeatureclause better.r? @BoxyUwU