druntime: replaces top level version(..) branching by tags

[denizzzka]

Squashed version of #15822

Forum: https://forum.dlang.org/post/[email protected]

[dlang-bot]

Thanks for your pull request and interest in making D better, @denizzzka! 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

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#15887"

[dkorpel]

During the Dec 8 DLF monthly meeting, Walter was in favor of putting different OS'es (Windows, Linux, FreeBSD, macOS) in different folders.

[denizzzka]

@dkorpel Is there a transcript or record somewhere?

[dkorpel]

Mike posts summaries in the Announce forum, though it has a bit of delay. The latest one is: D Language Foundation October 2023 Quarterly Meeting Summary

[dukc]

Ok @denizzzka, time for the comments you requested at the forums.

I'm somewhat disappointed that this needs a script. On the other hand, a src/core/sync/event.d that just read like version (Posix) public import coreconfig.posix.core.sync; wouldn't be ideal either since it would require updating the public imports each time platforms are added, while this mechanism is smarter.

It would probably have been better to write the file copying script in D since that's what this repo usually does when the logic is about more than just flag setting. We require a host D compiler with a standard library anyway because we have compiler/src/build.d. But asking you to rewrite it before accepting this would probably be more than it's worth so I don't.

But otherwise, I very much like this PR! I think it can also be a big help in the very thing I complained about, as we're getting rid of the version() else static assert(0, "unsupported") thing along with version statements in general, meaning stubbing stuff is becoming easier than it was. It will be much easier than before to just copy files from the config/ directory and replace them with stubs when programming for platform that doesn't yet have DRuntime support.

Also I'm happy to hear Walter is interested in supporting this, or at least somthing along these lines. I recommend merging this.

[denizzzka]

I'm somewhat disappointed that this needs a script.

Seems, there's nothing can do about it: something has to take over version(..) functionality. Script or maybe something like parse_tagged_hier.d binary is necessary.

But Makefile scripts changes will be simplified in a short time because #15894 removes win*.mak and mak/WINDOWS

On the other hand, a src/core/sync/event.d that just read like version (Posix) public import coreconfig.posix.core.sync; wouldn't be ideal either since it would require updating the public imports each time platforms are added, while this mechanism is smarter.

It would probably have been better to write the file copying script in D since that's what this repo usually does when the logic is about more than just flag setting.

Windows copying script line will be removed in a short time (see above)

Copying which is used by GNU Make is implemented not in this PR and already used for years for import/ dir creation

We require a host D compiler with a standard library anyway because we have compiler/src/build.d. But asking you to rewrite it before accepting this would probably be more than it's worth so I don't.

But otherwise, I very much like this PR! I think it can also be a big help in the very thing I complained about, as we're getting rid of the version() else static assert(0, "unsupported") thing along with version statements in general, meaning stubbing stuff is becoming easier than it was. It will be much easier than before to just copy files from the config/ directory and replace them with stubs when programming for platform that doesn't yet have DRuntime support.

I completely agree!

Also I'm happy to hear Walter is interested in supporting this, or at least somthing along these lines. I recommend merging this.

Thank you for your feedback!

[ibuclaw]

During the Dec 8 DLF monthly meeting, Walter was in favor of putting different OS'es (Windows, Linux, FreeBSD, macOS) in different folders.

To clarify, the idea is generally favourable. The means requires strict approval of both @kinke and @ibuclaw.

One attempt at splitting these bindings into different architectures ~10 years ago sank - that there's too many potential to support didn't help. Making the OS the dividing line for the module system may be more palatable. It will have to be tested.

But otherwise, I very much like this PR! I think it can also be a big help in the very thing I complained about, as we're getting rid of the version() else static assert(0, "unsupported") thing along with version statements in general, meaning stubbing stuff is becoming easier than it was.

I don't know who "we" is and why they want to remove static assert(false);. That is not the current consensus as far as I'm aware.

[dukc]

