otp icon indicating copy to clipboard operation
otp copied to clipboard

Make the compiler report 'and'/'or' operators as obsolete

Open richcarl opened this issue 1 year ago • 22 comments

The strict 'and'/'or' operators are mostly useless, confusing for newcomers, rarely used in practice, and when used they can always be replaced with andalso/orelse. I think they should be obsoleted and subsequently dropped from the language.

richcarl avatar Nov 26 '24 13:11 richcarl

CT Test Results

    23 files     909 suites   6h 57m 45s ⏱️ 11 759 tests 11 069 ✅   690 💤 0 ❌ 25 377 runs  23 517 ✅ 1 860 💤 0 ❌

Results for commit 054c362d.

:recycle: This comment has been updated with latest results.

To speed up review, make sure that you have read Contributing to Erlang/OTP and that all checks pass.

See the TESTING and DEVELOPMENT HowTo guides for details about how to run test locally.

Artifacts

// Erlang/OTP Github Action Bot

github-actions[bot] avatar Nov 26 '24 13:11 github-actions[bot]

Should and and or be made to work the same as andalso and orelse in the future (after deprecation, removal, reintroduction, any other necessary steps, perhaps over a span of 5-10 years) so that we can shorten a bit our function clauses?

essen avatar Nov 27 '24 11:11 essen

Should and and or be made to work the same as andalso and orelse in the future (after deprecation, removal, reintroduction, any other necessary steps, perhaps over a span of 5-10 years) so that we can shorten a bit our function clauses?

Nah, I thought I'd try to introduce && and || instead. :trollface:

Honestly I'm not sure. I think there's a bit of pedagogical value and visibility in the andalso/orelse keywords. (These names go back to Standard ML, if anyone wonders.) As I was replacing the occurrences of and/or in the code, I saw that a tiny 'or' or 'and' was easy to miss, but 'orelse' and 'andalso' stood out much better. So it might not even be worth doing, even if it saves horizontal space. We can submit a followup EEP in 2030 and see what people think then. :stuck_out_tongue_winking_eye:

Btw, replacing existing uses showed mainly two categories: 1) valid ways of writing more complex guards or other nested logical expressions (not all that common, in comparison to the total size of the codebase), and 2) as a personal style thing, where you could just as well have used comma and semicolon in a guard instead (only found in a few isolated modules) , possibly due to a lack of understanding of guard syntax. Also, a some of these occurrences probably go back to before semicolon was added to guards.

richcarl avatar Nov 27 '24 12:11 richcarl

I saw that a tiny 'or' or 'and' was easy to miss, but 'orelse' and 'andalso' stood out much better.

Indeed. I just tried finding them in RabbitMQ to fix them and it is clearly non-obvious. The other two have this advantage.

essen avatar Nov 27 '24 12:11 essen

