Fix issue 18472: betterC cannot use format at compile time. #14676

etcimon · 2022-12-07T19:39:29Z

For various reasons, isInputRange!string is now false even during CTFE because a string in betterC doesn't have the requirements to be considered a range. The Appender and format will not work, much like a lot of the standard library.

However, CTFE is mainly based on being able to allocate a new T during the interpret phase, which means we need to include TypeInfo in the code but avoid the external references during linking. These changes open up the opportunity to do it in a if (__ctfe) statement, so that the same function can be used during runtime in betterC without a TypeInfo or GC allocation error.

I'm currently working on adding diet-ng to the @skoppe/spasm library using my own memory allocation libraries that will fall back to new T or new T[] when necessary, and currently CTFE is nearly impossible due to the allocations being blocked.

maxhaton · 2022-12-08T15:15:33Z

compiler/src/dmd/backend/util2.d

@@ -41,7 +41,7 @@ void file_progress()
 /****************************
 * Clean up and exit program.
 */
-
+@trusted


Is this required?

This one is unrelated, the master branch DMD was erroring because of a @system call in @safe code

maxhaton · 2022-12-08T15:17:55Z

compiler/src/dmd/toobj.d

@@ -715,7 +715,8 @@ void toObjFile(Dsymbol ds, bool multiobj)

        override void visit(TypeInfoDeclaration tid)
        {
-            if (isSpeculativeType(tid.tinfo))
+            import dmd.globals : global;
+            if (isSpeculativeType(tid.tinfo) || global.params.betterC)


I'm Not in love with this.

Should be 1 condition: does it need emission or not, rather than isSpeculative or if the compiler is in betterC.

i.e. do the logic somewhere.else

It could also be if !global.params.useTypeInfo. The problem is that the TypeInfo is generated by CTFE, and DsymbolTable gets populated with external references.

I think a better option would be to clean it up, I tried adding a global ctfe semaphore to keep track of how deep we are in startCTFE (++) and endCTFE(--), isInterpreting(++/--) and actually forget about betterC by setting the useTypeInfo to true during CTFE. I was able to compile a string through std.format that way but the ~30 phobos / druntime external references caused a linker problem and the application ended with the semaphore at 222. Without the semaphore, the compiler seems to consider it's not even in CTFE after a few calls deep within phobos. It looks like the Scope is lost in a TemplateInstance. It seems like a lot of work to get phobos running through ctfe in betterC

Actually, a betterC CTFE symbol could almost be considered speculative? Hmm

What I mean is that there's no single global state or symbol tracking for ctfe execution. It goes in and out of ctfe temporarily on occasion and in many cases the execution path code ends up in the binary.

maxhaton · 2022-12-08T15:18:43Z

druntime/test/betterc/src/test18472.d

@@ -0,0 +1,37 @@
+/*******************************************/


This could be a compiler test rather than a druntime one I think

Where would that go? I couldn't find

compiler/test/compilable or maybe runnable if absolutely necessary

dkorpel

I'm not sure the implementation is right. The ctfeGlobals.active doesn't look like it respects scoping rules. It looks like the new logic is only active with -betterC, so it gets very limited test coverage. I suspect it handles this wrong and allows you to cheat @nogc:

auto getFun() {
    if (__ctfe) {
        static ubyte[] gcAlloc() @nogc {return new ubyte[10];}
        return &gcAlloc;
    }
    return null;
}

immutable fun = getFun();

void main() {
    fun();
}

dkorpel · 2022-12-08T18:53:49Z

compiler/src/dmd/dinterpret.d

@@ -206,6 +210,10 @@ void incArrayAllocs()
    ++ctfeGlobals.numArrayAllocs;
 }

