fix Issue 9726 - Add minimum % coverage required for -cov testing #450
Conversation
| @@ -39,15 +36,21 @@ private | |||
| } | |||
| body | |||
| { | |||
| return cast(bool) bt( ptr, i ); | |||
There was a problem hiding this comment.
bt() and friends are no longer necessary, as the compiler will now recognize the form and generate the instructions directly. They're obsolete, and when I see 'em, I'll remove 'em.
I also don't like bt() because it's @System, and we should be moving away from those when we can.
There was a problem hiding this comment.
I don't quite understand, you're copy-pasting code from bt's body here, what's the benefit of that over simply issuing a function call?
There was a problem hiding this comment.
It's an unnecessary layer, and it is @System code yet it doesn't need to be if inlined.
There was a problem hiding this comment.
How is the inlined result not @system? You are indexing a raw pointer.
Even if DMD can now do these peephole optimizations as well, there is still value in avoiding code duplication.
There was a problem hiding this comment.
Good point. But that can be fixed, while bt() cannot be.
There was a problem hiding this comment.
I wouldn't call that a simplification of user code (#388 (comment)).
Good point. But that can be fixed, while bt() cannot be.
You could add a @safe overload to core.bitop which uses an array and indexing, it will still get optimized to bt after checking the bounds.
There was a problem hiding this comment.
I do call it a simplification. The array indexing and masking is a pretty standard sequence, so standard that modern compilers specifically recognize it and generate the BT instruction. Building two layers of abstraction for something so trivial is not simplification.
| { | ||
| fclose(flst); | ||
| fprintf(stderr, "Error: %.*s is %d%% covered, less than required %d%%\n", | ||
| c.filename.length, c.filename.ptr, percent, c.minPercent); |
| @@ -105,17 +108,26 @@ extern (C) void dmd_coverSetMerge( bool flag ) | |||
| * valid = ??? | |||
| * data = ??? | |||
| */ | |||
| extern (C) void _d_cover_register( string filename, size_t[] valid, uint[] data ) | |||
| extern (C) void _d_cover_register2(string filename, size_t[] valid, uint[] data, ubyte minPercent) | |||
There was a problem hiding this comment.
BTW, I find it very odd that this uses an size_t[] array to pass a size_t* and the number of bits.
Also the compiler uses uint[] internally so on x64 this only works because of the 16-byte data section alignment.
Maybe this is the right moment to improve this.
There was a problem hiding this comment.
size_t is used because that is the "native" word size on those machines, and it generates the most efficient code.
Whether the compiler calls it uint[] or not doesn't matter.
There was a problem hiding this comment.
Whether the compiler calls it uint[] or not doesn't matter.
It does matter if 4 of 8 loaded bytes are not part of the array.
It's also bad, that valid.length is not the length of the array.
fix Issue 9726 - Add minimum % coverage required for -cov testing
http://d.puremagic.com/issues/show_bug.cgi?id=9726
The druntime end. Will need a dmd pull, too.