Warnings: Lack of parenthesis in conditional if statement when mixing AND and OR?

Hi!

I'm relatively new to Rust (but not to programming). I was writing the following code (simplified):

    let a = true;
    let b = true;
    let c = false;

    if a && b || c {
        println!("Hello");
    }

And, coming from C++, I noticed Rust didn't issue any warning. And I was like "Wait, what???".

(a && b) || c is not the same as a && (b || c) . Most C++ compilers nowadays will warn when encountering this dangerously confusing construct without paranthesis.

Am I missing something? Should I be using a different syntax? Is this in rustc's todo list?

Cheers

3 Likes

It is a well-known convention in mathematics that conjunction has higher precedence than disjunction. There is no need for a warning here, unless you also believe that a * b + c should generate a diagnostic, too.

19 Likes

Well, I did four years of university level mathematics, and when I looked at the code above, I couldn't immediately remember which way to parse it. In hindsight, I just compulsively parenthesise expressions like this for exactly that reason.

rustc is known for its excellent diagnostics, many of which exist because of an acceptance that programmers are (at least for the moment) human, and humans are fallible. Likewise, C++ compilers are notorious for responding to even minor mistakes with fire and brimstone and hanging your terminal while it tries to output seven thousand pages of template errors.

It the mistake is common enough for C++ compilers to be showing some compassion, maybe rustc should consider it, too.

(Also, arithmetic precedence is taught in primary school, logical precedence is first year university. They are not really that comparable.)

14 Likes

Clippy has a clippy::precedence lint that covers some similar cases, and there are some open issues about extending it to additional cases. I didn’t find any issue covering this use case, so it is probably worth filing one.

13 Likes

They are, because conjunction and disjunction are in direct correspondence with multiplication and addition (e.g., with false meaning 0 and true meaning "strictly positive").

C++ compilers are notoriously not a great source of inspiration as to what to warn about by default. Most C++ compilers don't generate a whole lot of diagnostics about real issues unless you pass them the equivalent of -Wall -Wextra, and then they start warning about absolutely nit-picky nonsense.

This is absolutely not something we should be aiming for. It's bad UX and counter-productive (you'll end up ignoring the error messages and just doing trial and error until it compiles). A higher volume of diagnostics doesn't directly correspond to better code.

3 Likes

In C# it is much worse. It takes orders which are quite impossible to predict. For example with shift operators.
I think brackets should be a personal preference. There are already enough warnings in my opinion.
What I am wondering what the precedence of "as" is, for example with ints.

Ok, touchy subject.

In my experience around 30% of people will believe one form was intended, while 70% will believe the other one.

It is a well-known convention in mathematics that conjunction has higher precedence than disjunction. There is no need for a warning here, unless you also believe that a * b + c should generate a diagnostic, too.

Aside what DanielKeep said, they're not even remotely comparable.

For arithmetic operations, most values of a, b and c the result will very likely result in the wrong value which is easy to spot.

However in the case of logical operations, many combinations of a, b and c will still result in taking the intended path but it is still broken code.

Thorough unit tests can help identify those broken paths but it still very difficult to test because for many inputs the code will produce the correct results and hit all the correct paths.

It is a well-known convention in mathematics that conjunction has higher precedence than disjunction

That is not a valid argument. It is also very well-known that in C one should always initialize and allocate memory, and free it when one stops using it, and never reuse that memory again. Yet that doesn't stop C developers from making mistakes.

This is absolutely not something we should be aiming for. It's bad UX and counter-productive (you'll end up ignoring the error messages and just doing trial and error until it compiles). A higher volume of diagnostics doesn't directly correspond to better code.

Rust aims towards writing safe code, extensive error validation, and getting rid of risky undefined behavior (like the dreaded integer overflow in C).

So I found it odd that it would say nothing (no error, no warning) when encountering a || b && c which is a very common source of bugs.