+bool isInterpreting() {


This function looks pointless. It's also missing a DDoc comment and has the wrong brace style ({ should be on the next line).

This function is currently the only way to know whether we're in CTFE

if (ctfeGlobals.active)

It's marked private

I added a new SCOPE.ctfeonly which makes this fail with a @nogc error correctly now

dkorpel · 2022-12-08T18:56:39Z

compiler/src/dmd/dinterpret.d

@@ -227,6 +235,7 @@ struct CtfeGlobals
    int maxCallDepth = 0;     // highest number of recursive calls
    int numArrayAllocs = 0;   // Number of allocated arrays
    int numAssignments = 0;   // total number of assignments executed
+    bool active;              // whether we are interpreting a ctfe function


How's this different from SCOPE.ctfe?

The SCOPE.ctfe flag is not set when going through the interpret phase which is why we get the TypeInfo error within a static assert. Also, this flag seems to be temporarily turned off during execution anyways and is not reliable. https://github.com/dlang/dmd/blob/master/compiler/src/dmd/expressionsem.d#L3965

etcimon · 2022-12-08T19:54:05Z

I'm not sure the implementation is right. The ctfeGlobals.active doesn't look like it respects scoping rules. It looks like the new logic is only active with -betterC, so it gets very limited test coverage. I suspect it handles this wrong and allows you to cheat @nogc

It does handle this wrong, because anything within if (__ctfe) specifically doesn't get any IR so this results test2.obj : error LNK2001: unresolved external symbol _D5test26getFunFZ7gcAllocFNaNbNfZAh. But when using if (!__ctfe) } else { we do get the @nogc error.

test2.d(3): Error: expression `new ubyte[](10LU)` uses the GC and cannot be used with switch `-betterC`

or without -betterC

test2.d(3): Error: cannot use `new` in `@nogc` function `test2.getFun.gcAlloc`

etcimon · 2022-12-08T20:13:38Z

Without static if (__ctfe), I don't think we can even find a logical way to make any of this work, because if implies that we keep the code designed for ctfe even though it may escape the scope of ctfe. And for my tests to work, I need to make sure everything inside if (__ctfe) behaves like a static if in betterC otherwise we get references to D features that don't exist.

etcimon · 2022-12-08T20:20:28Z

Basically, this is the behavior I want to achieve

@noCodeGen if (__ctfe) {
  new T();
}

etcimon · 2022-12-08T20:50:08Z

I think D needs to have a @skipir attribute to really track ctfe and filter out Dsymbols that refer to unsupported features. When skipping IR we could run ctfe in betterC with the full feature set and phobos. Do you think that would be a better solution?

dkorpel · 2022-12-08T21:11:17Z

Without static if (__ctfe)

I think dmd could recognize that pattern and skip code gen for whatever is inside it.

I think D needs to have a @skipir attribute to really track ctfe and filter out Dsymbols that refer to unsupported features. When skipping IR we could run ctfe in betterC with the full feature set and phobos. Do you think that would be a better solution?

That's somewhat similar to pragma(ctfe) (#14036), and it's a mixed bag to me. On the one hand, enabling GC/Exceptions/Phobos during ctfe for betterC is awesome of course. On the other hand, ctfe should just simply work, and the more attributes / special code paths required to make it work, the worse the user experience.

One simple solution is moving betterC errors from compile time to run time, by emitting assert(0, "GC not available in betterC") during code gen for things that require GC allocations, and similar for exceptions and other unavailable constructs.

maxhaton · 2022-12-08T21:22:11Z

I think the existing CFA code might be able to recognize the CTFE patterns. I tried doing it but I don't remember if it worked or not

etcimon · 2022-12-08T21:47:39Z

One simple solution is moving betterC errors from compile time to run time

I'll add that. The other issue is that the code would not be @nogc even though it should.

etcimon · 2022-12-08T22:30:23Z

I think the existing CFA code might be able to recognize the CTFE patterns.

Come to think about it, there should be a flag on each Dsymbol about whether it was identified in a chain of runtime calls to filter out ctfe references from being included in the final binary. Combined with static if (__ctfe) this would make the compiler smart enough to run even phobos in a betterC ctfe.

dlang-bot · 2022-12-09T01:17:33Z

Thanks for your pull request and interest in making D better, @etcimon! We are looking forward to reviewing it, and you should be hearing from a maintainer soon.
Please verify that your PR follows this checklist:

My PR is fully covered with tests (you can see the coverage diff by visiting the details link of the codecov check)
My PR is as minimal as possible (smaller, focused PRs are easier to review than big ones)
I have provided a detailed rationale explaining my changes
New or modified functions have Ddoc comments (with Params: and Returns:)

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 references

Auto-close	Bugzilla	Severity	Description
✓	18472	regression	[Reg 2.078] betterC: cannot use format at compile time.

Testing this PR locally

If you don't have a local development environment setup, you can use Digger to test this PR:

dub run digger -- build "master + dmd#14676"

etcimon · 2022-12-09T01:24:38Z

The static if solution wouldn't work because it's not considered in the >=semantic2done pass / there's no way to dynamically rebuild the FuncDeclaration with a changing expression. What would work is a if (__static_ctfe) which behaves similarly and has its IR skipped away from the binary. It would be possible to extend it into a global state for the compiler to call functions unavailable in betterC and have them stripped out from the IR as well.

Also, I think the compile-time errors for TypeInfo are a little too useful to resort to removing them for now

etcimon · 2022-12-10T20:26:03Z

I think the existing CFA code might be able to recognize the CTFE patterns

It could, but only if the code initially goes through CTFE and evaluates the condition that contains __ctfe. You can't reliably predict what would happen in that if statement's condition at runtime, it could have function calls that invert the value of __ctfe, so, much of the stuff not allowed in betterC (that you want in CTFE) can be cheated if the expression is complex and improperly "guessed".

ljmf00 · 2022-12-10T21:14:55Z

Needs spec PR since adding a new CTFE boolean pseudo-variable. See https://dlang.org/spec/function.html#interpretation .

etcimon · 2022-12-10T23:05:39Z

Are there instructions on how to do this?

maxhaton · 2022-12-11T01:10:23Z

I think the existing CFA code might be able to recognize the CTFE patterns

It could, but only if the code initially goes through CTFE and evaluates the condition that contains __ctfe. You can't reliably predict what would happen in that if statement's condition at runtime, it could have function calls that invert the value of __ctfe, so, much of the stuff not allowed in betterC (that you want in CTFE) can be cheated if the expression is complex and improperly "guessed".

What guessing?

maxhaton · 2022-12-11T01:11:51Z

The same code to recognize noreturn and so on should work for assert(CTFE) adding an entirely new identifier seems really extreme and also useless for some existing code.

etcimon · 2022-12-11T05:17:03Z

The same code to recognize noreturn and so on should work for assert(CTFE)

You mean assert(__ctfe) ? It could definitely trigger static compilation of the rest of the scope but I feel like that could make it less obvious that nothing can escape the scope. The static functions declared inside are not meant to be registered into the binary because the machinery that would allow Range, GC and so on is likely to interfere and that would make it a nightmare to figure out how to implement.

Also, it would break any code that used assert(__ctfe) without expecting this new behavior

PetarKirov · 2023-01-08T17:07:55Z

@WalterBright you may want to take a look, as you were working on similar BetterC issues.

etcimon · 2023-01-17T02:35:05Z

I also made the changes in LDC

etcimon/ldc@3dcd9f9

Now I can use Diet Templates with a Webassembly project, which means they run in the client browser. The output HTML is correctly generated. I just need to setup an algorithm to selectively merge the HTML with the (spasm) structs and I can write most of a website using diet templates and D (without a GC) instead of React/Angular.

https://github.com/etcimon/libwasm/tree/master/diet-ng


    import diet.html : compileHTMLDietString;
    Vector!char output;
    string ret = compileHTMLDietString!(`doctype html
- auto title = "Hello, <World>";
html
	head
		title #{title} - example page
	body
		h1= title
		h2 Index
		ol.pageindex
			- foreach (i; 0 .. 3)
				li: a(href="##{i}") Point #{i}
		- foreach (i; 0 .. 3)
			h2(id=i) Point #{i}
			p.
				These are the #[em contents] of point #{i}. Multiple
				lines of text are contained in this paragraph.`)(output);
        console.log(format!"Output length is %d"(output.length)[]);
      console.log(output[]);
      console.log(ret);

etcimon · 2023-02-24T22:24:24Z

I rebased this in the most recent commit. However, it seems like everyone wants to sweep this under the rug. Is this going to require a rebase every month as it gets burried in pull hell? Should I just close it and abandon ctfe in betterC?

mdparker · 2023-02-25T00:34:11Z

@etcimon Let's give it a bit more time, please. I'll make sure it's on Walter's radar so he can look it over

WalterBright · 2023-02-25T01:56:41Z

@etcimon can you please change the commit message to:

Fix Issue 18472: betterC cannot use format at compile time

this is so the dlang bot can successfully associate it with the bug report. The "fix Issue NNNN" is the trigger.

Is the test case in the PR the one you want to resolve? Is it a boiled down case from the bugzilla issue?

yes, please rebase!

WalterBright · 2023-02-25T06:54:34Z

I started work on a fix:

#14913

etcimon · 2023-02-25T07:29:18Z

Yes, it boils down to this. There are a lot of implementations of format but you need to be able to allocate an array on the GC. I made it work with diet templates in my fix as you can see with my comment, which builds an html string builder from D code at compile-time in betterC and displays it in the browser console entirely from webassembly

etcimon · 2023-02-25T07:33:37Z

However, I had a bug in my library that I couldn't replicate with a test yet, which is portrayed with skipretnogc. Basically, the expression that does new in a function is run in checkGC as the return type, BUT the scope of the if block is lost.

WalterBright · 2023-02-25T08:23:55Z

@etcimon I think I have a handle on a good fix for this, and we'll see if we can get it merged with some alacrity. Thanks for your patience on this issue.

etcimon · 2023-02-25T17:24:40Z

Thanks @WalterBright, I didn't expect you to personally pick this up. There's other pieces that would be missing to make the standard format work, the general rule of thumb that I've learned to respect with CTFE in betterC is that your function has to compile with betterC before it can be used in CTFE, so for the standard library the other problem is that isOutputRange!(char[]) returns false in betterC. There's no way to know this is for CTFE During compile-time. Also, even when I bypassed this I had linker errors, because CTFE will cause any called function to be included in the object as if they were called in regular code. At least that's my take on it. But none of that is an issue if you use third party libraries that are particularly built to be compatible like I did.

dkorpel · 2023-02-25T17:56:58Z

However, it seems like everyone wants to sweep this under the rug.

I'm sorry for neglecting the PR. I didn't consider it ready because of two outstanding issues:

Unsoundness (Allows creation of GC-using @nogc functions: Fix issue 18472: betterC cannot use format at compile time. #14676 (review))
Adding new syntax / special patterns to mark CTFE-only functions needs approval

Because of the second point, I brought the issue up in the Quarterly D Language Foundation Meeting last January. Mike hasn't posted a summary in the Announce Forum yet, but the summary of my part is:

Dennis brought up the future direction of -betterC. Various people have raised issues about -betterC disabling druntime-dependent features even for CTFE, and while there have been some small fixes, there's no consensus yet on how to solve it in general. Dennis brought up three approaches:

Explicitly annotate code as CTFE-only with new syntax: pragma(ctfe), if (ctfe) etc. Walter noted that the syntax is an extra () (i.e. use template functions).

Implicitly make functions using druntime features as CTFE-only. This might be surprising and unintuitive

Generate runtime errors instead of compile time errors. This makes errors easier to slip by.

Martin Kinke brought up a fourth option: phase out -betterC, because it's a 'pile of hacks'.
Dennis considered that, but thought betterC users would not be happy when it gets deprecated without a suitable replacement.
Walter said that the best approach couldn't be decided in the meeting, and should be discussed in an e-mail / forum post.
As a final question, Dennis asked what the 'official' intended use for betterC is in the first place: just a C migration tool or also something for new D code?
Mike thinks -betterC shouldn't be used for writing new code. Walter said it can be used for whatever calls for it, be it integrating with C, targeting embedded systems, or any scenario where you don't want to link druntime.

Since then, Walter Bright made some incremental improvements:
#14789
#14791
#14819
#14830

Currently I'm not sure what exactly needs to be done still to make std.format work with -betterC, but some parts of this PR seem to be redundant now.

In any case, I should have communicated this to you.

etcimon · 2023-02-25T18:22:18Z

Deprecating -betterC would definitely kill any potential of webassembly in D altogether. There's absolutely no way anyone would agree to embed the browser API in druntime, but what do I know? The current front runner for webassembly is Rust with this library:

https://github.com/yewstack/yew

I think D could do a better job, but if betterC is deprecated I might as well consider D a too high-level language altogether that tries to compete with python, and move on.

dkorpel · 2023-02-25T18:38:41Z

Deprecating -betterC would definitely kill any potential of webassembly in D altogether.

There's no plan to do that without offering an alternative that covers the same grounds. Note that -betterC is not the only way to compile D without druntime dependencies:

http://dpldocs.info/this-week-in-d/Blog.Posted_2020_07_27.html
http://dpldocs.info/this-week-in-d/Blog.Posted_2020_08_10.html

Ideally, druntime becomes more lightweight and pay as you go, meaning it doesn't drag in any features your code doesn't use. However, there's a long way to go.

etcimon · 2023-02-25T20:52:55Z

Those articles are interesting, it would definitely be nice to be able to have custom exceptions without pulling in druntime, it seems like you can write code that doesn't pull in druntime if you're careful. Obviously, a good addition to this would be a compiler flag that gives you errors you when you do, you know, just like betterC currently does. There's nothing wrong with adding features to betterC though, just like we're trying to do here.

maxhaton · 2023-02-25T21:37:45Z

The issue with betterC isn't that people use it for weird targets but rather that people use it to try and be efficient (i.e. where druntime works fine) which then completely balkanizes the language in a way that isn't introspectable inside the language and also causes lots of linker errors if it sneaks into a dependency graph. DRuntime can and is being made pay-as-you-go, eventually it will just emit (if no hooks are used) what's needed for the runtime to initialize the module correctly, And even that should be removed by the linker if unused.

…

On Sat, 25 Feb 2023, 18:22 Etienne Cimon, ***@***.***> wrote: Deprecating -betterC would definitely kill any potential of webassembly in D altogether. There's absolutely no way anyone would agree to embed the browser API in druntime, but what do I know? The current top runner for webassembly is Rust with this library: https://github.com/yewstack/yew I think D could do a better job, but if betterC is deprecated I might as well consider D a too high-level language altogether that tries to compete with python, and move on. — Reply to this email directly, view it on GitHub <#14676 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABLI75G5222B6BKWMBVWUT3WZJEWNANCNFSM6AAAAAASXHDS6Q> . You are receiving this because you commented.Message ID: ***@***.***>

etcimon · 2023-02-25T21:44:54Z

If people are using betterC to build their library, I don't believe that should even be worth mention when developing the language because by definition they declared independence on it and are totally fine with being ignored. The articles above discusses the alternative to betterC which is declaring a custom object module, that should be much worse because it overrides druntime in a way that you can't even add the library as a dependency.

etcimon · 2023-02-25T22:26:32Z

It seems like the main concerns about betterC could be addressed by putting them in a separate cluster in the dub registry. On my end, I don't think I could develop this web framework without having a compiler that helps users avoid druntime.

etcimon force-pushed the fix_18472 branch 6 times, most recently from fb14d42 to 36f3353 Compare December 7, 2022 23:55

maxhaton reviewed Dec 8, 2022

View reviewed changes

dkorpel reviewed Dec 8, 2022

View reviewed changes

dlang-bot added the Severity:Bug Fix label Dec 9, 2022

etcimon force-pushed the fix_18472 branch from b3cae7e to 5de3908 Compare December 9, 2022 01:42

etcimon requested a review from ibuclaw as a code owner December 9, 2022 01:42

ljmf00 added the Review:Needs Spec PR A PR updating the language specification needs to be submitted to dlang.org label Dec 10, 2022

PetarKirov requested a review from WalterBright January 8, 2023 17:07

dlang-bot added the Review:Needs Rebase label Jan 10, 2023

dlang-bot removed the Review:Needs Rebase label Feb 23, 2023

etcimon force-pushed the fix_18472 branch from 6ce53ef to 0df8a57 Compare February 23, 2023 00:23

Fix 18472: betterC cannot use format at compile time.

6b6a370

etcimon force-pushed the fix_18472 branch from 0df8a57 to 6b6a370 Compare February 23, 2023 01:02

dlang-bot added Review:Needs Work Review:Needs Rebase labels Feb 23, 2023

etcimon mentioned this pull request Feb 24, 2023

mlock return -1 etcimon/memutils#2

Open

etcimon changed the title ~~Fix 18472: betterC cannot use format at compile time.~~ Fix issue 18472: betterC cannot use format at compile time. Feb 25, 2023

dlang-bot added the Review:stalled label May 27, 2023

dlang-bot added Merge:needs rebase stalled labels Dec 2, 2024

		@@ -0,0 +1,37 @@
		/*******************************************/

Fix issue 18472: betterC cannot use format at compile time. #14676

Are you sure you want to change the base?

Fix issue 18472: betterC cannot use format at compile time. #14676

Conversation

etcimon commented Dec 7, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

etcimon Dec 8, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dkorpel left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

etcimon Dec 8, 2022 • edited Loading

Choose a reason for hiding this comment

etcimon commented Dec 8, 2022 • edited Loading

etcimon commented Dec 8, 2022 • edited Loading

etcimon commented Dec 8, 2022

etcimon commented Dec 8, 2022

dkorpel commented Dec 8, 2022 • edited Loading

maxhaton commented Dec 8, 2022

etcimon commented Dec 8, 2022

etcimon commented Dec 8, 2022

dlang-bot commented Dec 9, 2022 • edited Loading

Bugzilla references

Testing this PR locally

etcimon commented Dec 9, 2022 • edited Loading

etcimon commented Dec 10, 2022 • edited Loading

ljmf00 commented Dec 10, 2022

etcimon commented Dec 10, 2022

maxhaton commented Dec 11, 2022

maxhaton commented Dec 11, 2022

etcimon commented Dec 11, 2022 • edited Loading

PetarKirov commented Jan 8, 2023

etcimon commented Jan 17, 2023

etcimon commented Feb 24, 2023

mdparker commented Feb 25, 2023

WalterBright commented Feb 25, 2023

WalterBright commented Feb 25, 2023

etcimon commented Feb 25, 2023 • edited Loading

etcimon commented Feb 25, 2023 • edited Loading

WalterBright commented Feb 25, 2023

etcimon commented Feb 25, 2023 • edited Loading

dkorpel commented Feb 25, 2023

etcimon commented Feb 25, 2023 • edited Loading

dkorpel commented Feb 25, 2023

etcimon commented Feb 25, 2023

maxhaton commented Feb 25, 2023 via email

etcimon commented Feb 25, 2023

etcimon commented Feb 25, 2023

etcimon commented Dec 7, 2022 •

edited

Loading

etcimon Dec 8, 2022 •

edited

Loading

etcimon Dec 8, 2022 •

edited

Loading

etcimon commented Dec 8, 2022 •

edited

Loading

etcimon commented Dec 8, 2022 •

edited

Loading

dkorpel commented Dec 8, 2022 •

edited

Loading

dlang-bot commented Dec 9, 2022 •

edited

Loading

etcimon commented Dec 9, 2022 •

edited

Loading

etcimon commented Dec 10, 2022 •

edited

Loading

etcimon commented Dec 11, 2022 •

edited

Loading

etcimon commented Feb 25, 2023 •

edited

Loading

etcimon commented Feb 25, 2023 •

edited

Loading

etcimon commented Feb 25, 2023 •

edited

Loading

etcimon commented Feb 25, 2023 •

edited

Loading