Replace static if checks for OS and IN_GCC

[joakim-noah]

All stdlib tests pass locally for me on linux/x64, now passed auto-tester too.

Update:
I went ahead and made the OS checks runtime variables because they will be used that way whenever dmd is turned into a cross-compiler for different OSs. However, the OMF/COFF/ELF/MACH library generation modules in the frontend will need to be made portable first, so those are versioned out for now.

[yebblies]

Not an improvement.

[joakim-noah]

Why? This is how immutable variables are initialized.

[yebblies]

Only when their values aren't known at runtime.

[joakim-noah]

That's the plan. :) I made these immutable runtime variables rather than enums because whenever dmd is turned into a cross-compiler, this shared static this can be easily modified to parse the cross-compilation command-line options at runtime and then set the target OS.

I should have mentioned that above, but wanted to make sure it passed the auto-tester first, added that now.

[yebblies]

This is not the API we want for cross-compiling. All conditions should eventually be moved into target.

[ibuclaw]

FYI, the same should also be done for global.params.isXXX too. Now you know all our plans for the target module. :-)

[joakim-noah]

My PR has nothing to do with ldc, which simply ignores these enums for its own runtime variables, and I don't really care what the plans are for ddmd.target. I specifically asked how Daniel planned to get rid of TARGET_* without using similar runtime variables to this PR. That is not answered anywhere in your three replies.

[dnadlinger]

Just make them compile-time versions.

[joakim-noah]

Just make them compile-time versions.

I did, in one instance below, because of some non-portable code. He's even more against that.

[ibuclaw]

That is not answered anywhere in your three replies.

It is! In the first step.

The first step is to move all uses of TARGET_XXX into the ddmd.target module.

Once that is done then as far as I'm concerned it is gone from the frontend. Because ddmd.target is not a shared/common module.

[joakim-noah]

Fixed the style issue and the bug Daniel pointed out, can't do anything about the remaining vague comments.

[yebblies]

What I'm trying to convey, is that I doubt any of these changes are going to be usable for the eventual runtime cross-compiling implementation.

For example, the GCC varargs special case needs to either be pushed down to the glue layer, homogenized between frontends, or abstracted into Target. Changing static if to version simply does not do anything to help.

The Library.factory methods need to be fixed to work no matter what host platform, then chosen based on an abstraction in Target (or something). Changing them to be based on host is just wrong! And again it doesn't help.

So it is at best, a negligible improvement to a part of the compiler source we plan to remove.

[joakim-noah]

What I'm trying to convey, is that I doubt any of these changes are going to be usable for the eventual runtime cross-compiling implementation

That's fine, as the goal of this PR is to get rid of static if, which Walter called an abomination that he'd like removed from ddmd. It may or may not help with cross-compiling eventually, but since that's not the explicit goal here, is not too important.

For example, the GCC varargs special case needs to either be pushed down to the glue layer, homogenized between frontends, or abstracted into Target. Changing static if to version simply does not do anything to help.

Again, it gets rid of static if, which is the goal of this PR. In other places in the frontend, version (IN_GCC) is already used, so all this PR does is use it consistently.

The Library.factory methods need to be fixed to work no matter what host platform, then chosen based on an abstraction in Target (or something). Changing them to be based on host is just wrong! And again it doesn't help.

If choosing Library.factory methods based on the host is wrong, the code is wrong now! All I'm replacing is a version check in ddmd.globals that sets a TARGET_LINUX enum that is checked with a static if in ddmd.lib by a direct version check in ddmd.lib. In regard to choosing based on the host, nothing has changed! All that has changed is the indirect use of static if to check the OS, rather than direct use of version. Other than replacing static if with version in ddmd.lib, the code is functionally the same.

As for whether this ddmd.lib change helps or not, I refer you to my previous comments: it gets rid of static if. It doesn't make those Library.factory modules cross-platform because that's outside the scope of this PR, which is to get rid of static if. Should I repeat that again?

So it is at best, a negligible improvement to a part of the compiler source we plan to remove.

