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

Forbid #[rustc_const_stable] without #[stable] #79551

Open
CraftSpider opened this issue Nov 30, 2020 · 13 comments
Open

Forbid #[rustc_const_stable] without #[stable] #79551

CraftSpider opened this issue Nov 30, 2020 · 13 comments
Labels
A-const-eval Area: Constant evaluation, covers all const contexts (static, const fn, ...) A-stability Area: `#[stable]`, `#[unstable]` etc. C-enhancement Category: An issue proposing an enhancement or a PR with one. T-libs Relevant to the library team, which will review and decide on the PR/issue.

Comments

@CraftSpider
Copy link
Contributor

Currently, it is possible for a function in std to be marked stably const without the function being stable. In #79548, it was noted that rustdoc assumes that stably const implies stable, and that seems to be a reasonable requirement. There should be an error added to ensure the consistency of const stability.

@jyn514 jyn514 added A-stability Area: `#[stable]`, `#[unstable]` etc. C-enhancement Category: An issue proposing an enhancement or a PR with one. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Nov 30, 2020
@RalfJung
Copy link
Member

Cc @rust-lang/wg-const-eval

@inquisitivecrystal
Copy link
Contributor

Note that the Guide to Rustc Development says that #[rustc_const_stable] is permitted without #[stable].

@oli-obk
Copy link
Contributor

oli-obk commented Jun 4, 2021

This is needed for intrinsics, which are never stable (except for transmute). It may also be needed to allow an unstable const function to be used within a stable const function.

So these two attributes are actually independent. Const stability requirements are recursive in the body, unlike regular stability

@jhpratt
Copy link
Member

jhpratt commented Sep 29, 2021

Aside from intrinsics, are there any situations where this would be the desired behavior? I would imagine it's possible to special-case intrinsics.

@oli-obk
Copy link
Contributor

oli-obk commented Sep 29, 2021

I don't know, there may already be such use cases in libstd. If not, let's do this and worry about it when we have such a case

@jhpratt
Copy link
Member

jhpratt commented Sep 29, 2021

I'll give it a shot when I get a chance — probably next week at the earliest. My initial thought is to have a deny-by-default internal lint that is allowed for the intrinsics module.

@jhpratt
Copy link
Member

jhpratt commented Nov 7, 2021

As I just mentioned in #90356, I recommend this issue be closed. While intrinsics seems to be the most common use-case, others certainly exist. There is one existing usage in stdarch on a private method, and my attempt to fix a bug in const panic will likely introduce another usage.

@RalfJung
Copy link
Member

RalfJung commented Nov 9, 2021

Fine for me as long as we have a test which ensures that such a const fn cannot be directly called from stable user code in any way.

However, there seems to be another problematic case:

Much more problematic to me are stable const fns that have no rustc_const_* attribute at all, as that seems like an oversight and may be an accidental constness stabilization

Do we have an issue tracking that, or tests ensuring that this will work as expected (namely, such functions are not called in const-contexts in stable code)?

@jhpratt
Copy link
Member

jhpratt commented Nov 9, 2021

To my knowledge there's not an open issue for that. For tests, I'm pretty sure that's just regular const-stability checks that are recursive, no?

@RalfJung
Copy link
Member

Well, the tests would have to specifically check the behavior of a stable const fn without any const stability attribute. I guess @oli-obk has a better idea for the kind of example they had in mind.

@jhpratt
Copy link
Member

jhpratt commented Nov 10, 2021

My intent for that was to require #[rustc_const_stable] or #[rustc_const_unstable] on any publicly accessible method. This wouldn't alter the behavior of anything — it would only be a lint. Perhaps I'm just misunderstanding what you're saying, though.

@RalfJung
Copy link
Member

Right, we should have that lint -- but we might not want to rely on it so it might also be worth testing that we fail safely in case there is a screwup.

@jhpratt
Copy link
Member

jhpratt commented Nov 10, 2021

Well, first thing's first is getting the lint 😄 Can definitely add in tests as needed.

@RalfJung RalfJung added A-const-eval Area: Constant evaluation, covers all const contexts (static, const fn, ...) and removed A-const-fn labels Dec 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-const-eval Area: Constant evaluation, covers all const contexts (static, const fn, ...) A-stability Area: `#[stable]`, `#[unstable]` etc. C-enhancement Category: An issue proposing an enhancement or a PR with one. T-libs Relevant to the library team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants