Date:   Fri, 5 May 2023 14:49:57 -0400
From:   Mathieu Desnoyers <mathieu.desnoyers@...icios.com>
To:     Linus Torvalds <torvalds@...ux-foundation.org>
Cc:     Andrew Morton <akpm@...ux-foundation.org>,
        linux-kernel@...r.kernel.org, Jens Axboe <axboe@...nel.dk>,
        linux-block@...r.kernel.org
Subject: Re: [RFC PATCH 12/13] blk-mq.h: Fix parentheses around macro
 parameter use

On 2023-05-05 14:40, Linus Torvalds wrote:
> On Fri, May 5, 2023 at 6:56 AM Mathieu Desnoyers
> <mathieu.desnoyers@...icios.com> wrote:
>>
>> Which way do we want to go with respect to the rvalue of the assignment
>> operator "=" in a macro ? (with or without parentheses)
>>
>> In short:
>>
>> #define m(x) do { z = (x); } while (0)
>>
>> or
>>
>> #define m(x) do { z = x; } while (0)
> 
> I suspect that the first one is preferred, just as a "don't even have
> to think about it" thing.
> 
> In general, despite my suggestion of maybe using upper-case to show
> odd syntax (and I may have suggested it, but I really don't like how
> it looks, so I'm not at all convinced it's a good idea), to a
> first-order approximation the rule should be:
> 
>   - always use parentheses around macros
> 
>   - EXCEPT:
>       - when used purely as arguments to functions or other macros
>       - when there is some syntax reason why it's not ok to add parens

I would add to this list of exceptions cases where the argument is used 
as an expression within brackets, e.g.

#define m(x) myvar[x]

Because the content within the brackets is already an expression.

The other exception I would add is when a parameter is used as an 
lvalue, as:

#define m(x) do { x = 2; } while (0)

because I cannot find a case where it would cause unexpected operator 
precedence.

Are you OK with those 2 additional exceptions ?

> 
> The "arguments to functions/macros" is because the comma separator
> between arguments isn't even a operator (ie it is *not* a
> comma-expression, it's multiple expressions separated by commas).
> There is no "operator precedence" subtlety.

Good point.

> 
> So we have a lot of macros that are just wrappers around functions (or
> other macros), and in that situation you do *not* then add more
> parentheses, and doing something like
> 
>      #define update_screen(x) redraw_screen(x, 0)
> 
> is fine, and might even be preferred syntax because putting
> parentheses around 'x' not only doesn't buy you anything, but just
> makes things uglier.
> 
> And the "syntax reasons" can be due to the usual things: we not only
> have that 'pass member name around' issue, but we have things like
> string expansion etc, where adding parentheses anywhere to things like
> 
>      #define __stringify_1(x...)     #x
>      #define __stringify(x...)       __stringify_1(x)
> 
> would obviously simply not work (or look at our "SYSCALL_DEFINEx()"
> games for more complex examples with many layers of token pasting
> etc).
> 
> But in general I would suggest against "this is the lowest priority
> operator" kind of games. Nobody remembers the exact operator
> precedence so well that they don't have to think about it.
> 
> So for example, we have
> 
>      #define scr_writew(val, addr) (*(addr) = (val))
> 
> to pick another VT example, and I think that's right both for 'addr'
> (that requires the parentheses) and for 'val' (that might not require
> it, but let's not make people think about it).

Indeed, brain power and reviewer time is a scarce resource. It's a shame 
to waste it on figuring out operator priority again and again.

Thanks,

Mathieu


-- 
Mathieu Desnoyers
EfficiOS Inc.
https://www.efficios.com

Powered by blists - more mailing lists