Yanok/ctfe fun attr 1.30 by yanok · Pull Request #6 · weka/ldc

yanok · 2025-02-17T16:12:20Z

Support for @__ctfe attribute for marking CT-only functions, back ported to 1.30.

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 `__ctfe` functions in codegen, and you should never want to take apart `__ctfe` version from non-`__ctfe` version during linking. Following that logic, I also chose **not** to check `@__ctfe` in `attributesEqual`, so functions that only differ in `@__ctfe`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 `@__ctfe` 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 `@__ctfe` functions are called from CTFE context or from other `@__ctfe` functions. 2. Addresses of `@__ctfe` 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 `@__ctfe` function as lvalue completely. That probably could be made less restrictive, for example, it's probably totally fine to return `@__ctfe` function by reference, as long as it stays inside CTFE context... This implementation works _almost_ like I want it to work, requiring _all_ `@__ctfe` functions to be _explicitly_ marked. There is an annoying case though: templates. Consider this example: ```D int f(int x) @__ctfe { return x + 1; } enum a = map!f([1, 2, 3]).array; ``` This is a perfectly valid code, but this implementation will complain about `@__ctfe` function `f` being called from non-`@__ctfe` function `MapResult!(f, ...).front`.

It would be nice to be able to use templates from the standard library with `@__ctfe` functions, without having to update the template to be `@__ctfe` aware. To make it work we need to implement the following rules: 1. If a `@__ctfe` 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 `@__ctfe`-inferred because of `f`. 2. If a `@__ctfe`-inferred function (with infer reason `f`) is called from a template instance function **and** `f` is still in the instantiation stack, mark calling function `@__ctfe`-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 `@__ctfe` for non-templated functions). This could cause spurious infers which will result in unclear errors like "`@__ctfe` `f` calls non `@__ctfe` `g`", but in the code none of them is `@__ctfe`. This could be fixed by either printing the whole promotion chain or by actually implementing checking the instantiation stack.

yanok · 2025-02-25T11:30:46Z

@ljmf00-wekaio @JohanEngelen FYI, I don't seem to have rights to assign reviewers :)

ljmf00-wekaio · 2025-02-25T11:43:38Z

            this.isproperty = true;
        if (stc & STC.live)
            this.islive = true;
+        if (stc & STC.ctonly)


This is an unfortunate duplication that DMD still has on many places, you miss some places:

typeSemantic.visitFunction on typesem.d adds the STC inherited from the previous scope

addStorageClass.visitFunction on typesem.d

I would recommend adding a few tests:

a simple one with @__ctfe

a test with inner functions, make sure inner functions are also not generated (I think this is the main case where typesem acts)

This should also work, I believe this falls into some code that inherits the attributes:

@__ctfe: void foo(){} void bar(){}

Right, thanks for the pointers! I think I even found that @__ctfe: doesn't work but didn't pay much attention to it.

Regarding nested functions: I actually wanted to make it not automatically inherited. The reason was I was worried there will be no way to "undo" the attribute and what if we actually want to compile-time generate something with references to run-time things? Like a table with closures or function pointers? But provided closures are not supported inside CTFE anyway and for function pointers a fix would be to unnest the referenced functions, probably it is not such a big deal and we can make it inheritable.

Being said that, the existing implementation does something in the middle: the attribute is not inherited, as you pointed out. So, errors for inner functions are not emitted. But the code for inner functions is not generated anyway :) I guess if I want to support @__ctfe not being inherited, I need to recurse into children in the glue code, instead of returning immediately (another reason to do the inheritance :) ) That's a bug and needs to be fixed one way or another.

I've changed visitFunction in typesem.d and now @__ctfe: works. But I couldn't find the second place you mentioned, could you please point me to the code once again? Thanks!

ljmf00-wekaio · 2025-02-25T11:57:05Z

+        // That's a hacky way to check if exp.e1 is an alias template parameter.
+        // I found that such aliases have parent == null, but I'm not sure if
+        // this guarantees that it's a template parameter.


So, this is to support alias something = __ctfe?

Oh no, I don't think that will work... (since the code compares identifiers) I'm not sure I want this to work though :)

That chunk of code tries to do this:

we are analyzing a CallExp, so exp.e1 is a callable

I want to detect if exp.e1 comes from a (alias?) template parameter, so if check something like:
int(alias f)(int x) { return f(x); }
I want fIsAliasParam to be true when checking the application f(x). Unfortunately I didn't find a nicer way to achieve this.

ljmf00-wekaio · 2025-02-25T12:00:35Z


 #define STC_TYPECTOR    (STCconst | STCimmutable | STCshared | STCwild)
-#define STC_FUNCATTR    (STCref | STCnothrow | STCnogc | STCpure | STCproperty | STCsafe | STCtrusted | STCsystem)
+#define STC_FUNCATTR    (STCref | STCnothrow | STCnogc | STCpure | STCproperty | STCsafe | STCtrusted | STCsystem | STCctonly)


You also missed frontend.d, I believe these files are partially auto-generated from frontend.d code. Its mainly for DMD as a library / plugins, maybe only used by DMD.

Hm... But I don't see frontend.d in the source code. Could you please point me to it? Thanks!

ljmf00-wekaio · 2025-02-25T12:11:57Z

        SCstring(STC.disable, "@disable"),
        SCstring(STC.future, "@__future"),
        SCstring(STC.local, "__local"),
+        SCstring(STC.ctonly, "@__ctfe"),


hdrgen also has a simple way to add tests, its cool for sanity, but I have a few points/questions here:

forward declarations of @__ctfe functions doesn't make much sense, and they should be forbidden

we should add logic here to only generate @__ctfe functions, whey they are template declarations. Forward declarations of template instances should also be forbidden.

e.g. this doesn't make sense to support and should be forbidden:

