www.digitalmars.com         C & C++   DMDScript  

digitalmars.D - Has someone encountered similar issues with -cov?

reply Andrei Alexandrescu <SeeWebsiteForEmail erdani.org> writes:
https://issues.dlang.org/show_bug.cgi?id=16224 -- Andrei
Jul 01 2016
next sibling parent Jack Stouffer <jack jackstouffer.com> writes:
On Friday, 1 July 2016 at 16:30:41 UTC, Andrei Alexandrescu wrote:

 https://issues.dlang.org/show_bug.cgi?id=16224 -- Andrei


Yeah: 
https://issues.dlang.org/buglist.cgi?quicksearch=coverage&list_id=209269
Jul 01 2016
prev sibling next sibling parent Basile B. <b2.temp gmx.com> writes:
On Friday, 1 July 2016 at 16:30:41 UTC, Andrei Alexandrescu wrote:

 https://issues.dlang.org/show_bug.cgi?id=16224 -- Andrei


I've reported this one a while back:

https://issues.dlang.org/show_bug.cgi?id=15590

if (__ctfe) branches are a problem because they really prevent to 
reach 100% coverage. D code similar to std.traits content is also 
a problem, it's never executed at run-time.
Jul 01 2016
prev sibling next sibling parent reply Chris <wendlec tcd.ie> writes:
On Friday, 1 July 2016 at 16:30:41 UTC, Andrei Alexandrescu wrote:

 https://issues.dlang.org/show_bug.cgi?id=16224 -- Andrei


I fail to see why it should not mark it as uncovered in the 
`cube` example. After all the statement is never covered, because 
`do` executes before the condition in `while` is checked. Unless 
you mean it should be optimized away by the compiler, which in 
turn has nothing to do with -cov.
Jul 01 2016
parent reply Andrei Alexandrescu <SeeWebsiteForEmail erdani.org> writes:
On 7/1/16 2:05 PM, Chris wrote:

 On Friday, 1 July 2016 at 16:30:41 UTC, Andrei Alexandrescu wrote:

 https://issues.dlang.org/show_bug.cgi?id=16224 -- Andrei


 I fail to see why it should not mark it as uncovered in the `cube`
 example. After all the statement is never covered, because `do` executes
 before the condition in `while` is checked. Unless you mean it should be
 optimized away by the compiler, which in turn has nothing to do with -cov.


