Perl's comma operator , performs several duties. The most widely known duty is to serve as a list separator:

my @list = (2, 3, 5, 7);

Outside of list context, the comma can also be used to combine multiple expressions into one statement. Each expression is evaluated, and its result is discarded. The last expression's result is returned as the result of the comma operator. Comma operators are called thin commas [Conway 2005]. This behavior was adopted from C.

The potential for confusing thin commas with commas in list context is large enough to forbid use of thin commas. Commas must be used only to separate items in list context.

Noncompliant Code Example

This code example validates a file and indicates if it exists.

sub validate_file {
  my $file = shift(@_);
  if (-e $file) {
    return 1; # file exists
  }
  die "$file does not exist";
}

my $file = $ARGV[0];
validate_file($file), print "hi!\n";

This code behaves as expected. The comma operator is used to separate the call to validate_file and subsequent call to print in the same statement. Consequently, the return value of validate_file is discarded before print is called.

This line of code looks like it would behave the same but instead behaves quite differently:

print validate_file($file), "hi!\n";

The print statement takes a list of items to print, and, in list context, the comma operator is assumed to separate list items. Consequently, if the file is valid, this program prints 1 before its friendly greeting.

Compliant Solution (Segregation)

This compliant solution segregates the call to validate_file into a separate statement.

validate_file($file);
print "hi!\n";

Compliant Solution (do)

If multiple functions must be invoked within one statement, a do block can be used to evaluate a list of expressions without using list context.

print do { validate_file($file); "hi!\n"};

Risk Assessment

Using commas to separate statements can lead to unexpected program behavior and surprising results.

Recommendation

Severity

Likelihood

Remediation Cost

Priority

Level

MSC30-PL

Low

Probable

Medium

P4

L3

Automated Detection

Tool

Diagnostic

Perl::Critic

ValuesAndExpressions::ProhibitCommaSeparatedStatements

Bibliography

 


7 Comments

  1. You may add an exception for the C-style for loop?

    1. I take it you mean to separate expressions that should be ignored? eg

      for (my $j = 0, my $i = 0; $i < 10; $i++) ...
      

      Given the many other ways code like this could be written, I'm not sure its worthwhile making an exception for this.

      1. Yes I mean the idiom common in C of using the comma operator in the initialization or update parts of a for loop:

        for (int i = 0, j = 0; i < 10; i++, j++)

                printf("%d\n", i + j);

         

        A programmer familiar with this from C might want to use a similar 'i++, j++' in Perl.

        1. Having re-studied Conway, I can see he did consider this possibility and did not consider it a 'valid exception'. Clearly, he would rather see your code example rewritten as:

          my $i;
          my $j;
          for (do {$i = 0; $j = 0}; $i < 10; do {$i++; $j++}) {
            printf("%d\n", $i + $j);
          }
          

          I see no reason to disagree with him.

          1. E. Choroba says, via email:

            I'd propose another, and in my humble opinion more Perlish, way to express the for loop with two variables:

            for ( my ($i, $j) = (0, 0); $i < 10; ($i, $j) = ($i + 1, $j + 2) ) {

            At least for me, it's more readable, and avoids do { ... } which might seem exotic to non-gurus.

  2. I believe there's a slight error in the first part of this recommendation; the comment on the fat comma Conway refers to (from page 66) means the "=>" construct used in hash key/value pair construction, as opposed to the 'thin' comma that is just used in a list ( , ).  If the last part of the first paragraph about the fat comma was stricken, it would probably be all that's needed.

    1. Reworded intro, thanks.