void foo() @__ctfe; void foo(string)() @__ctfe;

Oh, I didn't know that generating a header makes non-templated module definitions opaque and thus unusable with CTFE. Yes, than there is no much sense in generating declarations for them...

OTOH, wouldn't people be surprised if declarations will just disappear from the header? Maybe it will be a better UX to add the declaration and let the compiler complain in a usual way (f cannot be interpreted at compile time)?

Anyway, I think hdrgen is a lower priority for the Weka fork.

Speaking about forward declarations in general, unrelated to hdrgen, I think there is nothing wrong with them, as long as there is a matching definition. And I've found that my implementation unfortunately has this issue: it accepts

int f() @__ctfe; int f() { return 2; }

and makes f non-CTFE. This is a bug and should be fixed, either by prohibiting forward declarations with @__ctfe completely or by enforcing that the declaration and the definition are in agreement. I haven't found the code that does that yet. I cannot define both @__ctfe and non-@__ctfe versions of f with the same signature, but somehow I can declare it differently.

ljmf00-wekaio · 2025-02-25T12:13:48Z

        bool isctor;           /// the function is a constructor
        bool isreturnscope;    /// `this` is returned by value
+        bool isCtonly;         /// is compile time only (@__ctfe)
+        bool isCtonlyInferred; /// was compile time only inferred


Would be cool to also add a test around functions that inherit inference from template instances instantiated from ctfe functions. We also need to make sure two functions that use the same template instance but one of the function is marked as @__ctfe the other is not, codegen is still done (logic to invalidate isCtonlyInferred).

That's an inference in another direction actually :)

I'm not trying to tell that if f is @__ctfe and f instantiates some template T with template parameters pp, then code from T!pp could skip codegen. I tried experimenting with this direction as well, but it turned to be more complicated than I thought initially and I realized I don't really need that for sound codegen skip of marked functions. In the end, I think that's what was implemented in recent DMD versions around the skipCodegen flag.

So, my inference goes in the other direction. If I know that some function g:

is a template instance

calls a @__ctfe function f

f happens to be a template parameter somewhere in the instantiation stack of g

then g must be @__ctfe. Think map!f(arr), here g = MapResult!(f, arr).front. It calls f, so it has to be @__ctfe.

And if there is another function using the same template, it will be another instance, because the function itself is in the instance parameters.

In general, in this kind of inference (that's essentially a "must" condition propagation), we can't really quit and say "ok, but then I'll just codegen this". We can only decide if we want to complain here already, or try marking the template instance @__ctfe and continue.

ljmf00-wekaio · 2025-02-25T12:36:18Z

+        if (auto fd = var.isFuncDeclaration()) {
+            auto fty = fd.type.toTypeFunction();
+            if (fty.isCtonly()) {
+                error("cannot take address of CTFE-only function `%s`", var.toChars());


This is more complicated than it seems, because under CTFE context, this should be allowed, you should guard the code with sc.flags & SCOPE.ctfe, but I think its not enough, because it would only guard for CTFE instances.

Maybe do a test with taking addresses inside CTFE and on functions with @__ctfe that are also only @__ctfe. So this check should also contemplate that case.
I would go even further, why wouldn't we be able to take the address of a non-@__ctfe function, if that address is only used in the local scope?

In general, I believe this check should be elsewhere, near the fe/be glue code, because, taking address should be forbidden only when its stored in globals, no?

I think it's totally fine to stay on the conservative side here. What is the address of a CT-only function anyway? Yes, we can support some cases with first taking an address inside CTFE and then calling it using that address also inside CTFE. But that requires more complicated analysis: what if an address is casted to an integer, put into a structure and returned? It's hard to detect if it's going to leak to run-time.

That makes things like: ```d @__ctfe: int foo() { return 1; } int bar() { return 2; } ``` work properly. But not nested functions.

Co-authored-by: Luís Ferreira <107841641+ljmf00-wekaio@users.noreply.github.com>

Ilya Yanok added 4 commits February 18, 2025 11:30

declarations: skip codegen for ctonly functions

9ac2a17

yanok force-pushed the yanok/ctfe-fun-attr-1.30 branch from 8a5dbc4 to 9ac2a17 Compare February 18, 2025 10:30

yanok marked this pull request as ready for review February 25, 2025 11:28

ljmf00-wekaio requested review from JohanEngelen and ljmf00-wekaio February 25, 2025 11:35

ljmf00-wekaio reviewed Feb 25, 2025

View reviewed changes

Ilya Yanok and others added 9 commits March 11, 2025 17:27

ctonly: tests for basic (non-inference) cases

06264b7

ctonly: tests for inference

b389c17

ctonly: add RUN to inference tests

e90c3c1

ctonly: add RUN to basic tests

335a886

Fix check lines for template tests

0cd9285

Fix CHECK lines for basic tests

43ad824

ctonly: add a test for attribute inheritance

3c0aa4d

ctonly: propagate @__ctfe attribute from scope

20620af

That makes things like: ```d @__ctfe: int foo() { return 1; } int bar() { return 2; } ``` work properly. But not nested functions.

ctonly: add a comment for skip codegen line

6540983

Co-authored-by: Luís Ferreira <107841641+ljmf00-wekaio@users.noreply.github.com>

yanok requested a review from ljmf00-wekaio March 25, 2025 12:03

yanok force-pushed the weka-1.30 branch 2 times, most recently from 940f308 to 15473b3 Compare November 1, 2025 16:37

Conversation

yanok commented Feb 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

yanok commented Feb 25, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

yanok Mar 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

yanok Mar 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

yanok commented Feb 17, 2025 •

edited

Loading

yanok Mar 17, 2025 •

edited

Loading

yanok Mar 17, 2025 •

edited

Loading