-
Notifications
You must be signed in to change notification settings - Fork 14k
Add very basic "comptime" fn implementation #148820
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
base: main
Are you sure you want to change the base?
Changes from all commits
c8f3769
405e50f
e1f7d8f
783719e
dc3c148
953a740
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1390,6 +1390,11 @@ pub static BUILTIN_ATTRIBUTES: &[BuiltinAttribute] = &[ | |
| rustc_force_inline, Normal, template!(Word, NameValueStr: "reason"), WarnFollowing, EncodeCrossCrate::Yes, | ||
| "`#[rustc_force_inline]` forces a free function to be inlined" | ||
| ), | ||
| rustc_attr!( | ||
| rustc_comptime, Normal, template!(Word), ErrorFollowing, | ||
| EncodeCrossCrate::Yes, | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It this necessary? The other queries would store this information
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. find_attr!(tcx.get_all_attrs(), AttributeKind::Comptime(..))would do the same (slightly slower?) but avoids a new query which might be worth it. You can set
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In fact, you did mark it as cross-crate encodable oli, so if we keep the query you can turn cross crate encoding of the attribute off (see my other comment) |
||
| "the `#[rustc_comptime]` attribute is just used to avoid adding syntax for `comptime fn`" | ||
| ), | ||
|
|
||
| // ========================================================================== | ||
| // Internal attributes, Testing: | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -28,6 +28,7 @@ impl AttributeKind { | |
| BodyStability { .. } => No, | ||
| Coinductive(..) => No, | ||
| Cold(..) => No, | ||
| Comptime(..) => Yes, | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. can be |
||
| Confusables { .. } => Yes, | ||
| ConstContinue(..) => No, | ||
| ConstStability { .. } => Yes, | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2264,10 +2264,12 @@ pub enum ConstContext { | |
| /// - Array length expressions | ||
| /// - Enum discriminants | ||
| /// - Const generics | ||
| /// | ||
| /// For the most part, other contexts are treated just like a regular `const`, so they are | ||
| /// lumped into the same category. | ||
| Const { inline: bool }, | ||
| Const { | ||
| /// For backwards compatibility `const` items allow | ||
| /// calls to `const fn` to get promoted. | ||
| /// We forbid that in comptime fns and inline consts. | ||
| allow_const_fn_promotion: bool, | ||
| }, | ||
| } | ||
|
|
||
| impl ConstContext { | ||
|
|
@@ -4225,13 +4227,15 @@ impl fmt::Display for Safety { | |
|
|
||
| #[derive(Copy, Clone, PartialEq, Eq, Debug, Encodable, Decodable, HashStable_Generic)] | ||
| pub enum Constness { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Personally speaking I would rename the variants like so: pub enum Constness {
Always, // `#[rustc_comptime]`
Maybe, // `const`
Never,
}to mirror If it were up to me, I would even rename
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I prefer this naming over the suggestion I made in https://github.com/rust-lang/rust/pull/148820/files#r2517464261 actually |
||
| Comptime, | ||
| Const, | ||
|
Comment on lines
+4230
to
4231
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Seems worth doc comments distinguishing these two. |
||
| NotConst, | ||
| } | ||
|
|
||
| impl fmt::Display for Constness { | ||
| fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { | ||
| f.write_str(match *self { | ||
| Self::Comptime => "comptime", | ||
| Self::Const => "const", | ||
| Self::NotConst => "non-const", | ||
| }) | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2567,6 +2567,7 @@ impl<'a> State<'a> { | |
| match s { | ||
| hir::Constness::NotConst => {} | ||
| hir::Constness::Const => self.word_nbsp("const"), | ||
| hir::Constness::Comptime => self.word_nbsp("comptime"), | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Comptime is an attribute for now right; |
||
| } | ||
| } | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -917,7 +917,18 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { | |
| return; | ||
| } | ||
|
|
||
| let host = match self.tcx.hir_body_const_context(self.body_id) { | ||
| let const_context = self.tcx.hir_body_const_context(self.body_id); | ||
|
|
||
| if let hir::Constness::Comptime = self.tcx.constness(callee_did) { | ||
| match const_context { | ||
| Some(hir::ConstContext::Const { .. } | hir::ConstContext::Static(_)) => {} | ||
| Some(hir::ConstContext::ConstFn) | None => { | ||
| self.dcx().span_err(span, "comptime fns can only be called at compile time"); | ||
| } | ||
| } | ||
|
Comment on lines
+922
to
+928
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This doesn't enforce cross crate if I use a Would it make sense to also add logic to constck? (check_const.rs)
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Well I'm working on turning this check on unconditionally. I can just move the comptime check before the feature gate check |
||
| } | ||
|
|
||
| let host = match const_context { | ||
| Some(hir::ConstContext::Const { .. } | hir::ConstContext::Static(_)) => { | ||
| ty::BoundConstness::Const | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -180,6 +180,7 @@ fixed_size_enum! { | |
| hir::Constness { | ||
| ( NotConst ) | ||
| ( Const ) | ||
| ( Comptime ) | ||
| } | ||
| } | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2138,7 +2138,7 @@ impl<'tcx> TyCtxt<'tcx> { | |
| matches!( | ||
| self.def_kind(def_id), | ||
| DefKind::Fn | DefKind::AssocFn | DefKind::Ctor(_, CtorKind::Fn) | DefKind::Closure | ||
| ) && self.constness(def_id) == hir::Constness::Const | ||
| ) && self.constness(def_id) != hir::Constness::NotConst | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I know this enum is unlikely to change any time soon, but might be nicer to write as a match exhaustively naming the variants |
||
| } | ||
|
|
||
| /// Whether this item is conditionally constant for the purposes of the | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2449,7 +2449,20 @@ impl<'a> Parser<'a> { | |
| case: Case, | ||
| ) -> PResult<'a, (Ident, FnSig, Generics, Option<Box<FnContract>>, Option<Box<Block>>)> { | ||
| let fn_span = self.token.span; | ||
| let header = self.parse_fn_front_matter(vis, case, FrontMatterParsingMode::Function)?; // `const ... fn` | ||
| let mut header = self.parse_fn_front_matter(vis, case, FrontMatterParsingMode::Function)?; // `const ... fn` | ||
| if let Some(attr) = attrs.iter().find(|attr| attr.has_name(sym::rustc_comptime)) { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This doesn't account for Even if you anticipate it being turned into a keyword / qualifier eventually, you can't assume that and it would be super easy anyway to update the parser & AST once it's truly necessary. |
||
| match std::mem::replace(&mut header.constness, Const::Comptime(attr.span)) { | ||
| Const::Comptime(_) => unreachable!("there is no syntax for comptime fn"), | ||
| // A function can't be `const` and `comptime` at the same time | ||
| Const::Yes(span) => { | ||
| return Err(self | ||
| .dcx() | ||
| .create_err(errors::ConstComptimeFn { span, attr_span: attr.span })); | ||
| } | ||
| // Good | ||
| Const::No => {} | ||
| } | ||
| } | ||
| let ident = self.parse_ident()?; // `foo` | ||
| let mut generics = self.parse_generics()?; // `<'a, T, ...>` | ||
| let decl = match self.parse_fn_decl(&fn_parse_mode, AllowPlus::Yes, RecoverReturnSign::Yes) | ||
|
|
@@ -2786,7 +2799,7 @@ impl<'a> Parser<'a> { | |
| // that the keyword is already present and the second instance should be removed. | ||
| let wrong_kw = if self.check_keyword(exp!(Const)) { | ||
| match constness { | ||
| Const::Yes(sp) => Some(WrongKw::Duplicated(sp)), | ||
| Const::Comptime(sp) | Const::Yes(sp) => Some(WrongKw::Duplicated(sp)), | ||
| Const::No => { | ||
| recover_constness = Const::Yes(self.token.span); | ||
| match parsing_mode { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,7 @@ | ||
| #![feature(rustc_attrs)] | ||
|
|
||
| #[rustc_comptime] | ||
| const fn foo() {} | ||
| //~^ ERROR a function cannot be both `comptime` and `const` | ||
|
|
||
| fn main() {} |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,10 @@ | ||
| error: a function cannot be both `comptime` and `const` | ||
| --> $DIR/const_comptime.rs:4:1 | ||
| | | ||
| LL | #[rustc_comptime] | ||
| | ----------------- `comptime` because of this | ||
| LL | const fn foo() {} | ||
| | ^^^^^ help: remove the `const` | ||
|
|
||
| error: aborting due to 1 previous error | ||
|
|
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,24 @@ | ||
| #![feature(rustc_attrs, const_trait_impl)] | ||
|
|
||
| #[rustc_comptime] | ||
| fn foo() {} | ||
|
|
||
| fn main() { | ||
| // Ok | ||
| const { foo() }; | ||
| // Not ok | ||
| foo(); | ||
| //~^ ERROR: comptime fns can only be called at compile time | ||
| } | ||
|
|
||
| const fn bar() { | ||
| // Not ok | ||
| foo(); | ||
| //~^ ERROR: comptime fns can only be called at compile time | ||
| } | ||
|
|
||
| #[rustc_comptime] | ||
| fn baz() { | ||
| // Ok | ||
| foo(); | ||
| } |
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.
Would be nice to some day make all the variant names meaningful. Something like
Looking at your PR yday that's one of the first things I thought. First decided not to comment it because it might not be for this PR but coming back I thought I'd mention it anyway.