Because

is really

try || instead of or

I get:

Possible precedence issue with control flow operator at noname.pl line
+ 23.
Useless use of string in void context at noname.pl line 23.
[download]

for the return $result{'message'} ... line, which makes sense. Remember or is low precedence so the compiler sees:

(return $result{'message'}) or "$result{'api-key'}:$result{'session'}"
+;
[download]

High precedence || would fix the problem

Update: note that I'm using Perl v5.32.0.

I can never remember the rules of precedence, so I just remove the relevance of such considerations by using brackets to make my intentions clear:

C:\>perl -MO=Deparse,-p -e "return $x or $y"
((return $x) or $y);
-e syntax OK

C:\>perl -MO=Deparse,-p -e "return ($x or $y)"
(return ($x || $y));
-e syntax OK
[download]

Cheers,
Rob

It has been my habit of long standing to use the word forms of the logical operators (and, or and not) purely for control flow and to use the symbolic forms (&&, || and !) for comparing values within an expression. This makes precedence bugs of the kind you have reported here much less frequent.

HTH.

Ditto! Strongly agree!! :)

Note that Perl Best Practices, chapter 6 (Control Structures), "Use block if, not postfix if" argues that postfix-if does not scale as well as block-if, and is harder to comprehend (except in simple cases). I agree, especially with the scaling argument. Always using block-if has made code reviews more enjoyable for me over many years because there are fewer changed lines of code to review whenever you just add an extra statement to a block-if (compared to more violently restructuring the code from postfix-if to block-if).

In chapter 4 (Values and Expressions), "Don't mix high- and low-precedence booleans", Conway recommends using the low precedence and and or operators for flow of control, for example:

open(my $fh, '<', $file) or die "error opening '$file': $!";
[download]

while reserving && and || for logical expressions (not flow of control) for example:

if ($x > 5 && $y < 10) ...
[download]

Following this simple rule over the years has made the code easier to understand at a glance, at least for me.

Use block if, not postfix if

Are you suggesting postfix if should never be used?

I agree that adding an extra conditional statement requires more work and more consideration but there are cases, like my conditional return statement, where it is rather unlikely that any extra statements would want to be added to the conditional execution.

G'day Bod,

When you encounter this sort of issue, you'll often find using B::Deparse, to see how Perl interprets your code, will help.

$ perl -MO=Deparse,-p -e 'return $x or $y'
((return $x) or $y);
-e syntax OK

$ perl -MO=Deparse,-p -e 'return $x || $y'
(return ($x || $y));
-e syntax OK
[download]

Thanks Ken
I've seen B::Deparse mentioned here and there and keep meaning to find out what it does and why I might want it. I guess you've just answered that for me!

FWIW, note also that the
    return "SOMETHING";
statement in the OPed code is unreachable.

FWIW, note also that the

    return "SOMETHING";
[download]

statement in the OPed code is unreachable.

Yes - I should have pointed out the reason for that line in the question...
It was added as part of trying to work out what was actually happening. Having a definite return ensured that I was not getting a return value from the last evaluated expression. I did try to eliminate some of the possibilities before turning to the Monastery.

I love this question because it identifies a problem that is a common pitfall. I've even discovered it in the past creeping into code at work.

Look at this code:

sub foo {
    return $bar or $baz
}
[download]

Pass that through B::Deparse:

perl -MO=Deparse -e 'sub foo {return $bar or $baz}'
sub foo {
    $baz unless return $bar;
}
[download]

The code return $bar has to be executed before its value can be calculated for the unless condition. But return exits the subroutine, and leaves "unless" hanging. Control passes out of the sub. Game over for the unless statement. It never gets an opportunity to regain control of flow, so $baz could never be hit.

We could run very similar code through Deparse using the || operator:

perl -MO=Deparse -e 'sub foo {return $bar || $baz}'
sub foo {
    return $bar || $baz;
}
[download]

Here we find none of the surprising behavior, the expression $bar || $baz has to get evaluated first, then the return happens.

The return could be coerced to work "as you wish" by using parenthesis, too:

sub foo {
    return ($bar or $baz);
}
[download]

We can pass that through B::Deparse as well:

perl -MO=Deparse  -e 'sub foo {return ($bar or $baz)}'
sub foo {
    return $bar || $baz;
}
[download]

Look at that: It parsed down to the || operator.

We had another one pop up in some code a year or so ago. It's not exactly the same, but in a way similar:

sub foo {
    my $filename = shift;
    my $content = do {
        local $/;
        open my $fh, '<', $filename or die $!;
        return scalar(<$fh>);
    }
    $content = s/abc/def/g;
    return $content;
}
[download]

Spot the problem? You shouldn't use return in a do block unless you actually intend to return out of the surrounding subroutine. The writer of that code knew that do blocks return something to their caller. So they used a return to make it explicit (rather than falling back to the last thing that got evaluated in the do block). But that's incorrect; the return returned out of the foo subroutine, leaving the assignment to $content hanging, and the remainder of the subroutine unreachable. In many of the cases I spotted of this, it wasn't apparent what was happening because the caller of foo() still got a return value that seemed reasonable. I think in the actual case where I saw it, the context was poor-man's caching:

sub get_foo {
    my $self = shift;
    return $self->{'foo'} ||= do {
        # expensive operation;
        return $result;
    }
}
[download]

Here the biggest clue there was a problem was that the call to that accessor took longer than it should on any call after the first time it was called. That's rather hard to test for, though a test that breaks encapsulation and looks at the object's guts would have noticed.

Every single line of code untenable…

syntax error at - line 1, near "+}"
Execution of - aborted due to compilation errors.
[download]

Not to mention that return $a ? $a : $b; can always* be written as return $a || $b; ... which is exactly what tybalt already said.
(*unless $a is tied.)