fix Issue 14464 - coverage merge doesn't work

[MartinNowak]

• fix parsing of existing .lst file
• use append semantics (create or merge with existing .lst file)
• enable merge by default
• some code cleanup (reuse buffers, log10, foreach, min/max)

Issue 14464 – coverage merge doesn't work

[MartinNowak]

Added a commit to perform race free updates of coverage files.

[MartinNowak]

Race free merging of coverage files finally works on all platforms.

[WalterBright]

Looks great, but I'm a bit perturbed by the automatic merging. My usage is along the lines of:

dmd std/file -main -unittest -cov
file
look at std-file.lst

and your change would make it:

dmd std/file -main -unittest -cov
rm std-file.lst
file
look at std-file.lst

which is annoying.

[MartinNowak]

Let me quickly move the coverage settings to core.runtime, then it's less annoying to turn merging on.

[MartinNowak]

Done.
It's still a bit surprising to have merging off by default, because other tools like gcov, gprof, and even dmd's -prof do merge by default.

[MartinNowak]

Ping @WalterBright

[MartinNowak]

3 weeks already, anyone?

[WalterBright]

What's the point of these trivial wrappers?

[MartinNowak]

Documenting the functionality.
Some extern C functions buried in the runtime don't qualify as API.

[WalterBright]

I don't understand this justification at all.

[MartinNowak]

???
Nobody knows that coverage generation can be configured.
If there are function that allow to change the behavior, we need to make them accessible and document them.

[MartinNowak]

Are you're wondering why I didn't just document the declarations for dmd_coverSourcePath?
Well b/c they have arcane names that don't fit into the rest of core.runtime's API, and I wanted to preserve the extern(C) names, in case someone is already using that functions.
I don't really like the namespacing Runtime struct either, but mixing different styles is worse.

[WalterBright]

Yes, I was wondering why. Thanks for the explanation. But I don't think it justifies creating yet more names. I really, really do not want Phobos/Druntime to become a bloated swamp of same-only-different names. We've been through this discussion of "arcane" names before, and the pros and cons of endless renamings. Let's stop doing it.

[MartinNowak]

Anything left @WalterBright?

[andralex]

@MartinNowak please do not add do-nothing one-liners that forward from Annas to Caiaphas. There are a few odd names in druntime, but no need to add yet another layer on top of those. Let's leave them be. Getting the right recommendations fostered for the larger set of contributors must start with the leadership observing them. Thanks.

[dnadlinger]

@andralex: I agree, but I think this point is irrelevant here. dmd_coverSourcePath et al. are currently not public API. Quick litmus test: Can you actually document them in a way visible to users? Or rather, how would a user know that these functions even exist from reading the druntime docs?

The functions discussed here are in rt.*, and as such can only be accessed from user code by manually defining a matching extern(C) declaration. There is a multitude of problems with that situation. First, forcing users to redeclare extern(C) functions in their code is error-prone; if there is a typo, there will be a silent ABI mismatch. Second, the existing names are odd. LDC now also implements a DMD-style -cov switch. dmd_* does not seem appropriate here. And third, as mentioned above, rt.* is private to druntime (at least for LDC, we don't even ship .d/.di files to users), so we are completely unable to document them.

[ibuclaw]

Exposing this in core.runtime is odd for me, as I don't use dmd-style coverage at all. And from my cursory look around, gcov doesn't offer any such equivalents to be set at runtime either.

[WalterBright]

1. We are NOT renaming functions any more. I've given the reasons why, at length, in a dozen threads. Please accept this and let's move on.
2. Defining such wrappers of module Y functions in module X means that linking in something from X will also pull in Y.
3. To make the extern (C) declaration of a Y function available from X, add this to X:

extern (C) T theYfunction();

The trailing ; means it is a declaration, not a definition. A wrapper is not required.

[dnadlinger]

Okay, I'll do nothing but smile and move on. Move on from D.

[dnadlinger]

@ibuclaw: You have a point there, although the functions could certainly be versioned off. Maybe a different module would be better? Or stick with with the "hidden" functions despite all their shortcomings, but then we'd need to make up for that with much better documentation than usual.

[MartinNowak]

What's all the discussion about? There are 3 internal functions that need to be documented and made available for usage and it just makes sense to do that in the same form as the rest of the core.runtime API.

[MartinNowak]

Exposing this in core.runtime is odd for me, as I don't use dmd-style coverage at all. And from my cursory look around, gcov doesn't offer any such equivalents to be set at runtime either.

So any suggestion on how to deal with this @ibuclaw? Apparently we need the same settings for LDC. Maybe add assertions or better make them noops?

[MartinNowak]

We are NOT renaming functions any more. I've given the reasons why, at length, in a dozen threads. Please accept this and let's move on.

Those functions arent't currently exposed and I still explicitly did not rename them, not sure why you're making that argument.

Defining such wrappers of module Y functions in module X means that linking in something from X will also pull in Y.

That's not true for our static libraries (we only link used functions) and dynamic libraries (they contain all code). We could also use an alias, but that's worse for documentation.

It is simply an API decision, and as the GDC/LDC discussion shows extern(C) dmd_coverSetMerge is not adequate.

[ibuclaw]

@MartinNowak maybe __traits(compiles, dmd_coverSetMerge) - I'm not one with answers on how to deal with internal APIs, just pointing out what breaks code convergence. ;-)

[WalterBright]

Please, take the wrappers out.

[MartinNowak]

Done

[WalterBright]

Thanks!

[WalterBright]

Auto-merge toggled on