During the Dec 8 DLF monthly meeting, Walter was in favor of putting different OS'es (Windows, Linux, FreeBSD, macOS) in different folders.

To clarify, the idea is generally favourable. The means requires strict approval of both @kinke and @ibuclaw.

Of course.

I don't know who "we" is and why they want to remove static assert(false);. That is not the current consensus as far as I'm aware.

With "we" I meant everyone involved. I could have also said "you" as in regular DMD contributors. In either case you're definitely included :-).

As for the "why", it's what this PR does. Since versioning is done by picking the correct D files from config/ subdirectories, it replaces the version statements DRuntime currently uses for that and along with them, the unsupported platform static assertions at their else clauses. I can't know if that's what you want to do, but that's what happens if you do the versioning like it's done here.

[dukc]

Seems, there's nothing can do about it: something has to take over version(..) functionality.

Yes, didn't mean that as criticism. Just that I'd prefer if we could use the module system in the language to do this as well as the script. Since that doesn't seem possible though, at least not without some ugly tricks, the script is justified IMO.

[RazvanN7]

To clarify, the idea is generally favourable. The means requires strict approval of both @kinke and @ibuclaw.

@kinke @ibuclaw where do we stand with this?

[denizzzka]

Important notice: this PR implies that import/ content will be different for each tag set

Because of this, for example, we must provide something like /usr/include/dmd/druntime/windows/import if we want to be able to cross-compile binaries for Windows on Linux.

This is same approach as different Linux kernel headers packages coresponding to a different kernel binaries, but derrived from one kernel source tree