Besides, Rust warns about things like using the wrong style convention (e.g. snake_case), unused variables and variables that don't need to be mutable. It's already very verbose (I'm not complaining).

9 Likes

@mbrubeck: Thanks. I didn't know about Clippy. Looks useful.

I will check it out!

1 Like

I agree that operator precedence for || and && usually isn't taught.

The way I remember is to look at regexes:

abc|def

Means (a and b and c) or (d and e and f) and not a and b and (c or d) and e and f.

5 Likes

I feel cognitive dissonance when the compiler deliberately defines certain precedence and then warns about using it. If it's bad form to assume certain precedence, then the language shouldn't define such precedence in the first place.

The best explanation I can conjure up for gcc's warning is that they decided that, in retrospect, defining that precedence was a mistake and it's there for legacy reasons.

For this reason I find it wrong that the clippy warning mentioned above is warn by default.

16 Likes

Please refrain from making attacks against strawmen. Operator precedence is much easier than memory management:

  • it's well-known in the sense that it's defined by widespread convention, and just that. There's nothing more to it, and there's neither serious disagreement nor any room for "good practices" or anything like that. Conjunction binds tighter than disjunction, end of story. There's nothing more to it.
  • Getting the precedence right in a Boolean expression literally has one bit of entropy. Getting management of arbitrary memory in the presence of indirection, questions of ownership, shared state, and concurrency, is a much harder problem. Based on a recent statistic by Google, around 70% of reported CVEs stem from memory management errors. Getting and and or mixed up is down in the noise.
1 Like

Hahaha probably. I remember searching for a bug for 2 full days because of a 'not' in Delphi without brackets.

1 Like

That's obviously not what was meant. They're no way comparable in familiarity. Everybody knows that multiplication comes before addition. But there are countless programmers who have never taken a single university-level logic or algebra course or in any other way learned about "conjunction" and "disjunction" and how they're similar to multiplication and addition. And out of those who have, not nearly all remember about it or make the connection to programming. And you don't even necessarily learn about the correspondence on a basic CS curriculum logic course either unless you're able to connect the dots yourself. And because (∧, ∨, ¬, ⊥, ⊤) is in fact not a ring, it's likely not mentioned on an abstract algebra course either.

Clippy should absolutely include && || (and & |) in the precedence lint, and I'm actually quite surprised that it doesn't already.

12 Likes

I don't know about most people, but for me I don't think there should ever be warnings about && and || operator precedence like in the first example. I always thought that boolean algebra is a very, very basic, elementary thing and hearing that people get this wrong honestly surprises me.

I remember learning about the not, and, or boolean operators in Python quite a few years ago, right when I started programming and I'm pretty sure I learned those on the second or third day. Since then I never had a problem with remembering three words in this order. :slight_smile:

I also remember one case when I was writing C++ code, when I stumbled upon that warning, getting annoyed that I had to put some technically useless parentheses just to get rid of the warning.

5 Likes

If that's so “basic and elementary” then teel us which way a && b | c && d binds and why?

I think that's where people become confused — both in C/C++ and Rust.

But yes, I work on codebase where that warning is hard error in C/C++ project and I hate it.

1 Like

Sorry, I don't understand what you mean by "which way it binds". And as for the expression, I'm not sure why are you mixing bitwise or with logical and. If that is intentional, then I honestly don't know what the operator precedence is in this specific case, as I don't know the operator precedence rules by heart in C++ or Rust.

Instead I'll point out that it's a bit dangerous IMO to mix operators like that. If I were to write such code, then I'll probably put parentheses even if not necessary. What I was saying earlier is that the precedence between logical operators is elementary and that is what every programmer/engineer should know about. And this whole discussion is about logical operators.

I have another story for you. :sweat_smile:
I remember in my first year of university that during an exam I wrote some C++ code that had an expression with some addition/multiplication operators mixed with the modulo operator. I think I assumed that modulo had higher precedence than multiplication or something like that and I didn't test my code enough to see that it was actually incorrect.

And so I could have gotten maximum grade, if I didn't just assume things. My takeaway from this is to not assume and check the rules if in doubt. Maybe with time you'll need neither the rules cheat sheet, or the redundant parentheses anymore.

I just noticed that I'm slightly contradicting myself (it's very late in my time). Yes, my first instinct would be to put parentheses when in doubt, but the better option is to take the time (30 seconds) to lookup on the precedence table.

Actually it's better to use parentheses. If you don't know the precedence rules, chances are good that neither will the next reader of your code (or realistically, your future self :sweat_smile:). If you use parentheses, it's obvious at a glance what your intention is, and easily avoids any potential bugs caused by someone not knowing Rust's operator precedence rules.

2 Likes

Optimization, obviously. To make sure compiler wouldn't add jumps where you don't need them. In ideal world that wouldn't be necessary but our word is not ideal

Although in practice a & b || c & d is more common and it works as most people expect.

Which is 100% bizzare.

Yes. But according to C/C++ compilers that is something obvious and simple. Look for yourself:

  1. a && b || c && d — that's confusing, add parentheses there pronto!
  2. a && b | c && d — that's not confusing at all, everyone knows hot that works by heart!

I kinda assumed that this warning originated from that kind of confusion (which you probably would encounter in old C programs with hand-optimized code more often than in modern Rust world) and then logical operators were added to the mix… but nope, you are right: that is not a problem, according to the C/C++ compilers. Ugh.

2 Likes

It's clear to me: bitwise & and | are normally for bitmask arithmetic and so bind stronger than comparisons, so that I can write things like bitmask & 1 << bit == 0, whereas logical && and || are normally for logic and thus bind weaker than comparisons so that I can write things like a == 5 && b == 7.