It has not been removed and is still relied upon by dmd, though it cannot be used by ldc. If and when it's removed, you can be happy that it's not necessary.

[yebblies]

That's fine, as the goal of this PR is to get rid of static if

static if is still in the compiler for a reason, and the upside of pleasing Walter's sensibilities is not worth the costs of hacking it out like this. We'll get rid of it the right way, eventually. Walter may disagree, we'll see.

If choosing Library.factory methods based on the host is wrong, the code is wrong now!

No, it's not. The code correctly chooses which library type based on the target. The target selection is currently limited to the current host, but that doesn't make selecting the library based on the target any less correct.

While I appreciate your effort trying to improve the compiler, this amounts to a style change with a net negative effect (and some incorrect changes thrown in). Since this is not the direction we want to go, I don't see any point in merging these changes just to throw them out later.

[joakim-noah]

static if is still in the compiler for a reason, and the upside of pleasing Walter's sensibilities is not worth the costs of hacking it out like this.

There is no reason for continued use of static if, which is the real hack, in ddmd and there's basically no cost to this PR.

No, it's not. The code correctly chooses which library type based on the target. The target selection is currently limited to the current host, but that doesn't make selecting the library based on the target any less correct.

Right, that's my point: it was correct but limited to the same target as the host before and it will be correct and limited after this PR, which changes nothing about host/target selection. If it's "wrong" or "incorrect" now, which you keep claiming but cannot back up with anything, it was the same before.

While I appreciate your effort trying to improve the compiler, this amounts to a style change with a net negative effect (and some incorrect changes thrown in).

It's not a mere "style change," it's preferred usage by the language designer for the codebase he works on, the compiler. Now, who knows, maybe he doesn't feel as strongly about using static if for targets as hosts, but since you cannot name a single negative effect or "incorrect" change, I'll take that for the nonsense it is.

Since this is not the direction we want to go, I don't see any point in merging these changes just to throw them out later.

Lol, or it could just be merged now and then thrown out when you actually make those changes? Nah, that would make too much sense.

[ibuclaw]

@joakim-noah - We have a plan to deal with this. However this PR has nothing to do with it. I second the closing.

[yebblies]

There is no reason for continued use of static if, which is the real hack, in ddmd

There's no reason to change it to something worse just for the sake of changing it.

there's basically no cost to this PR.

The cost is small, but so is the gain. The cost is pointless runtime branches and initialization, and replacement of static if with uglier constructs.

Right, that's my point: it was correct but limited to the same target as the host before and it will be correct and limited after this PR, which changes nothing about host/target selection. If it's "wrong" or "incorrect" now, which you keep claiming but cannot back up with anything, it was the same before.

I don't know how I can be any clearer about this. The library format choice is based on target, not host - it is incorrect to choose based on host, which is what this pull does.

It's not a mere "style change," it's preferred usage by the language designer for the codebase he works on, the compiler. Now, who knows, maybe he doesn't feel as strongly about using static if for targets as hosts, but since you cannot name a single negative effect or "incorrect" change, I'll take that for the nonsense it is.

Take it however you like. This change doesn't add any functionality, doesn't fix any bugs, and doesn't get us closer to our design goals. That leaves style changes or churn...

Lol, or it could just be merged now and then thrown out when you actually make those changes? Nah, that would make too much sense.

Seems like throwing it out now gets to the same place with less steps.

[joakim-noah]

I don't know how I can be any clearer about this. The library format choice is based on target, not host - it is incorrect to choose based on host, which is what this pull does.

It was not always clear because you were not always this specific. We've been around this a couple times: you feel that choosing a target using a host version is "incorrect" but that it's not as bad if laundered through a static if that's initialized the exact same way, ie by the host version, whereas I think using the host version directly, in one place, is an equivalent hack as long as it's documented by a comment alongside, as I did.

As for the rest, I agree that static if versus runtime variables is not a big deal, but I do think this is an improvement because dmd will have to do this anyway. Walter said he wanted this changed, it's up to him if he still feels that way.