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

Rob Clark robdclark at gmail.com
Tue Mar 27 18:58:40 UTC 2018


On Tue, Mar 27, 2018 at 2:40 PM, Emil Velikov <emil.l.velikov at gmail.com> wrote:
> 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...

I've pushed a fix this morning

BR,
-R

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


More information about the mesa-dev mailing list