Getlines precedence 20240713

[jkeenan]

While using cpan built with Task::CPAN::Reporter today to install a CPAN library (podlators) and file a CPANtesters report, I observed this warning:

Possible precedence issue with control flow operator (return) at /home/jkeenan/testing/e9f8aee2bc0fcdce0a13746848aad38c23073ef7/lib/perl5/site_perl/5.41.1/IO/Socket/SSL.pm line 2599.

This is similar to the problem addressed in the recently closed https://github.com/noxxi/p5-io-socket-ssl/issues/155.

perldoc perldiag has this to say (excerpt):

Possible precedence issue with control flow operator (%s)
    (W syntax) There is a possible problem with the mixing of a control
    flow operator (e.g. "return") and a low-precedence operator like
    "or". Consider:
    ...
    Note this may be also triggered for constructs like:

        sub { 1 if die; }

While looking at the code that triggered the warning, I wondered whether the croak statement was being exercised in the test suite. I ran the tests through Devel::Cover and observed that it was not being exercised. So I added a test to detect the exception as the first commit in this pull request. Please review.

[noxxi]

Thanks for the report. I don't understand the cause of the warning though and why the change would "fix" it. I also cannot reproduce the issue - how can I do this?

[jkeenan]

Thanks for the report. I don't understand the cause of the warning though and why the change would "fix" it. I also cannot reproduce the issue - how can I do this?

I don't have an answer for that yet. I simply observed the warning in the course of running the program previously described, and then noticed that the statement in question was similar in syntax to the example from perldiag.

[mauke]

This is similar to the problem addressed in the recently closed https://github.com/noxxi/p5-io-socket-ssl/issues/155.

Isn't it identical? As in, if you upgrade to version 2.088 of IO::Socket::SSL, the cpan warning should disappear.

The code modified in this commit (line 2256),

    return(shift->readline()) if wantarray();

is not mentioned in the warning message. (It wouldn't trigger a warning anyway because wantarray is not a control flow operator, so if wantarray is fine­ ­— unlike if die, which transfers control and never returns a value.)

[mauke]

This PR has been open for a while.

• I think the second commit should just be removed; there is nothing wrong with return ... if.
• The "getlines in scalar context" test looks good to me, but its placement is weird (right in the middle of the @array tests). I'd move it 4 lines up, before @array = $client->getlines;.