Libguestfs - Feb 2022 - [PATCH nbdkit 1/2] common/include: Change unique-name macros to use __COUNTER__

On Tue, Feb 22, 2022 at 09:35:03PM +0000, Richard W.M. Jones
wrote:
> Previously the macros used __LINE__ which meant they created a unique
> name specific to the line on which the macro was expanded.  This
> worked to a limited degree for cases like:
> 
>   #define FOO \
>      ({ int NBDKIT_UNIQUE_NAME(foo) = 42; \
>         NBDKIT_UNIQUE_NAME(foo) * 2 })
Missing a ; after '* 2'.
> 
> since the ?FOO? macro is usually expanded at one place so both uses of
> NBDKIT_UNIQUE_NAME expanded to the same unique name.  It didn't work
> if FOO was used twice on the same line, eg:
> 
>   int i = FOO * FOO;
This didn't actually fail. The failure we saw was more subtle:

MIN (MIN (1, 2), 3)

compiled (the inner MIN() creates an _x evaluated in the context of
the initializer of the outer MIN's _x, which is not in scope until the
initializer completes), but:

MIN (1, MIN (2, 3))

failed to compile with -Werror -Wshadow, because now the inner MIN's
_x is declared inside the scope of the initializer of the outer MIN's
_y, when an outer _x is already in scope.
> 
> would fail, but this would work:
> 
>   int i = FOO *
>           FOO;
Or, back to the example we actually hit,

MIN (1,
     MIN (2, 3))

worked.
> 
> Use __COUNTER__ instead of __LINE__, but NBDKIT_UNIQUE_NAME must now
> be used differently.  The FOO macro above must be rewritten as:
> 
>   #define FOO FOO_1(NBDKIT_UNIQUE_NAME(foo))
>   #define FOO_1(foo) \
>      ({ int foo = 42; \
>         foo * 2 })
> 
> Thanks: Eric Blake
> ---
> +++ b/common/include/test-checked-overflow.c
> @@ -39,29 +39,25 @@
>  
>  #define TEST_MUL(a, b, result, expected_overflow, expected_result)      \
>    do {                                                                  \
> -    bool NBDKIT_UNIQUE_NAME(_actual_overflow);                          \
> +    bool actual_overflow;                                               \
>                                                                          \
> -    NBDKIT_UNIQUE_NAME(_actual_overflow) =                              \
> -      MUL_OVERFLOW_FALLBACK ((a), (b), (result));                       \
> -    assert (NBDKIT_UNIQUE_NAME(_actual_overflow) == (expected_overflow));
\
> +    actual_overflow =   MUL_OVERFLOW_FALLBACK ((a), (b), (result));     \
Extra spacing after 
The commit message may need touching up, but ACK to the change itself.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

On 02/23/22 00:05, Eric Blake wrote:
> On Tue, Feb 22, 2022 at 09:35:03PM +0000, Richard W.M. Jones wrote:
>> Previously the macros used __LINE__ which meant they created a unique
>> name specific to the line on which the macro was expanded.  This
>> worked to a limited degree for cases like:
>>
>>   #define FOO \
>>      ({ int NBDKIT_UNIQUE_NAME(foo) = 42; \
>>         NBDKIT_UNIQUE_NAME(foo) * 2 })
> 
> Missing a ; after '* 2'.
> 
>>
>> since the ?FOO? macro is usually expanded at one place so both uses of
>> NBDKIT_UNIQUE_NAME expanded to the same unique name.  It didn't
work
>> if FOO was used twice on the same line, eg:
>>
>>   int i = FOO * FOO;
> 
> This didn't actually fail. The failure we saw was more subtle:
> 
> MIN (MIN (1, 2), 3)
> 
> compiled (the inner MIN() creates an _x evaluated in the context of
> the initializer of the outer MIN's _x, which is not in scope until the
> initializer completes), but:
> 
> MIN (1, MIN (2, 3))
> 
> failed to compile with -Werror -Wshadow, because now the inner MIN's
> _x is declared inside the scope of the initializer of the outer MIN's
> _y, when an outer _x is already in scope.
I wish we could suppress -Wshadow in macro definitions, but I think
"#pragma GCC diagnostics" may not work that way (or is not available
on
all the necessary compilers) :/

for the series

Acked-by: Laszlo Ersek <lersek at redhat.com>

Laszlo
> 
>>
>> would fail, but this would work:
>>
>>   int i = FOO *
>>           FOO;
> 
> Or, back to the example we actually hit,
> 
> MIN (1,
>      MIN (2, 3))
> 
> worked.
> 
>>
>> Use __COUNTER__ instead of __LINE__, but NBDKIT_UNIQUE_NAME must now
>> be used differently.  The FOO macro above must be rewritten as:
>>
>>   #define FOO FOO_1(NBDKIT_UNIQUE_NAME(foo))
>>   #define FOO_1(foo) \
>>      ({ int foo = 42; \
>>         foo * 2 })
>>
>> Thanks: Eric Blake
>> ---
>> +++ b/common/include/test-checked-overflow.c
>> @@ -39,29 +39,25 @@
>>  
>>  #define TEST_MUL(a, b, result, expected_overflow, expected_result)    
\
>>    do {                                                                
\
>> -    bool NBDKIT_UNIQUE_NAME(_actual_overflow);                        
\
>> +    bool actual_overflow;                                             
\
>>                                                                        
\
>> -    NBDKIT_UNIQUE_NAME(_actual_overflow) =                            
\
>> -      MUL_OVERFLOW_FALLBACK ((a), (b), (result));                     
\
>> -    assert (NBDKIT_UNIQUE_NAME(_actual_overflow) ==
(expected_overflow)); \
>> +    actual_overflow =   MUL_OVERFLOW_FALLBACK ((a), (b), (result));   
\
> 
> Extra spacing after > 
> The commit message may need touching up, but ACK to the change itself.
>