-
Notifications
You must be signed in to change notification settings - Fork 12.9k
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
Require stable/unstable annotations for the constness of all stable fns with a const modifier #67136
Conversation
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
@@ -83,15 +88,36 @@ impl<'tcx> TyCtxt<'tcx> { | |||
} | |||
|
|||
if self.features().staged_api { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The function this is in is called is_min_const_fn
, but the body comments make it sounds more like "is the fn required to conform to min_const_fn restrictions", which is not reflected in the function name. Maybe must_be_min_const_fn
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
well... it's both :D We call is_min_const_fn
to check whether a function can be called from other min const fn. When checking a function we call is_min_const_fn
to check if we are callable from other min const fn and thus must conform.
// Note: this is an arbitrary choice that does not affect stability or const | ||
// safety or anything, it just changes whether we need to annotate some | ||
// internal functions with `rustc_const_stable` or with `rustc_const_unstable` | ||
true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So internal unannounced functions are more restricted than stable functions with #[rustc_const_unstable]
? That seems odd.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah I see @Centril asked for this. Well I think this is somewhat silly and would prefer to treat functions without staging attributes the same as unstable functions, but I won't fight over this.^^
I'm strongly in favor of the general approach! However, I am unfamiliar with how stability checking is implemented, so I'll have to pass on the review. r? @Centril |
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
r=me with comments addressed
find_stability_generic(sess, attrs.iter(), item_sp) | ||
} | ||
|
||
fn find_stability_generic<'a, I>(sess: &ParseSess, | ||
attrs_iter: I, | ||
item_sp: Span) | ||
-> Option<Stability> | ||
-> (Option<Stability>, Option<ConstStability>) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have a feeling this function is doing way too much but let's not overhaul it here; making a note-to-self for now tho. I think the right structure would be to look for each attribute form independently and then do a post-processing step to merge information.
0dacbac
to
50bc0d2
Compare
@bors r=Centril |
📌 Commit 50bc0d2f72babab9d9038274a0ba1e93ebcca85c has been approved by |
⌛ Testing commit 50bc0d2f72babab9d9038274a0ba1e93ebcca85c with merge 49c221ca4e14ea4210101b3be94ee74be1e6b444... |
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
💔 Test failed - checks-azure |
…unctions with a `const` modifier
50bc0d2
to
f12affe
Compare
@bors r=Centril |
📌 Commit f12affe has been approved by |
@bors r- |
@bors r=Centril |
📌 Commit 0b47ba7 has been approved by |
Require stable/unstable annotations for the constness of all stable fns with a const modifier r? @RalfJung @Centril Every `#[stable]` const fn now needs either a `#[rustc_const_unstable]` attribute or a `#[rustc_const_stable]` attribute. You can't silently stabilize the constness of a function anymore.
Require stable/unstable annotations for the constness of all stable fns with a const modifier r? @RalfJung @Centril Every `#[stable]` const fn now needs either a `#[rustc_const_unstable]` attribute or a `#[rustc_const_stable]` attribute. You can't silently stabilize the constness of a function anymore.
☀️ Test successful - checks-azure |
r? @RalfJung @Centril
Every
#[stable]
const fn now needs either a#[rustc_const_unstable]
attribute or a#[rustc_const_stable]
attribute. You can't silently stabilize the constness of a function anymore.