-
Notifications
You must be signed in to change notification settings - Fork 13k
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
Initial implementation of const fn #25609
Conversation
Oh, I've run tests locally, but not since rebasing. make check is running now. |
@@ -8,6 +8,10 @@ | |||
// option. This file may not be copied, modified, or distributed | |||
// except according to those terms. | |||
|
|||
// TODO list: |
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.
tidy fails on this, it should be FIXME
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.
No, it should not be a FIXME. I always put TODOs intentionally so that I actually am forced to DO the things in question. In this case, though, I DID do that, but forgot I made the TODO list, so I should just remove the comment...
// except according to those terms. | ||
|
||
// Test that we can't declare a const fn in an impl -- right now it's | ||
// just not allowed at all, though eventualy it'd make sense to allow |
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.
s/eventualy/eventually
☔ The latest upstream changes (presumably #24333) made this pull request unmergeable. Please resolve the merge conflicts. |
Rebased. But when I run |
- add feature gate - add basic tests - adjust parser to eliminate conflict between `const fn` and associated constants - allow `const fn` in traits/trait-impls, but forbid later in type check - correct some merge conflicts
Passes everything now. |
abi: self.fty.abi | ||
abi: self.fty.abi, | ||
|
||
// trait methods canot (currently, at least) be const |
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.
typo: cannot
@nikomatsakis if you're still looking for suggestions of tests to add, I noticed that while you have both You might also add tests that uses of conditional ( |
It overall looks good to me; I'm going to r+ it and we can add more tests later, just to avoid future rebase pain. |
This is a port of @eddyb's `const-fn` branch. I rebased it, tweaked a few things, and added tests as well as a feature gate. The set of tests is still pretty rudimentary, I'd appreciate suggestions on new tests to write. Also, a double-check that the feature-gate covers all necessary cases. One question: currently, the feature-gate allows the *use* of const functions from stable code, just not the definition. This seems to fit our usual strategy, and implies that we might (perhaps) allow some constant functions in libstd someday, even before stabilizing const-fn, if we were willing to commit to the existence of const fns but found some details of their impl unsatisfactory. r? @pnkfelix
From what I remember from the RFC, there was less consensus that const functions were the right way long term than that they were the best stop-gap for the time being. That would seemingly point to making them only usable as normal functions from stable, so unstable users can get all the benefits of them without forcing a decision on what we want to do long-term. Finally, if we want to commit to the existence of const functions, we can easily just allow them to be used again from stable code, and std will be ready to go. |
This is a port of @eddyb's
const-fn
branch. I rebased it, tweaked a few things, and added tests as well as a feature gate. The set of tests is still pretty rudimentary, I'd appreciate suggestions on new tests to write. Also, a double-check that the feature-gate covers all necessary cases.One question: currently, the feature-gate allows the use of const functions from stable code, just not the definition. This seems to fit our usual strategy, and implies that we might (perhaps) allow some constant functions in libstd someday, even before stabilizing const-fn, if we were willing to commit to the existence of const fns but found some details of their impl unsatisfactory.
r? @pnkfelix