@essen I did however come up with another use for 'or' (doesn't strictly require removing the original). Here's a very small proof of concept: https://github.com/erlang/otp/compare/master...richcarl:otp:or-patterns - e.g. case X of {a, Y} or {b, Y} or {c, Y} -> {yes, Y}; _ -> no end.

richcarl avatar Dec 03 '24 21:12 richcarl

case X of {a, Y} or {b, Y} or {c, Y} -> {yes, Y}; _ -> no end.

That would definitely remove a frequent pain point with regard to case clauses! :+1:

essen avatar Dec 04 '24 08:12 essen

That would definitely remove a frequent pain point with regard to case clauses! 👍

Yes. It might be technically possible to use ; as separator, but I'm afraid it would get too unreadable. For example, foo({a, Y}; {b, Y}, {c, Y}) when Y >= $0, Y =< $9; Y =:= $@ -> {yes, Y}; _ -> no end. has the problem that ; binds tighter than , in the patterns, but the other way around in the guard. foo({a, Y} or {b, Y}, {c, Y}) when Y >= $0, Y =< $9; Y =:= $@ -> {yes, Y}; _ -> no end. is easier to read.

richcarl avatar Dec 04 '24 11:12 richcarl

Updated like @bjorng suggested. Definitely yields nicer code, but a bit harder to review.

richcarl avatar Dec 11 '24 21:12 richcarl

Fixed , instead of ; in megaco that failed the type check.

richcarl avatar Dec 12 '24 14:12 richcarl

The windows build died for some reason: /mnt/d/a/_temp/87953a19-9c2f-4c2d-bffc-af974820c086: line 1: 284 Segmentation fault apt install -y g++-mingw-w64 gcc-mingw-w64 make autoconf unzip.

richcarl avatar Dec 16 '24 07:12 richcarl

The windows build died for some reason: /mnt/d/a/_temp/87953a19-9c2f-4c2d-bffc-af974820c086: line 1: 284 Segmentation fault apt install -y g++-mingw-w64 gcc-mingw-w64 make autoconf unzip.

I re-triggered the windows run and now it seems to have passed.

garazdawi avatar Dec 16 '24 09:12 garazdawi

and when used they can always be replaced with andalso/orelse

False. Sometimes one is after the side effects of an expression that returns a boolean, e.g.: [edited] This code runs several actions and returns true when any of them detects a change. orelse would totally wreak this logic.

confusing for newcomers

Please stop using this excuse to cripple a language: virtually anything can be confusing for a newcomer, depending on their background. Imagine the bewilderment of someone who switches from COBOL.

ieQu1 avatar Feb 08 '25 19:02 ieQu1

and when used they can always be replaced with andalso/orelse

False. Sometimes one is after the side effects of an expression that returns a boolean, e.g.: https://github.com/k32/anvl/blob/master/src/anvl_erlc.erl#L158

This is typically written over multiple lines, with the result computed from the intermediate results like

R1 = precondition(...),
R2 = [...]
R3 = [...]
R4 = [...]
R1 orelse R2 orelse R3 orelse R4

It's more verbose but also a lot clearer since you don't need to know the semantics of or or orelse to understand what is going on.

essen avatar Feb 08 '25 19:02 essen

It's more verbose but also a lot clearer since you don't need to know the semantics of or or orelse to understand what is going on.

We can go further: orelse is utterly redundant (just as if, case, variables, function names, ...), and nobody should be forced to learn its semantics, because ι-combinator alone is Turing-complete.

ieQu1 avatar Feb 08 '25 20:02 ieQu1

We can go further: orelse is utterly redundant (just as if, case, variables, function names, ...), and one shouldn't have to learn its semantics, because ι-combinator alone is Turing-complete.

Let's not exaggerate, the problem is that there's two or and and, where there should be one.

Some people argue that if and case are in a similar boat and if should be removed.

I don't think I've seen calls to remove anything else.

essen avatar Feb 08 '25 20:02 essen

Let's not exaggerate

I simply explained why "more verbose but also a lot clearer" is flawed argument, because syntactic reductionism leads there: [edited] Sure, I can rewrite any code to avoid any syntax construct and semantic overhead.

the problem is that there's two or and and

There's no such problem. There are two pairs of operators with different semantics, just like & and && in C.

These operators have existed in the language since time immemorial, removing them won't make any code clearer, it will just litter the existing code with compiler pragmas: or is clear code, -compile(nowarn_or_and_and). isn't.

ieQu1 avatar Feb 08 '25 20:02 ieQu1

P.S. I speak on behalf of a small and unimportant group of people: those who're deeply invested in large and/or complex Erlang projects. There's already a growing problem of abandoned dependencies that we have to fork and maintain due to bit-rot. Such arbitrary changes to the language driven by subjective arguments will just waste more of our resources that could be invested into building new things.

ieQu1 avatar Feb 09 '25 12:02 ieQu1

syntactic reductionism

That is not what is happening. Some syntax gets added, some gets removed. We have to remove some things in order to improve the language.

P.S. I speak on behalf of a small and unimportant group of people: those who're deeply invested in large and/or complex Erlang projects. There's already a growing problem of abandoned dependencies that we have to fork and maintain due to bit-rot. Such arbitrary changes to the language driven by subjective arguments will just waste more of our resources that could be invested into building new things.

Me too? The removal of or/and has a very small maintenance impact. There's been far more complex removals such as parameterised modules.

essen avatar Feb 09 '25 12:02 essen

Friendly reminder that a compiler warning is just a warning, and that we still retain obsolete things like atom/1 (obsoleted by is_atom/1), functions like queue:lait/1 (misspelled version of queue:liat/1), the aforementioned parameterized modules (now handled by the compiler rather than the runtime), et cetera.

As such, we are not going to drop and/or from the language. Warnings are nothing more than recommendations from the OTP team, and the PR asks us to recommend against the use of and/or. I am personally inclined to do so: the only reason to use these over andalso/orelse is to evaluate all arguments for effect, however, the evaluation order of these operators is undefined so a small change in the compiler can cause code to stop working when there's an implicit dependency between the arguments.

If the PR is accepted, you're free to disable the warning project-wide because it's fine not to follow our recommendations. Again, warnings are not an indication that we are about to remove something.

jhogberg avatar Feb 10 '25 09:02 jhogberg

There's one more issue with and and or that hasn't been mentioned here and that is their quite surprising precedence. X > 3 or is_tuple(X) is parsed as X > (3 or is_tuple(X)) - normally this would result in an exception, but in a guard this causes a silent failure and can lead to hours of debugging.

michalmuskala avatar Feb 10 '25 12:02 michalmuskala

As @Maria-12648430 pointed out in a similar PR, there is another difference between and/or and andalso/orelse. and/or will raise an error if any of the operands is a non-boolean. andalso/orelse will only raise an error if the left-hand operand is a non-boolean, and return whatever the right-hand operand is if the left-hand operand is true (for andalso) or false (for orelse).

This means that sth like Foo = false or x will throw an error, but work and result in Foo = x when turned into orelse (similar for and/andalso). And a guard like when true or x will fail, but succeed when turned into orelse.

Thus, simply replacing all ands/ors with andalso/orelse could lead to subtle changes in behavior, things silently starting to work that didn't work before.

juhlig avatar Aug 25 '25 12:08 juhlig

Changed to off by default and not replacing any uses in the code, just enabling the flag in the apps where they occur and adding nowarn-flags to specific modules.

richcarl avatar Nov 27 '25 11:11 richcarl