Yah it's a bit subtle. That line is in fact pure punctuation, so even 
though there's no flow through it that's totally fine (as much as you 
wouldn't expect a line with a "}" to show no coverage). -- Andrei
Jul 01 2016
next sibling parent reply Chris <wendlec tcd.ie> writes:
On Friday, 1 July 2016 at 18:15:56 UTC, Andrei Alexandrescu wrote:

 Yah it's a bit subtle. That line is in fact pure punctuation, 
 so even though there's no flow through it that's totally fine 
 (as much as you wouldn't expect a line with a "}" to show no 
 coverage). -- Andrei


Not sure if it's pure punctuation. It is after all a statement 
checking for a condition, i.e. actually doing something. The fact 
that you bypass this check should not concern -cov, whose job it 
is to see whether a task is executed or not. You cannot expect 
-cov to do the optimizer's job on top of that. In fact one could 
argue that it shouldn't make assumptions.
Jul 01 2016
parent reply Andrei Alexandrescu <SeeWebsiteForEmail erdani.org> writes:
On 7/1/16 2:27 PM, Chris wrote:

 On Friday, 1 July 2016 at 18:15:56 UTC, Andrei Alexandrescu wrote:

 Yah it's a bit subtle. That line is in fact pure punctuation, so even
 though there's no flow through it that's totally fine (as much as you
 wouldn't expect a line with a "}" to show no coverage). -- Andrei


 Not sure if it's pure punctuation. It is after all a statement checking
 for a condition


What is the condition? -- Andrei
Jul 01 2016
parent Chris <wendlec tcd.ie> writes:
On Friday, 1 July 2016 at 20:06:39 UTC, Andrei Alexandrescu wrote:


 What is the condition? -- Andrei


`while`'s job is it to test for a condition and loop while the 
condition is true, even if the condition is `true` or `0`. So 
-cov does the right thing. It checks whether this part of the 
loop (`while xyz is the case`) is executed. In your case it isn't 
which is _your_ problem and not -cov's. It actually shows that 
your code might not be optimal, if you have something in it that 
is never executed and can never be covered. A dead statement.
Jul 02 2016
prev sibling parent reply Steven Schveighoffer <schveiguy yahoo.com> writes:
On 7/1/16 2:15 PM, Andrei Alexandrescu wrote:

 On 7/1/16 2:05 PM, Chris wrote:

 On Friday, 1 July 2016 at 16:30:41 UTC, Andrei Alexandrescu wrote:

 https://issues.dlang.org/show_bug.cgi?id=16224 -- Andrei


 I fail to see why it should not mark it as uncovered in the `cube`
 example. After all the statement is never covered, because `do` executes
 before the condition in `while` is checked. Unless you mean it should be
 optimized away by the compiler, which in turn has nothing to do with
 -cov.


 Yah it's a bit subtle. That line is in fact pure punctuation, so even
 though there's no flow through it that's totally fine (as much as you
 wouldn't expect a line with a "}" to show no coverage). -- Andrei


Suppose one wants to check if you've covered all cases inside the while 
loop (with breaks or returns). Then, one would WANT to see 0 coverage 
there (non-zero coverage would mean an error in logic).

To remove that feedback would mess up someone else's use case.

-Steve
Jul 01 2016
parent reply Andrei Alexandrescu <SeeWebsiteForEmail erdani.org> writes:
On 7/1/16 2:46 PM, Steven Schveighoffer wrote:

 On 7/1/16 2:15 PM, Andrei Alexandrescu wrote:

 On 7/1/16 2:05 PM, Chris wrote:

 On Friday, 1 July 2016 at 16:30:41 UTC, Andrei Alexandrescu wrote:

 https://issues.dlang.org/show_bug.cgi?id=16224 -- Andrei


 I fail to see why it should not mark it as uncovered in the `cube`
 example. After all the statement is never covered, because `do` executes
 before the condition in `while` is checked. Unless you mean it should be
 optimized away by the compiler, which in turn has nothing to do with
 -cov.


 Yah it's a bit subtle. That line is in fact pure punctuation, so even
 though there's no flow through it that's totally fine (as much as you
 wouldn't expect a line with a "}" to show no coverage). -- Andrei


 Suppose one wants to check if you've covered all cases inside the while
 loop (with breaks or returns). Then, one would WANT to see 0 coverage
 there (non-zero coverage would mean an error in logic).

 To remove that feedback would mess up someone else's use case.


This argument is phony. Unconvinced. -- Andrei
Jul 01 2016
next sibling parent reply Steven Schveighoffer <schveiguy yahoo.com> writes:
On 7/1/16 4:08 PM, Andrei Alexandrescu wrote:

 On 7/1/16 2:46 PM, Steven Schveighoffer wrote:

 On 7/1/16 2:15 PM, Andrei Alexandrescu wrote:

 On 7/1/16 2:05 PM, Chris wrote:

 On Friday, 1 July 2016 at 16:30:41 UTC, Andrei Alexandrescu wrote:

 https://issues.dlang.org/show_bug.cgi?id=16224 -- Andrei


 I fail to see why it should not mark it as uncovered in the `cube`
 example. After all the statement is never covered, because `do`
 executes
 before the condition in `while` is checked. Unless you mean it
 should be
 optimized away by the compiler, which in turn has nothing to do with
 -cov.


 Yah it's a bit subtle. That line is in fact pure punctuation, so even
 though there's no flow through it that's totally fine (as much as you
 wouldn't expect a line with a "}" to show no coverage). -- Andrei


 Suppose one wants to check if you've covered all cases inside the while
 loop (with breaks or returns). Then, one would WANT to see 0 coverage
 there (non-zero coverage would mean an error in logic).

 To remove that feedback would mess up someone else's use case.


 This argument is phony. Unconvinced. -- Andrei


I don't use while(0), so I have to invent a hypothetical use case. It's 
not phony. I could just as easily say that your coding style is 
illegitimate, and you should use while(true) instead (and thereby get 
100% coverage analysis). But I'm not saying that.

There is a legitimate difference between breaking out of the while loop 
using breaks, and breaking out using the while(0) condition. Why do you 
want coverage analysis to ignore that difference? Why would someone 
else's use case that relies on executing or not executing while(0) be 
invalid?

-Steve
Jul 01 2016
parent reply Patrick Schluter <Patrick.Schluter bbox.fr> writes:
On Friday, 1 July 2016 at 20:30:30 UTC, Steven Schveighoffer 
wrote:

 On 7/1/16 4:08 PM, Andrei Alexandrescu wrote:

 On 7/1/16 2:46 PM, Steven Schveighoffer wrote:

 On 7/1/16 2:15 PM, Andrei Alexandrescu wrote:

 On 7/1/16 2:05 PM, Chris wrote:

 On Friday, 1 July 2016 at 16:30:41 UTC, Andrei Alexandrescu 
 wrote:

 https://issues.dlang.org/show_bug.cgi?id=16224 -- Andrei


 I fail to see why it should not mark it as uncovered in the 
 `cube`
 example. After all the statement is never covered, because 
 `do`
 executes
 before the condition in `while` is checked. Unless you mean 
 it
 should be
 optimized away by the compiler, which in turn has nothing 
 to do with
 -cov.


 Yah it's a bit subtle. That line is in fact pure 
 punctuation, so even
 though there's no flow through it that's totally fine (as 
 much as you
 wouldn't expect a line with a "}" to show no coverage). -- 
 Andrei


 Suppose one wants to check if you've covered all cases inside 
 the while
 loop (with breaks or returns). Then, one would WANT to see 0 
 coverage
 there (non-zero coverage would mean an error in logic).

 To remove that feedback would mess up someone else's use case.


 This argument is phony. Unconvinced. -- Andrei


 I don't use while(0), so I have to invent a hypothetical use 
 case. It's not phony. I could just as easily say that your 
 coding style is illegitimate, and you should use while(true) 
 instead (and thereby get 100% coverage analysis). But I'm not 
 saying that.

 There is a legitimate difference between breaking out of the 
 while loop using breaks, and breaking out using the while(0) 
 condition. Why do you want coverage analysis to ignore that 
 difference? Why would someone else's use case that relies on 
 executing or not executing while(0) be invalid?

 -Steve


The do {} while(0) "trick" is generally used in C as a poor mans 
scope(exit) construct without using goto. It is to have one point 
where the resources used by a function can be freed without 
duplicating the code everywhere, especially if the function has 
multiple return points. So in practice it is only "useful" on 
complex functions with acquisition of resources (memory, files 
etc) and several exit points (error variants). These kind of 
functions already have difficult execution flows and adding a 
level of obfuscation on top of it is not the right solution. 
Using a goto, while not being the best solution, has at least the 
merrit of providing a searchable and meaningfull label (goto 
error or goto finally or goto cleanup are obvious, break not so 
much) to find the exit. With break it's a little bit more 
difficult. Add loops and switch's to the equation and you're in 
for a very confusing debugging session.
Jul 02 2016
parent Patrick Schluter <Patrick.Schluter bbox.fr> writes:
On Saturday, 2 July 2016 at 07:04:28 UTC, Patrick Schluter wrote:

 On Friday, 1 July 2016 at 20:30:30 UTC, Steven Schveighoffer 
 wrote:

 On 7/1/16 4:08 PM, Andrei Alexandrescu wrote:

 On 7/1/16 2:46 PM, Steven Schveighoffer wrote:

 On 7/1/16 2:15 PM, Andrei Alexandrescu wrote:

 On 7/1/16 2:05 PM, Chris wrote:

 On Friday, 1 July 2016 at 16:30:41 UTC, Andrei 
 Alexandrescu wrote:

 https://issues.dlang.org/show_bug.cgi?id=16224 -- Andrei


 I fail to see why it should not mark it as uncovered in 
 the `cube`
 example. After all the statement is never covered, because 
 `do`
 executes
 before the condition in `while` is checked. Unless you 
 mean it
 should be
 optimized away by the compiler, which in turn has nothing 
 to do with
 -cov.


 Yah it's a bit subtle. That line is in fact pure 
 punctuation, so even
 though there's no flow through it that's totally fine (as 
 much as you
 wouldn't expect a line with a "}" to show no coverage). -- 
 Andrei


 Suppose one wants to check if you've covered all cases 
 inside the while
 loop (with breaks or returns). Then, one would WANT to see 0 
 coverage
 there (non-zero coverage would mean an error in logic).

 To remove that feedback would mess up someone else's use 
 case.


 This argument is phony. Unconvinced. -- Andrei


 I don't use while(0), so I have to invent a hypothetical use 
 case. It's not phony. I could just as easily say that your 
 coding style is illegitimate, and you should use while(true) 
 instead (and thereby get 100% coverage analysis). But I'm not 
 saying that.

 There is a legitimate difference between breaking out of the 
 while loop using breaks, and breaking out using the while(0) 
 condition. Why do you want coverage analysis to ignore that 
 difference? Why would someone else's use case that relies on 
 executing or not executing while(0) be invalid?

 -Steve


 The do {} while(0) "trick" is generally used in C as a poor 
 mans scope(exit) construct without using goto. It is to have 
 one point where the resources used by a function can be freed 
 without duplicating the code everywhere, especially if the 
 function has multiple return points. So in practice it is only 
 "useful" on complex functions with acquisition of resources 
 (memory, files etc) and several exit points (error variants). 
 These kind of functions already have difficult execution flows 
 and adding a level of obfuscation on top of it is not the right 
 solution. Using a goto, while not being the best solution, has 
 at least the merrit of providing a searchable and meaningfull 
 label (goto error or goto finally or goto cleanup are obvious, 
 break not so much) to find the exit. With break it's a little 
 bit more difficult. Add loops and switch's to the equation and 
 you're in for a very confusing debugging session.


Sorry for answering my own comments but the do while(0) code at 
the link is already a good example of why that construct is bad. 
To discover what it does, one has to try every possible way in 
its head, see that it uses an inverted logic (i.e. a double 
negation) if my parameter isn't a specific value don't do what is 
close by.

     do
     {
         if (x != 0)
             break;
         if (x != 1)
             break;
         if (x != -1)
             break;
         return x;
     }
     while (0);
     return x * x * x;

is really a lot of lines to write

     if (x == 0 || x == 1 || x == -1)
         return x*x*x;
     else
         return x;

or

     return (x == 0 || x == 1 || x == -1) ? x*x*x : x;


Sorry to still insist on that side show, but it is this inverted 
logic that is worst part of this construct, making it really, 
really difficult to analyze in practice.
Jul 02 2016
prev sibling parent Ola Fosheim =?UTF-8?B?R3LDuHN0YWQ=?= writes:
On Friday, 1 July 2016 at 20:08:30 UTC, Andrei Alexandrescu wrote:

 To remove that feedback would mess up someone else's use case.


 This argument is phony. Unconvinced. -- Andrei


If you are hellbent on using a loop to execute a scope once, do 
something that makes it clear at the top of the scope:

for once {
    ...
}

e.g.:

import std.stdio;

void main(){

	foreach(_;[1]){
		writeln("once");
	}
	
}
Jul 02 2016
prev sibling next sibling parent reply Patrick Schluter <Patrick.Schluter bbox.fr> writes:
On Friday, 1 July 2016 at 16:30:41 UTC, Andrei Alexandrescu wrote:

 https://issues.dlang.org/show_bug.cgi?id=16224 -- Andrei


That

     do {
     } while(0)

construct is ridiculous. It's cargo cult at its worst. It is NOT 
more readable than an honest to god goto. It's an obfuscated way 
to write naughty gotos without the guilt of sinning (I 
deliberately use religous vocabulary because it is a religious 
disease). It's the goto of the hypocrits.
Sorry if I sound harsh, but I share a project (in C) with a 
colleague who loves that construct and after almost 15 years of 
having to debug that sh... I can affirm that that construct is a 
scourge and deserves all the scorn that can be heaped on it.
It looks like a loop, but isn't one.
It doesn't look like a goto, but is one.
It's horribly difficult to amend, especially if there is actually 
a real loop somewhere around or inside it.
It pushes the user to write unnecessary big functions (anecdote, 
I modified once a 200 lines do while(0) behemoth that could be 
reduced to 4 functions of 10 lines each and a small loop. After 
conversion and simplification I discovered that the original 
didn't even cover all functional cases and leaked some memory).
I don't think it makes even an inkling of sense to use it in D as 
there are real language constructs that can cleanly replace it 
(scope, try/catch/finally, destructors, etc.).
Jul 01 2016
next sibling parent reply Andrei Alexandrescu <SeeWebsiteForEmail erdani.org> writes:
On 7/1/16 3:43 PM, Patrick Schluter wrote:

 On Friday, 1 July 2016 at 16:30:41 UTC, Andrei Alexandrescu wrote:

 https://issues.dlang.org/show_bug.cgi?id=16224 -- Andrei


 That

     do {
     } while(0)

 construct is ridiculous. It's cargo cult at its worst.


That escalated quickly. -- Andrei
Jul 01 2016
parent reply Patrick Schluter <Patrick.Schluter bbox.fr> writes:
On Friday, 1 July 2016 at 20:09:30 UTC, Andrei Alexandrescu wrote:

 On 7/1/16 3:43 PM, Patrick Schluter wrote:

 On Friday, 1 July 2016 at 16:30:41 UTC, Andrei Alexandrescu 
 wrote:

 https://issues.dlang.org/show_bug.cgi?id=16224 -- Andrei


 That

     do {
     } while(0)

 construct is ridiculous. It's cargo cult at its worst.


 That escalated quickly. -- Andrei


Nothing personal. As I said in the initial message, I had to put 
up with that construct for the last 15 years because my colleague 
loves it. So I had ample time to learn to hate it with a passion.
Jul 01 2016
next sibling parent Patrick Schluter <Patrick.Schluter bbox.fr> writes:
On Saturday, 2 July 2016 at 06:45:37 UTC, Patrick Schluter wrote:

 On Friday, 1 July 2016 at 20:09:30 UTC, Andrei Alexandrescu 
 wrote:

 On 7/1/16 3:43 PM, Patrick Schluter wrote:

 On Friday, 1 July 2016 at 16:30:41 UTC, Andrei Alexandrescu 
 wrote:

 https://issues.dlang.org/show_bug.cgi?id=16224 -- Andrei


 That

     do {
     } while(0)

 construct is ridiculous. It's cargo cult at its worst.


 That escalated quickly. -- Andrei


 Nothing personal.


I should detail, nothing personal against you, but very personal 
on my side :-)
Jul 02 2016
prev sibling parent reply Andrei Alexandrescu <SeeWebsiteForEmail erdani.org> writes:
On 07/02/2016 02:45 AM, Patrick Schluter wrote:

 On Friday, 1 July 2016 at 20:09:30 UTC, Andrei Alexandrescu wrote:

 On 7/1/16 3:43 PM, Patrick Schluter wrote:

 On Friday, 1 July 2016 at 16:30:41 UTC, Andrei Alexandrescu wrote:

 https://issues.dlang.org/show_bug.cgi?id=16224 -- Andrei


 That

     do {
     } while(0)

 construct is ridiculous. It's cargo cult at its worst.


 That escalated quickly. -- Andrei


 Nothing personal. As I said in the initial message, I had to put up with
 that construct for the last 15 years because my colleague loves it. So I
 had ample time to learn to hate it with a passion.


How would you reshape this? It's important that the call to hook is 
physically at the end of the function and issued just in that place, and 
that the code does not do any redundant work.

U hook(U, T)(T value) { return U.init; }

U opCast(U, T)(T payload)
{
	import std.traits;
	do
	{
		static if (!isUnsigned!T && isUnsigned!U &&
				   T.sizeof <= U.sizeof)
		{
			if (payload < 0) break; // extra check needed
		}
		auto result = cast(U) payload;
		if (result != payload) break;
		static if (isUnsigned!T && !isUnsigned!U)
		{
			static assert(U.sizeof <= T.sizeof);
			if (result < 0) break;
		}
		return result;
	} while (0);
	return hook!U(payload);
}

void main()
{
	opCast!short(42);
}


Thanks,

Andrei
Jul 02 2016
next sibling parent reply Johan Engelen <j j.nl> writes:
On Saturday, 2 July 2016 at 12:26:34 UTC, Andrei Alexandrescu 
wrote:

 How would you reshape this?


Maybe:

U opCast(U, T)(T payload)
{
     import std.traits;

     enum descriptiveName = !isUnsigned!T && isUnsigned!U && 
T.sizeof <= U.sizeof;
     enum unsT_sigU = isUnsigned!T && !isUnsigned!U;
     static if (unsT_sigU)
     {
         static assert(U.sizeof <= T.sizeof);
     }

     if (!descriptiveName || payload >= 0)
     {
         auto result = cast(U) payload;
         if (result == payload)
         {
             if (!unsT_sigU || result >= 0)
             {
                 return result;
             }
         }
     }

     return hook!U(payload);
}
Jul 02 2016
parent reply Andrei Alexandrescu <SeeWebsiteForEmail erdani.org> writes:
On 07/02/2016 09:23 AM, Johan Engelen wrote:

 On Saturday, 2 July 2016 at 12:26:34 UTC, Andrei Alexandrescu wrote:

 How would you reshape this?


 Maybe:

 U opCast(U, T)(T payload)
 {
      import std.traits;

      enum descriptiveName = !isUnsigned!T && isUnsigned!U && T.sizeof <=
 U.sizeof;
      enum unsT_sigU = isUnsigned!T && !isUnsigned!U;
      static if (unsT_sigU)
      {
          static assert(U.sizeof <= T.sizeof);
      }

      if (!descriptiveName || payload >= 0)
      {
          auto result = cast(U) payload;
          if (result == payload)
          {
              if (!unsT_sigU || result >= 0)
              {
                  return result;
              }
          }
      }

      return hook!U(payload);
 }


Nice, thanks. I've tried to not rely too much on mixing statically-known 
and dynamically-known Boolean expressions. Can I safely assume that all 
compilers will generate good code for such? According to asm.dlang.org, 
dmd generates identical code at least for the two functions above. -- Andrei
Jul 02 2016
parent Johan Engelen <j j.nl> writes:
On Saturday, 2 July 2016 at 14:03:29 UTC, Andrei Alexandrescu 
wrote:

 Nice, thanks. I've tried to not rely too much on mixing 
 statically-known and dynamically-known Boolean expressions. Can 
 I safely assume that all compilers will generate good code for 
 such?


I'd be very surprised if not (especially with statically-known 
booleans upfront in the expression).

I get this for opCast!(short, int)(int):

asm.dlang.org ("-O -inline")
     push   %rax
     mov    %rdi,%rcx
     movswl %cx,%eax
     cmp    %ecx,%eax
     jne    label1
     mov    %rdi,%rax
     jmp    label2
     label1:
     xor    %eax,%eax
     label2:
     pop    %rcx
     retq

LDC trunk (-O3)  (identical code for your and my version)
     movswl  %di, %eax
     cmpl      %edi, %eax
     jne         LBB2_1
     movswl  %di, %eax
     retq
     LBB2_1:
     xorl        %eax, %eax
     retq

Define "good" ;-) ;-)

PS: you can look at LDC's annotated asm output with "-c 
-output-s".
Jul 02 2016
prev sibling next sibling parent reply Guillaume Boucher <guillaume.boucher.d gmail.com> writes:
On Saturday, 2 July 2016 at 12:26:34 UTC, Andrei Alexandrescu 
wrote:

 How would you reshape this? It's important that the call to 
 hook is physically at the end of the function and issued just 
 in that place, and that the code does not do any redundant work.


Your function does redundant work.  E.g. opCast!(int, ubyte) 
should not require any checks.  I also don't understand why 
opCast!(int, ubyte) is not allowed.

The following code should do the same as yours, but without 
unnecessary checks:

U opCast(U, T)(T payload)
{
         import std.traits;
         enum Tsizeof = is(T==bool) ? 0 : T.sizeof;
         enum Usizeof = is(U==bool) ? 0 : U.sizeof;
         enum noCheck = isUnsigned!T == isUnsigned!U && Tsizeof <= 
Usizeof ||
                        isUnsigned!T && Tsizeof < Usizeof;
         enum checkPayload = !isUnsigned!T && isUnsigned!U;
         enum checkResult = isUnsigned!T && !isUnsigned!U;
         static if (checkResult)
         {
                 static assert(U.sizeof <= T.sizeof);  // I don't 
understand this
         }

         if (!checkPayload || payload >= 0)
         {
                 auto result = cast(U) payload;
                 if (noCheck || result == payload && (!checkResult 
|| result >= 0))
                         return result;
         }
         return hook!U(payload);
}
Jul 02 2016
next sibling parent Guillaume Boucher <guillaume.boucher.d gmail.com> writes:
On Saturday, 2 July 2016 at 15:15:39 UTC, Guillaume Boucher wrote:

 E.g. opCast!(int, ubyte) should not require any checks.


Or opCast!(long, int) and opCast!(int, int).
Jul 02 2016
prev sibling next sibling parent Andrei Alexandrescu <SeeWebsiteForEmail erdani.org> writes:
On 07/02/2016 11:15 AM, Guillaume Boucher wrote:

 Your function does redundant work.  E.g. opCast!(int, ubyte) should not
 require any checks.  I also don't understand why opCast!(int, ubyte) is
 not allowed.


It's an adapted fragment from a larger function that handles other cases 
separately. -- Andrei
Jul 02 2016
prev sibling parent Ola Fosheim =?UTF-8?B?R3LDuHN0YWQ=?= writes:
On Saturday, 2 July 2016 at 15:15:39 UTC, Guillaume Boucher wrote:

 U opCast(U, T)(T payload)
 {
         import std.traits;
         enum Tsizeof = is(T==bool) ? 0 : T.sizeof;
         enum Usizeof = is(U==bool) ? 0 : U.sizeof;
         enum noCheck = isUnsigned!T == isUnsigned!U && Tsizeof 
 <= Usizeof ||
                        isUnsigned!T && Tsizeof < Usizeof;
         enum checkPayload = !isUnsigned!T && isUnsigned!U;
         enum checkResult = isUnsigned!T && !isUnsigned!U;
         static if (checkResult)
         {
                 static assert(U.sizeof <= T.sizeof);  // I 
 don't understand this
         }

         if (!checkPayload || payload >= 0)
         {
                 auto result = cast(U) payload;
                 if (noCheck || result == payload && 
 (!checkResult || result >= 0))
                         return result;
         }
         return hook!U(payload);
 }


I got to something similar (probably with some typos), assuming 
.sizeof exists:


U opCast(U, T)(T payload)
{
     import std.traits;

     enum unsigned_to_signed =  isUnsigned!T && !isUnsigned!U;
     enum signed_to_unsigned =  !isUnsigned!T && isUnsigned!U;
     enum maybe_to_smaller = T.sizeof >= U.sizeof;
     enum to_smaller = T.sizeof > U.sizeof;

     static assert( !unsigned_to_signed || maybe_to_smaller);

     if( !signed_to_unsigned || to_smaller || payload >= 0 )
     {
         auto result = cast(U) payload;
         if (result == payload && !( unsigned_to_signed && result 
< 0))
             return result;
     }

     return hook!U(payload);
}
Jul 02 2016
prev sibling parent reply Timon Gehr <timon.gehr gmx.ch> writes:
On 02.07.2016 14:26, Andrei Alexandrescu wrote:

 How would you reshape this? It's important that the call to hook is
 physically at the end of the function and issued just in that place, and
 that the code does not do any redundant work.

 U hook(U, T)(T value) { return U.init; }

 U opCast(U, T)(T payload)
 {
      import std.traits;
      do
      {
          static if (!isUnsigned!T && isUnsigned!U &&
                     T.sizeof <= U.sizeof)
          {
              if (payload < 0) break; // extra check needed
          }
          auto result = cast(U) payload;
          if (result != payload) break;
          static if (isUnsigned!T && !isUnsigned!U)
          {
              static assert(U.sizeof <= T.sizeof);
              if (result < 0) break;
          }
          return result;
      } while (0);
      return hook!U(payload);
 }

 void main()
 {
      opCast!short(42);
 }


 Thanks,

 Andrei


How do you decide what 'redundant work' is? Is this combination of 
branches and type casts really particularly cheap to execute? Could 
masking out the "sign bit" from the unsigned value before comparison be 
faster? Also, isn't the result != payload check redundant if sizeof(U) 
== sizeof(T)?

Anyway, this is the kind of optimization that compilers are supposed to 
be good at, it should pick whatever procedure is optimal, not 
necessarily the one specified. Personally, I'd just write something like:

U opCast(U,T)(T payload)if(...){
     auto result = cast(U)payload;
     if(result == payload && (result<0) == (payload<0))
         return result;
     return hook!U(payload);
}
Jul 02 2016
parent Ola Fosheim =?UTF-8?B?R3LDuHN0YWQ=?= writes:
On Sunday, 3 July 2016 at 00:57:04 UTC, Timon Gehr wrote:

 How do you decide what 'redundant work' is? Is this combination 
 of branches and type casts really particularly cheap to execute?


He just provided an example, so preserving the  essence of the 
logic for all kinds of U and T is important. The sizeof test is 
flawed in general (it does not reflect the maximum for all types 
on all hardware).

The question was more whether "break" usually is more clear and 
performant for this type of code. And the answer is no...
Jul 02 2016
prev sibling parent Ola Fosheim =?UTF-8?B?R3LDuHN0YWQ=?= writes:
On Friday, 1 July 2016 at 19:43:05 UTC, Patrick Schluter wrote:

 It looks like a loop, but isn't one.
 It doesn't look like a goto, but is one.


Yes, it looks buggy, and the -cov did the right thing by marking 
it as uncovered as this could be a serious and difficult to find 
bug.

I wonder why people write such ugly and misleading code 
deliberately? The compiler turns it into jumps anyway and in this 
case a bool or gotos are much more readable and not slower.

In fact, a statemachine implemented as gotos can be very clean, 
efficient and easy to debug. It's a matter of structuring the 
code layout.
Jul 01 2016
prev sibling parent Dicebot <public dicebot.lv> writes:
On 07/01/2016 07:30 PM, Andrei Alexandrescu wrote:

 https://issues.dlang.org/show_bug.cgi?id=16224 -- Andrei


Works as it should, nothing to fix.
Jul 02 2016