const_eval: implies_by in rustc_const_unstable

[davidtwco]

Fixes #107605.

Extend support for implies_by (from #[stable] and #[unstable]) to #[rustc_const_stable] and #[rustc_const_unstable].

cc @steffahn

[rustbot]

r? @wesleywiser

(rustbot has picked a reviewer for you, use r? to override)

[steffahn]

Haven’t tried understanding the code in actual detail, but I’m wondering why the implications are separated for the const and non-const case. Same feature flag could gate some normal #[unstable] items and also some other #[rustc_const_unstable] items, couldn’t it? If the feature gates can overlap, wouldn’t a unified table of implications make sense, too?

---

While I’m at it, commenting on the general design… one small downside of the implied_by directionality (as opposed to implies) is that the setting I layed out in the original Zulip thread where a piece of API could be moved to a new feature flag multiple times is no longer so well supported. I do see benefits with this directionality, in particular that the stabilized items will never need to be touched again, but it makes me wonder whether perhaps also allowing a list of multiple features in the implied_by field could be reasonable.

Feel free to ignore this second part of the comment though, as it’s not directly relevant to the PR at hand, I suppose. (And also the use-case should be rather uncommon (and we’re still talking about unstable features anyways), so it’s not a pressing issue.)

[davidtwco]

Haven’t tried understanding the code in actual detail, but I’m wondering why the implications are separated for the const and non-const case. Same feature flag could gate some normal #[unstable] items and also some other #[rustc_const_unstable] items, couldn’t it? If the feature gates can overlap, wouldn’t a unified table of implications make sense, too?

This makes sense, I've updated it to this.

While I’m at it, commenting on the general design… one small downside of the implied_by directionality (as opposed to implies) is that the setting I layed out in the original Zulip thread where a piece of API could be moved to a new feature flag multiple times is no longer so well supported. I do see benefits with this directionality, in particular that the stabilized items will never need to be touched again, but it makes me wonder whether perhaps also allowing a list of multiple features in the implied_by field could be reasonable.

I think I went with the current directionality for the advantage you've noted. Extending the current approach to accept a list of features makes most sense if we were to do this (though I'm not sure if it's worth it if there isn't a need for that).

[steffahn]

Thanks for the quick response.

While I’m at it, more small general thoughts where just asking seems faster than me trying to figure it out from the code of testing: Given that previously, apparently implied_by was allowed but silently ignored inside the rustc_const_unstable attribute… I guess the first though/question is: is there – in general – a check that checks what kind of key-value pairs are allowed in such attributes, and if yes, why was it previously allowing the implied_by on the rustc_const_unstable? Do they kind-of “share the syntax”? My point is that we probably don’t ever want implied_by in a #[stable] or #[rustc_const_stable] attribute (right? or is there any usefulness in something like that?), and helping rustc library authors with a lint or error for this seems like a useful idea.

[apiraino]

(hope it's fine to reroll reviewer, was autoassigned)

r? compiler

[Noratrieb]

Would be nice if the stability and const stability code was a little more unified in the future.
Having stricter checking around stability attributes as mentioned by @steffahn would also be useful, but that's also something for a follow-up PR if deemed useful.
The implementation here looks good.
@bors r+ rollup

[bors]

📌 Commit 9127834 has been approved by Nilstrieb

It is now in the queue for this repository.