|| <empty list> not covered properly

[ribasushi]

Apologies if this is a known issue, I couldn't find it in the existing bug reports.

A simple program like this:

my @foo;
push @foo, $_ || () for 0,1;

Produces 67% condition coverage like this:

A   B   dec
0   0   0
0   1   1
1   X   1

Is the 1/3 penalty warranted here, and is there a way to avoid it?

Cheers!

[pjcj]

Thanks for the report! I think this is a new problem, and seems to be that this empty list is not being recognised as a constant.

There are various tests in tests/cond_or, but not this one, and I presume I have also left it out of the code which actually checks for constants.

I will try to get to it during the QAH, if not before.

Thanks again.

[ribasushi]

Thanks for the prompt reply. While investigating a related failure, it occurred to me the rabbit hole goes deeper. I.e. it's not about constants alone, it is about "last part of implicit OR". Consider the following program:

use warnings;
use strict;

my $something = rand(10);

my $result;

for my $cond (0,1) {
  $result = $cond || $something;
}

for my $cond (0,1) {
  $result = $cond ? $cond : $something;
}

for my $cond (0,1) {
  if ($cond) {
    $result = $cond
  }
  else {
    $result = $something;
  }
}

The only spot that complains is the $this || $that variant, while the rest correctly report 100% coverage.

[pjcj]

I think the problem with $result = $cond || $something in this case is that you are wanting to use it in a specific way, and Devel::Cover is taking a more general view.

The general view is that this is a condition - a boolean value which, for full coverage, should be exercised such that $result is both true and false. And when it is true, it should be because $cond and $something are both true, separately.

Your use is, I think, that $result is simply a value. When $cond is false it should have the value of $something, but it does not matter what that value is.

Devel::Cover tries to work the way you are using || when $something is a constant, but when it is not it uses the more general view.

Running your code twice, once with $something being false and once with it true, would result in 100% coverage.

I don't think there is any way that Devel::Cover can know your intent in this case without some additional information. One way to provide additional information would be through the use of an uncoverable directive. But in this case you wouldn't know whether to specify that the case of $something being true or $something being false is the uncoverable case.

I'll take the original problem as a valid bug, but I'm not sure I want to do anything about the second one. It seems to me that you are conflating logic and general values. I am willing to be persuaded otherwise, particularly as the case of // seems stronger than ||, although the argument is actually the same there.

It's quite possible that the case makes sense in the context of your program though, and I really do like to keep Devel::Cover doing DWIM as much as possible. Changing this, though, would weaken coverage where people want the general case, so I don't want to just do that.

Perhaps some uncoverable directive which states that the value of the RHS is unimportant - that it should be treated the same way as a constant - could be a way forward.

[ribasushi]

It seems to me that you are conflating logic and general values.

This is actually a correct interpretation. What I think you are missing is that this is an incredibly common idiom. sub new { my $class = ref($_[0]) || $_[0]; ... alone is as old as perl5 itself.

I think a real in-depth solution is needed for this. Given you have access to the entire interpreter, and can reliably determine fine-grained context (akin to Want): can't you simply make an escape for "the last || or //" when you are not in a boolean context?

[dolmen]

See also #152 which is also about || with a constant on the right side.

[rwfranks]

ribasushi is right. This is so widely used that it merits a convenient solution.

my $suggestion = $like || $not; # uncoverable condition

The absence of any qualification indicates that any value is acceptable for both left and right arguments and the overall expression.

A similar situation arises with branches, and could be addressed in much the same way.

uncoverable branch

if ( unpredictable($value) ) { planA() } else { planB() }

assumed to be equivalent in every way to

uncoverable branch true

uncoverable branch false

if ( unpredictable($value) ) { ...

[ap]

FWIW, I just tripped over this myself.

(Hi @ribasushi 👋…)