Should a diagnostic be emmited for discarded value expressions that do not have side effects?

Question

After quite some debugging time I felt silly to discover a goof in my code that boils down to something like this :

int main()
{
    double p1[] = { 1, 2, 3 };
    double p2[] = { 1, 2, 3 };
    int color = 1;
    bool some_condition = true;

    if (some_condition) (p1, p2, color);
}

The (p1, p2, color) expression evaluates to its last operant, but should the compiler protect me in some way ? (Visual Studio told nothing)

And yes you guessed it right, I wanted to call a draw function : Draw(p1, p2, color)

Answer 1

In C++ the expression (p1, p2, color) forces the compiler to interpret the commas inside the parentheses as the sequential-evaluation operator. The sequential-evaluation operator is a binary operator that evaluates its first operand as void and discards the result, it then evaluates the second operand and returns its value and type. Therefore, the expression (p1, p2, color) is going to be evaluated in the following way:

1. First p1 is evaluated and discarded, then (p2, color) is evaluated and the result (p2, color) is returned.
2. First p2 is evaluated and discarded, then color is evaluated and the result color is returned.

Thus the statement:

if (some_condition) (p1, p2, color);

Is equivalent to:

if (some_condition) color;

Some compilers might raise a warning because during the evaluation of the expression (p1, p2, color) the evaluations of p1 and p2 will result in unused:

CLANG LIVE DEMO GCC LIVE DEMO (As you already mentioned Visual Studio will raise no warning at all.)

Apart from these warnings the code is legitimate C++ (i.e., C++ syntax isn't violated).

Now, whether the compiler should protect you is debatable. In my humble opinion it should protect you, since such expressions although correct from C++ syntax point of view might result, in very hard to spot bugs (e.g., the case of assingment inside an if expression).

Answer 2

It is perfectly valid code, so the compiler does not have to issue a diagnostic although having a diagnostic would be helpful in this case. This is one of the reasons many developers prefer clang since they tend to go above what is required when it comes to diagnostic.

For the standards rules on diagnostic messages, we can go to the draft C++ standard section 1.4 Implementation compliance which says (emphasis mine):

1. The set of diagnosable rules consists of all syntactic and semantic rules in this International Standard except for those rules containing an explicit notation that “no diagnostic is required” or which are described as resulting in “undefined behavior.”

2. Although this International Standard states only requirements on C++ implementations, those requirements are often easier to understand if they are phrased as requirements on programs, parts of programs, or execution of programs. Such requirements have the following meaning:

   • If a program contains no violations of the rules in this International Standard, a conforming implementation shall, within its resource limits, accept and correctly execute² that program.

   • If a program contains a violation of any diagnosable rule or an occurrence of a construct described in this Standard as “conditionally-supported” when the implementation does not support that construct, a conforming implementation shall issue at least one diagnostic message.

   • If a program contains a violation of a rule for which no diagnostic is required, this International Standard places no requirement on implementations with respect to that program.

No syntactic or semantic rules are violated by this program and so no diagnostic is required.

We have the following code:

if (some_condition) (p1, p2, color);
                    ^  ^   ^
                    1  2   3

1 is an expression-statement which is valid in this context for and if statement. We can see this by going to the grammar:

if ( condition ) statement

and:

statement:
  attribute-specifier-seqopt expression-statement

and:

expression-statement:
  expressionopt;

and:

primary-expression:
  ( expression )

Both 2 and 3 are the comma operator which will evaluate the left operand and discard the value and then evaluate the right operand again nothing invalid here.

So what does section 5.18 Comma operator says:

A pair of expressions separated by a comma is evaluated left-to-right; the left expression is a discarded value expression (Clause 5).⁸³

and a discarded value expression is covered in section 5 which says:

In some contexts, an expression only appears for its side effects. Such an expression is called a discarded-value expression.

So since the value of the result of the left hand expression is discarded then we must only care about the side-effect. In your specific case evaluating the variable has no other effect besides generating a value hence the warning, but if you had used a function in their place e.g.:

bool func()
{
    //...
}

and change your code to:

if (some_condition) (func(), func(), func() );

neither clang nor gcc will provide a warning since presumably func will perform some side-effet that you care about.