Pull OutBuffer into dmd.common and use it from both frontend and backend

[andralex]

This eliminates the duplication between OutBuffer (frontend) and Outbuffer (backend), which is exacerbated by the addition of memory-mapped files.

Going forward modules in dmd.common can be used across both frontend and backend.

[dlang-bot]

Thanks for your pull request, @andralex!

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 run digger -- build "master + dmd#12832"

[andralex]

I'll add commits in such a way that the initial commit contains all the drudgery. The others can be reviewed against the first so the differences are easily visible. Example.

[thewilsonator]

Nice! I would have expected a large section of red in the backend removing the backend's out buffer, although this is WIP so I understand.

[thewilsonator]

(resolved merge conflict)

[UplinkCoder]

Isn't root already common to dmd and the backend?

…

On Thu, Jul 8, 2021, 4:47 AM Nicholas Wilson ***@***.***> wrote: (resolved merge conflict) — You are receiving this because your review was requested. Reply to this email directly, view it on GitHub <#12832 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAVWSCBLCMGVOGMV2VD3SI3TWUGVVANCNFSM477MVX7Q> .

[thewilsonator]

/home/runner/work/dmd/dmd/dmd/src/tests/cxxfrontend.c:20:10: fatal error: root/outbuffer.h: No such file or directory
   20 | #include "root/outbuffer.h"

[andralex]

Isn't root already common to dmd and the backend?

Per @WalterBright there's no dependency of the backend on dmd.root. I should have verified:

$ git grep 'dmd\.root'
aarray.d:21:    import dmd.root.hash;
bcomplex.d:12:public import dmd.root.longdouble : targ_ldouble = longdouble;
bcomplex.d:15:    private import dmd.root.longdouble : fabsl, sqrt; // needed if longdouble is longdouble_soft
cdef.d:259:public import dmd.root.longdouble : targ_ldouble = longdouble;
cg87.d:663:        import dmd.root.longdouble;
elem.d:69:    import dmd.root.longdouble;
evalu8.d:1984:    import dmd.root.longdouble : fabsl; // needed if longdouble is longdouble_soft
fp.d:19:    import dmd.root.longdouble;
$ _

@WalterBright please advise.

[thewilsonator]

Yeah, I ran into those trying to compile DMD on macOS AArch64 (see #12764).

[Geod24]

Shouldn't the backend's outbuffer copy be removed ? Also I expect this will create issues with DMC.
I am more than happy with the direction here though, if it gets accepted by the BDFL.

Although note that root was supposed to be common. I don't see the point of introducing the new package, but if that's what it takes...

Per @WalterBright there's no dependency of the backend on dmd.root.

Are you referencing this discussion ?

To be a little clearer, if the files are all merely reshuffled into various packages, then they all violate the rule:

*** Never import a file from an uplevel directory ***

and understanding is not increased at all. And it isn't even just an uplevel directory, it's up then sideways then down.

This is still a sideway import.

[andralex]

Shouldn't the backend's outbuffer copy be removed ?

Yah, I plan to replace its uses incrementally and then eliminate it. That should make testing smooth (lesson learned with the mmfiles...).

Also I expect this will create issues with DMC.

I'd need to hear more about that.

I am more than happy with the direction here though, if it gets accepted by the BDFL.

Although note that root was supposed to be common. I don't see the point of introducing the new package, but if that's what it takes...

We'll see. It's actually simpler to keep OutBuffer in root and use it from there. However, root is already populated with a bunch of stuff that I assume has no place in the backend, so incrementally building common separately may make sense. We'll see.

Per @WalterBright there's no dependency of the backend on dmd.root.

Are you referencing this discussion ?

To be a little clearer, if the files are all merely reshuffled into various packages, then they all violate the rule:
*** Never import a file from an uplevel directory ***
and understanding is not increased at all. And it isn't even just an uplevel directory, it's up then sideways then down.

This is still a sideway import.

The view "Never import a file from an uplevel directory" or "Never import a file from an uplevel directory and then sideways" comes straight from a "high level modules depend on low level modules and low level modules should not depend on high level modules" viewpoint. The sideways part is "low level modules should not depend on other low level modules".

All of these are fallacious as explained by Robert Martin, so these points will be dismantled in good order.

High level modules and low level modules should depend on abstractions. OutBuffer is an abstraction. Both front end and back end can depend on it appropriately. Putting OutBuffer here or there is potayto-potahto.

As that book from Harvard on negotiation says: don't argue your point. Don't argue against the other person's point. By everything sacred don't argue against the other person's character.

Argue the principle. And you will win.

[WalterBright]

I should have verified

I keep removing them and they keep creeping back in.

[kinke]

The dmd.common.outbuffer module still imports dmd.root.{file,rmem,rootobject,string}, so moving it alone still increases the backend dependencies on dmd.root.

[andralex]

Reached an impasse with integrating Out[Bb]uffer across the backend and the rest of the project. The backend edition of the Outbuffer uses a pointer to buffer as a member, and exposes it as part of the public interface. The backend saves the address of that pointer in TInfoPair, which it subsequently puts in a hashtable AApair. The double pointers are quite nasty but the whole thing works and is used (according to the doc) by the C++ interface.

This makes it impossible to use OutBuffer instead of Outbuffer because the former uses a ubyte[] instead of a pointer, which makes it impossible (or at least nefarious) to tease out the address of the pointer to data. The cast needed would be something like (cast(ubyte**) &a) + 1 and could break spectacularly if we ever change the array layout. I think I'll do this as a bridge but I'm hoping to get fresh ideas on how to avoid it. Thanks.

[WalterBright]

Can't be proud of this :-/

[andralex]

Ideas?

[WalterBright]

Importing windows and posix api's should be gated with version declarations.

[andralex]

Hmmm, actually they're gated inside. So I don't think we should have both. What's best?

[WalterBright]

Most important issues:

1. no null checks on malloc calls
2. use of obsolete A Windows APIs

[WalterBright]

See Phobos for how to convert D strings to Windows W strings.

[andralex]

@WalterBright I made the requested changes and also asked a couple of questions - thanks!