(Currently, same import/ is used for all builds because all branching implemented inside of import/*.d files)

(Discussion https://forum.dlang.org/post/[email protected])

[kinke]

The cross-compilation use cases kill this approach IMO. LDC is an implicit cross-compiler, and its bundled druntime+Phobos source tree has been working fine for targeting all supported targets. Having to include separate trees is IMO out of the question.

Then LDC doesn't use the upstream Makefile, but CMake. We don't have an import tree; for the install build target, all *.d[i] files are copied over from druntime and Phobos, except for the rt druntime package IIRC. I'm also no fan of having to tell the build system which druntime modules to compile for a specific target; that's why I've slapped a bunch of version(Posix/Windows): at the top of some druntime modules, so that they can be compiled and imported on every platform (just being empty on unsupported targets).

How about splitting up the implementations in something like core.internal.sync.event.{windows,posix} and publicly importing the proper module in core.sync.event?

[denizzzka]

How about splitting up the implementations in something like core.internal.sync.event.{windows,posix} and publicly importing the proper module in core.sync.event?

Will not work for a range of targets for which core.internal.sync.event.{target} isn't implemented in main source tree. For embedded it isn't possible to define all targets, because each set of implementions of any needed primitive is a separate implementation in such D terms. So, it leads to combinatory explosion of implementations matrix.

[denizzzka]

The cross-compilation use cases kill this approach IMO.

(Cross-compilation also was main reason to suggest this PR)

Then LDC doesn't use the upstream Makefile, but CMake. We don't have an import tree; for the install build target, all *.d[i] files are copied over from druntime and Phobos, except for the rt druntime package IIRC.

This is ~same approach as works current dmd/druntime Makefile. Except that they use predefined list of modules what needed to be copied into import/. This list contains almost all druntime source files.

I'm also no fan of having to tell the build system which druntime modules to compile for a specific target;

At first sight this isn't looks as big problem: ldc2.conf syntax already allows to define separate paths to imports/ for all supported targets

[kinke]

Will not work for a range of tragets for which core.internal.sync.event.{target} isn't implemented in main source tree. For embedded it isn't possible to define all targets, because each set of implementions of any needed primitive is a separate implementation in such D terms. So, it leads to combinatory explosion of implementations matrix.

Adding an extra layer is as simple as adding a -I dir containing a custom, non-upstreamed core/sync/event.d module for one's favourite platform. For LDC, the -I path for the druntime/Phobos imports is a post-switches entry, meaning that all explicit -I in the compiler cmdline have a higher priority.

[denizzzka]

Adding an extra layer is as simple as adding a -I dir containing a custom, non-upstreamed core/sync/event.d module for one's favourite platform

-I is only for imports. In other words, we still end up with the situation where compiler is forced to switch paths to sources

And also we want (as enhancement) to be able replace modules not as individual elements or the entire "platform implementations", but as sets of elements (by groups of modules belonging to a same tags) - this functionality is also implemented in the script from this PR and therefore needs to support from druntime build facility

[denizzzka]

@kinke

Then LDC doesn't use the upstream Makefile, but CMake. We don't have an import tree; for the install build target, all *.d[i] files are copied over from druntime and Phobos, except for the rt druntime package IIRC.

This is how tags support can be implemented for CMake:
https://github.com/opendlang/opend/pull/24/files#diff-d55d21ac9c6461b7058d75699e8733f4a93b60d70bd216138b5dae2612715066

[denizzzka]

There is another option that solves only problem of external implementations: implement this PR but only with tags like officially_supported_threading, officially_supported_os, officially_supported_libc, etc. In other words, a tags will work but the official resulting code will be same for each target.

In this case cross-linking routine will not changes and "versions hell" will not be resolved. But we will be able to create external implementations of needed functions. (And in this case we still need to move, for example, Event unittests into a different module, as in this PR. And also maybe introduce new versions/enums like Posix_Threads)

In the future, if the community decides to reorganise all druntime code with a such tag system, this can be easily done. But for now propose to accept tags as a standard mechanism for switching of external implementations

[denizzzka]

In a short time I’ll move Event methods into a separate event_impl.d and revert tests back to event.d

[denizzzka]

In a short time I’ll move Event methods into a separate event_impl.d and revert tests back to event.d

Done

I think that it is better to maintain this specific module by this way

Also, as side effect, becomes obvious pointlessness of the handler checks before each Event method calling

[ryuukk]

This is over engineered.. why a cryptic bash script to split files???

It should be as simple as this:

core/sync/event.d
core/sync/event_posix.d
core/sync/event_windows.d
core/sync/event_darwin.d

grep-able and easy to maintain

Anything else is just bad

[denizzzka]

@ryuukk

This is over engineered..

You mean script or whole idea?

why a cryptic bash script to split files???

I hesitated to write on D because I was afraid of a potential chicken and egg problem. Bash script works on all our official platforms out of box. And script is a quite simple.

On CMake same functionality can be implemented too, but it is need to provide same ability for GNU Make, so I decided to write one script and use it everywhere.

It should be as simple as this:

core/sync/event.d
core/sync/event_posix.d
core/sync/event_windows.d
core/sync/event_darwin.d

I'm not sure that I understand correctly what you mean here

[denizzzka]

Anything else is just bad

This is a necessary evil: Unfortunately, currently druntime isn't same as C rtl/crt. In these C terms druntime is a huge rtl + libc. Therefore we are forced to move from current versions to something like (in C terms) different include dirs for a different libc implementations

And without that mechanism it will be impossible to explain to the community why some specific code suddenly needs to be allocated into a separate module (so that it can be replaced in some third party code) - the answer always will be: it is convenient for us as it is for now. Therefore, it is necessary to create a switching mechanism that will also be used by the official platforms. (By "official platforms" I mean druntime binaries that come with distributions from dlang.org or from packages maintainers)

In addition, we already using a similar switching mechanism for core.sys.* (in ldc2 cmake scripts) and (my opinion) we need same approach for core.stdc.* - just a huge unmaintainable files (as for me, of course). So, cmake script can be replaced by this script with same result but for all compilers.

Regular users won't notice anything. Only /etc/ldc.conf will got 8 lines like:

// Imports for officially supplied druntime binaries:

".+-windows-.+":    { post-switches = [ "-I/tmp/include/d", "-I/tmp/include/d/platforms/windows" ]; };
".+-osx-.+":        { post-switches = [ "-I/tmp/include/d", "-I/tmp/include/d/platforms/osx" ]; };
".+-dragonfly-.+":  { post-switches = [ "-I/tmp/include/d", "-I/tmp/include/d/platforms/dragonfly" ]; };
".+-freebsd-.+":    { post-switches = [ "-I/tmp/include/d", "-I/tmp/include/d/platforms/freebsd" ]; };
".+-linux-.+":      { post-switches = [ "-I/tmp/include/d", "-I/tmp/include/d/platforms/linux" ]; };
".+-netbsd-.+":     { post-switches = [ "-I/tmp/include/d", "-I/tmp/include/d/platforms/netbsd" ]; };
".+-openbsd-.+":    { post-switches = [ "-I/tmp/include/d", "-I/tmp/include/d/platforms/openbsd" ]; };
".+-sunos-.+":      { post-switches = [ "-I/tmp/include/d", "-I/tmp/include/d/platforms/sunos" ]; };

Thats all!

(The fact that such lines aren't added for now (with the all these necessary currently --d-version=CRuntime_Glibc, etc) - is an issue that we do not notice)

[denizzzka]

similar switching mechanism for core.sys.* (in ldc2 cmake scripts)

It will be difficult to change it now. If at the time of core.sys.* creation there was a proposed mechanism then it would be great

[ibuclaw]

For the sake of discussing all currently used mechanisms, did you consider breaking up druntime into features rather than versions?

https://github.com/gcc-mirror/gcc/blob/master/libphobos/libdruntime/gcc/config.d.in

This file is generated at configure-time for gdc after having tested what the target system has available.

Obviously static if and version doesn't mix, but if you use just one consistently it should be fine - assuming that you don't do strange things like the following: https://issues.dlang.org/show_bug.cgi?id=17883

[denizzzka]

For the sake of discussing all currently used mechanisms, did you consider breaking up druntime into features rather than versions?

https://github.com/gcc-mirror/gcc/blob/master/libphobos/libdruntime/gcc/config.d.in

Actually, I exactly suggest to breaking it down into features (named "tags"). But proposed "features" can be provided also from outside of the official sources (and this is for free)

In the above example, we do not have abilities to replace atomics with stubs (which is common for single-core platforms). Yes, maybe this will be solved by the replacement of a symbols during linking, but: there are dozens of such places in druntime, and are all a little different: somewhere it is need to replace class, somewhere need to replace bunch of enums, etc. Also, shove all druntime code into something like:

version (CRuntime_Glibc)
{
    version = Any_Have_Tgmath;
    version = Any_Have_TgMath_Mod;
    version = Any_Have_TgMath_NaN;
}
else version (NetBSD)
{
    version = Any_Have_TgMath;
    version = Any_Have_TgMath_Mod
}
else version (OpenBSD)
{
    version = Any_Have_TgMath;
}

? In my opinion, this which will only complicate possible solution in the future. And it is not a cheap, it's a thousands lines, I think.

Proposed in this PR approach allows us to achieve results "just right now". It will give us time to inventing some way to create static compilation branches based on standard language features. If successful, we will simply delete script, and even the ldc2 configuration file will continue to work (because it introduces only additional -I switches which compiller ignores if path is unavailable)

In the future, if a graceful solution will be found (without a script, etc), it will be possible to switch to it easily: until such a solution has been found, modules will move to a state more convenient for such hypotetical static branching due to the use of the proposed approach

This file is generated at configure-time for gdc after having tested what the target system has available.

So we found out that GDC also uses its own mechanism to perform near to the same task

[denizzzka]

For the sake of discussing all currently used mechanisms, did you consider breaking up druntime into features rather than versions?
https://github.com/gcc-mirror/gcc/blob/master/libphobos/libdruntime/gcc/config.d.in

Actually, I exactly suggest to breaking it down into features (named "tags"). But proposed "features" can be provided also from outside of the official sources (and this is for free)

This file is generated at configure-time for gdc after having tested what the target system has available.

So we found out that GDC also uses its own mechanism to perform near to the same task

Let me make it more clear, that's what I mean (this is my current cmake script for ldc2): https://github.com/opendlang/opend/blob/fda4c0c698137a064b4cbef6255b7f90518f09db/ldc/runtime/CMakeLists.txt#L282

# Official targets
set(OfficialPlatforms
    "windows" "osx" "dragonfly" "freebsd"
    "linux" "netbsd" "openbsd" "sunos"
)

# Tags matrix for them
set(TAGS_windows    "windows")
set(TAGS_osx        "posix_threads")
set(TAGS_dragonfly  "posix_threads")
set(TAGS_freebsd    "posix_threads")
set(TAGS_linux      "posix_threads")
set(TAGS_netbsd     "posix_threads")
set(TAGS_openbsd    "posix_threads")
set(TAGS_sunos      "posix_threads")

For each line of this matrix "official targets" imports is generated and then can be placed into a bundle with druntime binary. "windows" and "posix_threads" is a such "features". This works ~same as your sample from GDC, but for all targets (so as not to cause discontent ldc users who are used to cross-compile) and this format of a tags is universal between all compilers, applied to the entire druntime and usually does not require heavily changing *.d sources

[ibuclaw]

For the sake of discussing all currently used mechanisms, did you consider breaking up druntime into features rather than versions?
https://github.com/gcc-mirror/gcc/blob/master/libphobos/libdruntime/gcc/config.d.in

Actually, I exactly suggest to breaking it down into features (named "tags"). But proposed "features" can be provided also from outside of the official sources (and this is for free)

In the above example, we do not have abilities to replace atomics with stubs (which is common for single-core platforms). Yes, maybe this will be solved by the replacement of a symbols during linking, but: there are dozens of such places in druntime, and are all a little different: somewhere it is need to replace class, somewhere need to replace bunch of enums, etc. Also, shove all druntime code into something like:

I don't know where atomics came from - if you're referring specifically to the three enums in gcc/config.d, the libatomic library is there to do the right thing for you, so no need to do anything special on the druntime side.

? In my opinion, this which will only complicate possible solution in the future. And it is not a cheap, it's a thousands lines, I think.

Also, it's subtly incorrect to hard code it like that. It assumes that these things won't change from one version of a platform to the next.

This file is generated at configure-time for gdc after having tested what the target system has available.

So we found out that GDC also uses its own mechanism to perform near to the same task

Not really. GDC is doing something completely different to here. The config module is a guide for the implementation of the core run-time library, rather than the public C library bindings.

[ibuclaw]

Let me make it more clear, that's what I mean (this is my current cmake script for ldc2): https://github.com/opendlang/opend/blob/fda4c0c698137a064b4cbef6255b7f90518f09db/ldc/runtime/CMakeLists.txt#L282

[snip]

For each line of this matrix "official targets" imports is generated and then can be placed into a bundle with druntime binary. "windows" and "posix_threads" is a such "features". This works ~same as your sample from GDC, but for all targets (so as not to cause discontent ldc users who are used to cross-compile) and this format of a tags is universal between all compilers, applied to the entire druntime and usually does not require heavily changing *.d sources

That matrix makes some bold assumptions. You can't be that assertive about will be available from one version of a platform to the next, or platform-x86 has the same feature as platform-arm.

To pick out one example from my reference.

// Whether struct dl_phdr_info has dlpi_tls_modid member.
enum OS_Have_Dlpi_Tls_Modid = @DCFG_DLPI_TLS_MODID@;

This is false on Solaris 11.4, but true from Solaris 11.5 onwards, checking for the dlpi_tls_modid field is determined by a simple AC_CHECK_MEMBER "compile" test.

FreeBSD and MacOS are two similar moving targets with their APIs. e.g (1): someone decided it would be a good idea to call __fork() in the GC when compiling for OSX, ignoring that this symbol doesn't exist for 10.8 and earlier. e.g (2): FreeBSD completely rewrote stat_t between version 11 and version 12.

[ibuclaw]

More likely than not, ImportC will kill off the C run-time bindings as they currently are implemented in druntime.

https://github.com/ibuclaw/dmpdecimal/blob/main/dmpdecimal.d#L29-L37

You can even check for symbols that might be missing in the C headers, and selectively expose them.

https://github.com/ibuclaw/dmpdecimal/blob/main/dmpdecimal.d#L122-L127

[denizzzka]

I don't know where atomics came from - if you're referring specifically to the three enums in gcc/config.d, the libatomic library is there to do the right thing for you, so no need to do anything special on the druntime side.

Yes, I saw a reference to atomics library and remembered that I had to cover everything related to atomics with a stub that does nothing im my STM32 druntime binding (external libatomic for this platform simply does not exist)

Not really. GDC is doing something completely different to here. The config module is a guide for the implementation of the core run-time library, rather than the public C library bindings.

Ok

By the way in the proposed "tag scheme" I do not distinguish between published by druntime bindings and internal implementations - any module can be switched using this mechanism (just move it to config/ and add the path to it to the file TAGGED_COPY if it should be represented in import/ dir)

[denizzzka]

That matrix makes some bold assumptions. You can't be that assertive about will be available from one version of a platform to the next, or platform-x86 has the same feature as platform-arm.

I am absolutely sure here: it is from these sources and imports corresponding druntime will be builded and tested for all these platforms

To pick out one example from my reference.

Discussed matrix intended only to ldc2 with its own unique ldc2.conf cross-compilation sections

This is false on Solaris 11.4, but true from Solaris 11.5 onwards, checking for the dlpi_tls_modid field is determined by a simple AC_CHECK_MEMBER "compile" test.

Yes, such things will have to be checked in the same way during the compilation process. Or we can create two platforms with support for both versions - surely they are providing slightly different API for the end users too

[denizzzka]

@ibuclaw

More likely than not, ImportC will kill off the C run-time bindings as they currently are implemented in druntime.

Here is everything lives forever in the name of backward compatibility :-)

Well, that is, I think core.stdc.* files will stay with us for the next ~5 years

[denizzzka]

@ibuclaw Okay, let's also see to the patch for the druntime which I want to away by this PR: https://gist.github.com/denizzzka/e8325b75af3cfd1a942a7306f82f33f4

This is current minimal hackish patch to support a third-party targets. 90% of its lines is a like:

+version (DruntimeAbstractRt)
+{
+    public import external.rt.dmain : _d_cmain;
+}
+else

(My own main() because before D code starts it is need to prepare environment. If it returns it is need be able also to revert environment back. Official rt.dmain already contains internal version branching but it is impossible to replace its code by external.)

+else version (CRuntime_Abstract)
+{
+    public import external.libc.stddef : wchar_t;
+}

(Picolibc currently is used - it isn't available in the official druntime sources. But remember what official core.libc.* already contain internal version branching for switching libc.)

+version (DruntimeAbstractRt)
+    public import external.core.mutex;
+else

(Whole core.sync.* end user API is need to be implemented from scratch. Same, core.sync.* is already contain version branching for all of its methods, but it is impossible to replace its code by external code.)

And everything like that

How this can be solved by an another graceful way?

[denizzzka]

@kinke

Adding an extra layer is as simple as adding a -I dir containing a custom

Yes, I thought well and - you were right! There is really is no exactly need to produce identical files, many -I directives is what is need and it’s as easy to implement as copying files.

(But I won't do anything for now - let community make a positive decision about whole approach first)

[denizzzka]

Implemented in https://github.com/denizzzka/dfruntime