Move _ArrayEq, _ArrayPostblit and _ArrayDtor to druntime

[wilzbach]

This makes debugging DMD with simple source code easier as -betterC + an empty object.d actually don't add anything that hasn't been added by the user.

Out of interest: what's the reason for these three declarations to be in the compiler?
Shouldn't it be part of object.d?
Or is it just an historical artifact?

[JinShil]

See also #7765

Shouldn't it be part of object.d?

I don't see why they can't be moved to object.d, and until I understand otherwise, I would prefer that be done instead of this PR.

is it just an historical artifact?

I don't know, but it sure appears like it is.

[dlang-bot]

Thanks for your pull request, @wilzbach!

Bugzilla references

Your 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 locally

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

dub fetch digger
dub run digger -- build "master + dmd#8067"

[wilzbach]

I wonder whether we can directly replace the calls, e.g. for _ArrayPostblit it should be something like:

diff --git a/src/dmd/dsymbolsem.d b/src/dmd/dsymbolsem.d
index 2db047e9c..aa2ad3a79 100644
--- a/src/dmd/dsymbolsem.d
+++ b/src/dmd/dsymbolsem.d
@@ -151,10 +151,11 @@ private extern (C++) FuncDeclaration buildPostBlit(StructDeclaration sd, Scope*
 
             ex = new DotVarExp(loc, ex, sdv.postblit, false);
             ex = new CallExp(loc, ex);
+            postblitCalls.push(new ExpStatement(loc, ex)); // combine in forward order
         }
         else
         {
-            // _ArrayPostblit((cast(S*)this.v.ptr)[0 .. n])
+            // (cast(S*)this.v.ptr)[0 .. n]
 
             uinteger_t length = 1;
             while (tv.ty == Tsarray)
@@ -179,9 +180,29 @@ private extern (C++) FuncDeclaration buildPostBlit(StructDeclaration sd, Scope*
             // Prevent redundant bounds check
             (cast(SliceExp)ex).upperIsInBounds = true;
             (cast(SliceExp)ex).lowerIsLessThanUpper = true;
-            ex = new CallExp(loc, new IdentifierExp(loc, Id._ArrayPostblit), ex);
+
+            /**
+            foreach (ref e; cast(S*)this.v.ptr)[0 .. n])
+                e.__xpostblit();
+            */
+            Parameters* params = new Parameters();
+            auto elIdent = Identifier.generateId("__e");
+            params.push(new Parameter(STC.ref_ | STC.const_, sdv.type, elIdent, null));
+
+            // foreach body
+            auto identPostblit = new IdentifierExp(loc, Id.__xpostblit);
+            Expression bodyEx = new IdentifierExp(loc, elIdent);
+            bodyEx = new DotExp(loc, bodyEx, identPostblit);
+            bodyEx = new CallExp(loc, bodyEx);
+            Statement body_ = new ExpStatement(loc, bodyEx);
+
+            Statement frs = new ForeachStatement(loc, TOK.foreach_, params, ex, body_, loc);
+            printf("ForeachStatement: %s\n", frs.toChars);
+            postblitCalls.push(frs);
+
         }
-        postblitCalls.push(new ExpStatement(loc, ex)); // combine in forward order
 
         /* https://issues.dlang.org/show_bug.cgi?id=10972
          * When the following field postblit calls fail,

Though that segfaults for compilable/b16355.d.

[JinShil]

@wilzbach Were you able to get this to work locally. I tried pulling down the druntime change and dmd change to test locally, but I can't get it to work.

Update: scratch that, I got it to work. I must have done something wrong previously.

How do we verify this before merging? Do we just merge druntime on faith first?

[JinShil]

I wonder whether we can directly replace the calls, e.g. for _ArrayPostblit

In general, I would prefer if all rewrites/lowerings could be placed in the runtime when possible for the following reasons:

• It's easier to understand and maintain the implementation if it's straight D code, rather than trying to decipher D code that's writing D code.
• It allows the implementation to be optimized or customized for a given target without having to rebuild the compiler.
• It makes the compiler code base just that much simpler.

So, I would prefer trying to find ways to move feature implementations out of the compiler to the runtime rather than the other way around.

[wilzbach]

How do we verify this before merging? Do we just merge druntime on faith first?

Well, we could create a special branch at dmd and druntime, but while some CIs do checkout the respective branch for the repositories, I'm not sure whether all do (IIRC some still don't).
Also this is quite an overhead as we would have to create the branch, rebase the PRs to that branch, merge them, and then merge these branches into master again, so I think local testing is fine for this - especially given that its a tiny change and just adds new symbols which could always easily be reverted if it turns out that something isn't working as expected.

[JinShil]

This is congruent with item 4 in the Vision Statement:

4. Improve interoperability with other languages: Finishing -betterC should improve incremental migration of C and C++.

[wilzbach]

So is everyone okay with moving this to druntime (see dlang/druntime#2147)?
Or should the compiler generate the code of these functions inline like suggested in the diff?

[JinShil]

Ping!

[ibuclaw]

Needs rebasing.

[wilzbach]

Rebased. Though the druntime PR needs to be merged first.

[dnadlinger]

druntime PR has been merged.

[dnadlinger]

Do we actually need to special-case these?