Skip to content
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

Evaluate Pure Functions With CTFE #177

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

WalterBright
Copy link
Member

@WalterBright WalterBright commented Oct 2, 2019

Inspired by Don Clugston's idea to add __ctfe as a function attribute.

@rikkimax
Copy link
Contributor

rikkimax commented Oct 3, 2019

This should not need a DIP.

It is effectively an optimization ran by the frontend to minimize code that is ran at runtime.

From the spec:

It can enable optimizations based on the fact that a function is guaranteed to not mutate anything which isn't passed to it.

@PetarKirov
Copy link
Member

PetarKirov commented Oct 9, 2019

One advantage of Don Clugston' idea is that it allows to disable code generation for functions that are only used at compile-time.
When making parts of libraries (including Phobos) BetterC compatible (meaning removing link-time dependency), one trick that I've used is the following (attempt 2):

// before:
string calc(int a, int b) { /* ... */ }
// ...
enum c = calc(1, 2);
// Result: `calc` is emitted in the binary - bad.

// Attempt 1:
enum c = (int a, int b) { /* body of calc */ }(1, 2);
// Result:
// a) `calc` is not part of the binary - good
// b) `calc` is no longer reusable code - bad

// Attempt 2:
enum calc(int a, int b) = { /* body of calc */ }();
enum c = calc!(1, 2);
// Result:
// a) `calc` is not part of the binary - good
// b) `calc` is still reusable code - good
// c) `calc` is now a template which:
//   c.1) allows memoization - good
//   c.2) for many distinct template arguments is slower than plain ctfe - bad.

I think the best solution would be to use regular functions, but for the language to recognize a pattern like assert(__ctfe); and disable code generation for functions that begin with such pattern. That way we don't add new attributes to the language and give extra semantic meaning to an already valid statement.

@PetarKirov
Copy link
Member

Another option is for the language to guarantee that functions marked with pragma(inline, true) are not emitted separately and to forbid their address from being taken.

@rracariu
Copy link

I think adding pragma(ctfe, true) would be a much better option. This basically would force the compiler to perform CTFE and disallow taking the function address, and not overload the inline semantics.

@andralex
Copy link
Member

A pragma sounds nice, too. It would solve the composition problem that this DIP has:

Consider two pure functions f(int) and g(int). Then f(42) is CTFE, g(42) is CTFE, but f(g(42)) is not CTFE which is odd.

@andralex
Copy link
Member

Got to add that determining CTFEability based on the function and call seems at least on the surface a more principled approach than a pragma. Also, it will CTFE existing code.

@WalterBright
Copy link
Member Author

A pragma sounds nice, too. It would solve the composition problem that this DIP has:
Consider two pure functions |f(int)| and |g(int)|. Then |f(42)| is CTFE, |g(42)|
is CTFE, but |f(g(42))| is not CTFE which is odd.

There is no composition problem.

 f(g(42))

Doing constant folding depth first, which is how it is done now and how it's done everywhere else is:

  1. Look at f(g(42))
  2. Evaluate arguments to f()
  3. Look at g(42)
  4. Look at arguments to g()
  5. It's 42! Evaluate g(42)
  6. Argument to f() is a literal, evaluate f(literal)

@WalterBright
Copy link
Member Author

One advantage of Don Clugston' idea is that it allows to disable code generation for functions that are only used at compile-time.

The linker is supposed to remove functions that are never called.

@andralex
Copy link
Member

@WalterBright cool, all I'm saying is that the depth-first folding would be good to mention in the DIP in addition to here. Just paste your text there! :)

@PetarKirov
Copy link
Member

PetarKirov commented Oct 23, 2019

One advantage of Don Clugston' idea is that it allows to disable code generation for functions that are only used at compile-time.

The linker is supposed to remove functions that are never called.

The issue is that the linker thinks those functions are used and as a result, betterC programs fail to link when they use phobos templates such as std.typecons.Tuple which don't have any intrinsic link-time dependencies and should just work. I'll link one of the Bugzilla issues we had to fix for context.

Edit:

@dkorpel
Copy link
Contributor

dkorpel commented May 2, 2020

It recently occurred to me that this is sort of a breaking change with respect to unit tests and test coverage. Many unit tests use literals, and if they are testing pure functions then those tests will be constant folded once this DIP is active. Since code run during ctfe is not traced, the coverage amount will suddenly drop massively for certain projects.

Furthermore, some code paths will actually not be tested anymore.
Imagine you are testing an inline asm function:

int fooSoft() {return 0;}
int fooAsm() {asm{naked; mov RAX, 0; ret;}}
int foo() {return __ctfe ? fooSoft() : fooAsm();}

unittest {
    assert(foo() == 0); // expected to cover fooAsm
    static assert(foo() == 0);
}

Suddenly bugs in fooAsm won't be detected anymore.

I think the DIP should address this.

@Bolpat
Copy link
Contributor

Bolpat commented Nov 3, 2020

About the Prior Work section: Isn't C++'s consteval exactly what Don Clungston suggested __ctfe as a function attribute to do?

@Bolpat
Copy link
Contributor

Bolpat commented Dec 10, 2020

@PetarKirov wrote:

I think the best solution would be to use regular functions, but for the language to recognize a pattern like assert(__ctfe); and disable code generation for functions that begin with such pattern.

I'd say, recognize in (__ctfe) contracts as a pattern. It's more visible than any assert inside the body. A body can be hidden, but that way, it's part of the signature; basically making in (__ctfe) is a pseudo-attribute. Current code could theoretically rely on in (__ctfe) functions hanging around at run-time, i.e. taking their address or other shenanigans like calling them in code that's in fact never executed.

The biggest difference of recognizing in (__ctfe) versus adding a __ctfe an attribute is that overloading based on it is not possible. Whether this difference is good or bad certainly is opinion. Apart from not needing a grammar change and a more than necessary language change, I think recognizing the contract in (__ctfe) is better than adding the attribute __ctfe because of that overloading stuff. If you need a function to act differently based on compile-time function evaluation, put if (__ctfe) { } else { } in it. The only win I see is that using overloading, you could hide the non-ctfe function's code.

@ntrel
Copy link
Contributor

ntrel commented Feb 17, 2022

I'm concerned that automatically evaluating pure calls with literals at compile-time may slow down compilation. How about only doing this when -inline is passed to the compiler? There could still be some per-function way of turning this off even with -inline, if needed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants