I suspect we will want a whole lot more exceptions. C has lots of operators and almost nobody learns the precedence order. Perhaps this is a common sense rule, reduce parenthesis as far as is reasonable but no further?

E.g. a <= b == c >= d + 1 is a perfectly well defined C expression with minimal parenthesis but a lot of folks won't instantly know what it's doing.

I've extended the text and the example this way,
and now also explicitly cover the so-called bitwise operators &, ^, and |.

The whole approach of preventing "extra" parentheses is something I think should be removed completely - as mistakes do happen and parentheses do significantly improve readability.

@t-j-h I wholeheartedly disagree. For me a code with needless parentheses is much less readable actually. I always have to count the parentheses and so on to understand what is going on there. The example in this PR is a perfect one for me. The condition with the extra parentheses looks like parentheses hell to me where the one without is perfectly readable.

In most real life cases I'd agree with @t-j-h. I never remember the operators priority :(

Well, what to consider a reasonable amount of extra parentheses is a matter of both taste and fluency with C.
This could be debated ad nauseam.

If this ever becomes a problem in practice (in a code review) just update the coding style again when that happens?

This one surprised me 😕

So now, (a == 1 || b == 2) is badly seen? We have written that sort of expression all over the place until now...

This recent addition was motivated by #39 (comment).

That comment was intended as the kind of mess C operators can create. I don't think it's a great example to make a rule from, but I can live with it. I have seen code along the lines of a < b != c < d.

I think we need some flexibility in interpretation. Minimal parenthesis is an ideal but needs to be tempered but comprehension.

I think we need some flexibility in interpretation

I agree on principle, but have a hard time seeing the flexibility in "always"

I think it can be added after the minor policy edits is approved.

It would probably change the meaning...

The way I read it the sentence above is completely contradicting the immediately previous example.

I think it can be added after the minor policy edits is approved.

It would probably change the meaning...

No, the sentence @t8m proposed to add later:

I.E., do not depend on && having higher precedence than ||, <= having higher precedence than ==, and & having higher precedence than | or ^.

would just clarify the sentence before:

For clarity, always put parentheses when mixing && and || operations, comparison operators like <= and ==, and bitwise operators.

The way I read it the sentence above is completely contradicting the immediately previous example.

Which sentence do you mean here?

All the rules and examples about extra parentheses state exceptions on the most general rule stated upfront:

Avoid needless parentheses as far as reasonable.

In this sense, they are of course somewhat contradicting ;-)

I've understood it (and I think it was meant to be) as mixing operators in the same category only. I.E. this does not apply to a == b && c > d. But applies to a && b || c which should be written as (a && b) || c even if the former is equivalent according to C rules.

Upon re-reading it, I can understand the sentence like that. But it's not perfectly clear. I suspect that we might have disputes later on depending on interpretation.

Ah well

Where the number is intended to be a boolean value I think this is ok and is very common in the codebase (in particular when checking function return values)m e.g. if (!EVP_DecryptUpdate(...))

Sure, and therefore the rule is stated just about numbers (integers etc.), not about Boolean values.
Maybe this should be made more clear.

@mattcaswell it would have been good to receive such comments regarding clarifications earlier,
not after most people have already voted.

To clarify this, we could add (after the given examples) for instance:

Note that Boolean values shall be used directly as usual, e.g.,
if (check(x) && !success(y))

Looking at the sentence again, it wouldn't be very hard to make it clearer:

Hmm, when re-reading the above suggestion, which does include an improvement, I do not like the "category" and "such as" very much.
For instance, this could also be understood that one should put parentheses when mixing * and +, which is not intended.
So I propose a further refinement as follows:

For clarity, always put parentheses when mixing the Boolean && and || operations,
when mixing comparison operators like <= and ==, and when when mixing bitwise operators.

For clarity, always put parentheses when mixing the Boolean && and || operations, when mixing comparison operators like <= and ==, and when when mixing bitwise operators.

"Boolean" should be "Logical", to match C parlance.
The last "and" should probably be "or"
Drop the double "when"

With those mods, I approve

Oh, and "operations" should be "operators"

Yes, and I suggest including in the sentence also examples for the bitwise operators.
So:

For clarity, always put parentheses when mixing the logical && and || operators,
mixing comparison operators like <= and ==, or mixing bitwise operators like & and | .

Any changes will require a new vote -- too many have voted to change the wording now. We don't have a process to abort a vote without it failing.

I don't think there is a need for different failure modes. If the vote fails, it fails. But if the OTC members indicate that a revised version would be accepted, then discussion can be immediately restarted on a new PR.

@DDvO I agree with Richard: IMHO a PR which has been voted down should be closed. In cases like this one, were it just failed because changes were requested, it can immediately be replaced by a new PR from the same branch (or with some suffix like coding-style-expressions-r2, as you prefer).

True, at least when using the same branch, a new PR is very easily set up.
Yet a pointer to the previous PR should be given such that original discussion can be found quickly.

Having multiple votes within the same PR would make the thread history too confusing. But at this moment, that's just my personal opinion Your thoughts, @openssl/otc ?

That's a very good point in favor of a new PR.

The vote includes the hash. I don't see a big problem having a second vote here.
Another alternative would be an issue for the vote, keeping this PR open.

The current process IMO does not allow for second vote. If anyone wants to propose a change to it no problem. I suppose we would need to change it to always use a separate issue for the votes.

@paulidale @t8m this is actually a very good idea: we shouldn't carry out the vote in the PR, but always create a dedicated Issue for a vote, with a standardized subject line (VOTE: <subject>). In this way, we could decouple the PR from the vote. Nevertheless, we need to amend the policies to regulate things like a shortened waiting period, if desired.