[RFC] ctonly: new annotation to mark functions that are intended to be only used during compile time. #20858
Conversation
For now it's completely ignored by the compiler. I chose not to include it into mangling, since that's supposed to be a front-end only thing. Backends are free to omit `ctonly` functions in codegen, and you should never want to take apart `ctonly` version from non-`ctonly` version during linking. Following that logic, I also chose **not** to check `@ctonly` in `attributesEqual`, so functions that only differ in `@ctonly`ness will result in a compilation error. I think that's reasonable. Please let me know if I missed anything, it seems a new attribute needs to be added in many places, I might have missed something.
The goal of `@ctonly` annotation is to mark functions that are _never_
called during run time.
I believe, to achieve that goal, we need two things:
1. Obviously, check that `@ctonly` functions are called from CTFE
context or from other `@ctonly` functions.
2. Addresses of `@ctonly` functions are never taken.
This commit implements this. As for disabling taking the address of the
function, the implementation is more restrictive, it forbids using
`@ctonly` function as lvalue completely. That probably could be made
less restrictive, for example, it's probably totally fine to return
`@ctonly` function by reference, as long as it stays inside CTFE
context...
This implementation works _almost_ like I want it to work, requiring
_all_ `@ctonly` functions to be _explicitly_ marked. There is an
annoying case though: templates. Consider this example:
```D
int f(int x) @ctonly { return x + 1; }
enum a = map!f([1, 2, 3]).array;
```
This is a perfectly valid code, but this implementation will complain
about `@ctonly` function `f` being called from non-`@ctonly` function
`MapResult!(f, ...).front`.
some test cases for `@ctonly` annotation. `ctonly_simple_map.d` shows how templates could be `@ctonly` aware even with this simple implementation.
It would be nice to be able to use templates from the standard library
with `@ctonly` functions, without having to update the template to be `@ctonly` aware.
To make it work we need to implement the following rules:
1. If a `@ctonly` function `f` is called from a template instance function **and**
`f`is a template alias parameter somewhere in the instantiation stack,
mark calling function as `@ctonly`-inferred because of `f`.
2. If a `@ctonly`-inferred function (with infer reason `f`) is called
from a template instance function **and** `f` is still in the
instantiation stack, mark calling function `@ctonly`-inferred
because of `f`.
3. Otherwise apply rules from the basic implementation.
In this commit I've tried to implement this, but had to take some
shortcuts.
1. It turned out to be pretty complicated to check if a function comes
from an alias parameter... I think I've implemented it, but the code
is ugly. I'm open to suggestions.
2. In rule (1) I don't check the full instantiation stack, just the
top-level instantiation.
3. In rule (2) I don't check if `f` is still a template parameter at
all, just that we are still in a template instantiation (such that we
never infer `@ctonly` for non-templated functions). This could cause
spurious infers which will result in unclear errors like "`@ctonly` `f`
calls non `@ctonly` `g`", but in the code none of them is `@ctonly`.
This could be fixed by either printing the whole promotion chain or
by actually implementing checking the instantiation stack.
|
Thanks for your pull request and interest in making D better, @yanok! We are looking forward to reviewing it, and you should be hearing from a maintainer soon.
Please see CONTRIBUTING.md for more information. If you have addressed all reviews or aren't sure how to proceed, don't hesitate to ping us with a simple comment. Bugzilla referencesYour PR doesn't reference any Bugzilla issue. If your PR contains non-trivial changes, please reference a Bugzilla issue or create a manual changelog. Testing this PR locallyIf you don't have a local development environment setup, you can use Digger to test this PR: dub run digger -- build "master + dmd#20858" |
|
@thewilsonator FYI |
thewilsonator
added
Review:Needs Changelog
|
I still think this would be better as a pragma than an attribute. |
|
This must not be inferred. It prevents codegen, it'll result in some major surprises. Since you didn't use it, I assume you are not aware of the It's used to specify that a function will not be codegen'd and with that may be allowed to do stuff like GC. Line 236 in 8c0967f Also we have a way of detecting CTFE usage But yes attribute good. Oh and this needs to go through the DIP process. |
Personally I'm fine either way, but using an attribute seems a bit more consistent. I'll do a DIP and we'll see what will be the outcome. |
Sure, it can't and must not be inferred in general. That's the basic rules, everything must be marked However, there is this case with passing such functions as template parameters. There is absolutely no harm in inferring that I think for the feature to be really useful, we need to allow this limited kind of inference. But I'm open to other suggestions.
Thanks for the pointer! Yes, it looks like I must set it.
Uh, ok. I'm notably bad at naming things, I just had to give it some name :) (it was
Thanks for the link! I'll start it. Converting the PR into a draft for now then. |
|
I've posted my proposal to the dip.ideas forum: https://forum.dlang.org/post/[email protected] |
If this is appropriate, it should already be handled by A lot of work has gone into that flag to make it work right.
Those types of names have been floated before. I am prioritising keeping the delta to a minimum where there is no benefit to having more changes. |
Hm... I'm not entirely sure, but I think it won't. The two things are indeed related but they are not the same (kinda dual in a way). AFAIU, But the interaction with Regarding the naming, as I said before, I don't have strong opinions here. Another person suggested to use |
This is mostly a RFC and an invitation for discussion, the implementation is hacky and I guess I need to go via the official feature proposal process before the implementation could be merge (please advise!).
Motivation
My motivation is two-fold.
auto v = f(x);instead ofenum v = f(x);, so the function call shifts to run time. We could hope for some constant fold/partial evaluation optimization to kick in the later stages of the compilation, such that it still gets pre-computed at compile time. But why should we rely on that, while D gives us all the controls? Markingfas@ctonlywill easily uncover this unintended usages.@ctonly, backends can skip codegen for that function (currently I’ve only implemented that for LDC in my local branch, but the implementation is really straightforward). That might seem like a micro optimization at first glance, but if a@ctonlyfunction is templated and instantiated widely, code generation cost for it becomes non-trivial.Implementation Ideas
Basically, what we need is to check is that
@ctonlyfunctions are never called outside from CTFE context and that we never take addresses of these functions.New function attribute
Let’s add a new function attribute
@ctonlyto mark CT only functions.This doesn’t require mangling change, since the feature is essentially frontend-only. It also doesn’t make much sense to have both
@ctonlyand non@ctonlyversions of a function with the same signature, so I suggest to not count the new attribute when comparing function attributes for equality.This is implemented in the first commit of the PR.
Basic check rules
CallExp, make sure that if the callee function is@ctonly, either the caller function is also@ctonlyor the call is in CTFE context.@ctonlyfunctions.Basic rules are implemented in the second commit, and the third commit adds some tests to showcase accepted and rejected code snippets.
Basic rules limitations
Though this works in simple cases, unfortunately there is at least one case, where it doesn’t work as I would like. Consider this code:
There is nothing wrong with it,
fis only called during compile time. But with only the basic rules, the compiler fails to understand that. The problem with the basic rules is they require explicit@ctonlyeverywhere. Which I think is good for the most part. Except for this case with templates from the standard library (or any other library actually).There is a way to write
MapResultimplementation such that it checks if a function it gets via its template parameter is@ctonlyand adjusts all callers to also be@ctonly... but that’s going to result in a lot of code duplication and, probably more importantly, will require changes to the stdlib.So what if we enable inference for template instance functions?
Additional rules
@ctonlywas inferred.fis@ctonlyandfis called fromgandgis a template instance function andfis an alias parameter somewhere ing's instantiation stack, markgas inferred@ctonlywith an inference reasonf.fis inferred@ctonlywith reasonrandfis called fromgandgis a template instance function andris an alias parameter somewhere ing's instantiation stack, markgas inferred@ctonlywith an inference reasonr.A hackier version of these rules is implemented in the fourth commit. In particular I only look at the first level of template parameters for initial inference step. And don’t look at template parameters at all for subsequent steps, instead promoting further as long as the caller is a template instance function. This is suboptimal and could be improved.