[Mesa-dev] [PATCH] glsl: remove GCC construct from VECN macro

[Emil Velikov]

On 27 March 2018 at 00:08, Roland Scheidegger <sroland at vmware.com> wrote:
> Am 27.03.2018 um 00:44 schrieb Rob Clark:
>> On Mon, Mar 26, 2018 at 6:38 PM, Rob Clark <robdclark at gmail.com> wrote:
>>> On Mon, Mar 26, 2018 at 6:07 PM, Roland Scheidegger <sroland at vmware.com> wrote:
>>>> Am 26.03.2018 um 23:01 schrieb Rob Clark:
>>>>> correct me if I'm wrong, but I don't see how you could do the ## stuff
>>>>> to construct the built-in type name with templates.  So I think the
>>>>> options are tons of copy/pasta code (ie. what we had before) or
>>>>> macros.
>>>>>
>>>>> I agree that could just construct the whole method with the macro.. I
>>>>> like that better than changing VECN() macro to contain the return
>>>>> (which I was explicitly trying to avoid in the original version, since
>>>>> macros that contain return statements are evil)
>>>>
>>>> macros which don't compile are worse...
>>>> Albeit I'm not a fan of any kind of macros, but whatever compiles.
>>>
>>> fair enough, although tbh gcc is the compiler I have, so I tend not to
>>> realize when I introduce things that are gcc specific.  (And, well,
>>> the linux kernel uses constructs like these, so I tend to assume they
>>> are fair game.)
>>>
>>> Ian's suggestion to make the entire method body into the macro is the
>>> approach I like best (and even more negative diff-stat).. I'll send a
>>> patch in a few minutes.
>>
>> oh, and if there is a compiler flag that would disallow things that
>> intel or MS compiler doesn't like.. we should use that.  It would save
>> me from embarrassing myself ;-)
>>
>> (alternatively something that pro-actively pulls patches from list and
>> builds them with compilers that most mesa dev's don't have, along the
>> lines of what kbuild robot does, could work.. the original patch was
>> sitting on list for at least a week.. but I guess the compiler arg, if
>> it exists, is the easier solution.)
>
> I don't think there is such a flag - -Wpedantic should flag some usage
> of non-iso extensions but not all.
> I believe clang should warn about it unless you use -Wno-gnu (or maybe
> it would need -Wgnu).
>
Indeed there's seemingly no way (modulo pedantic) on GCC, while clang
has a plethora of -Wgnu options [1].
The one we need here is -Wgnu-statement-expression.

Although we should also add -Wmultistatement-macros - aka warn if
macro is not wrapped in do {} while (0)

Back on the patch in question - what's the consensus?
As-is, templates, another macro...

Thanks
Emil

[1] https://github.com/llvm-mirror/clang/blob/master/test/Sema/gnu-flags.c