[PATCH] D17444: PR26672: [MSVC] Clang does not recognize "static_assert" keyword in C mode

[Richard Smith via cfe-commits]

On Thu, Mar 24, 2016 at 11:29 AM, Aaron Ballman <aaron at aaronballman.com> wrote:
> On Thu, Mar 24, 2016 at 12:23 PM, David Majnemer
> <david.majnemer at gmail.com> wrote:
>>
>>
>> On Thu, Mar 24, 2016 at 9:09 AM, Aaron Ballman <aaron at aaronballman.com>
>> wrote:
>>>
>>> On Thu, Mar 24, 2016 at 11:49 AM, Reid Kleckner <rnk at google.com> wrote:
>>> > On Thu, Mar 3, 2016 at 10:40 AM, Aaron Ballman <aaron at aaronballman.com>
>>> > wrote:
>>> >>
>>> >> That was what I meant by "justification". I would say it has to be
>>> >> reasonably compelling code (win32 headers, boost, some other major
>>> >> library) as that's our usual bar for these sort of bug-for-bug
>>> >> compatible things, as I understand it.
>>> >
>>> >
>>> > I'd rather apply this patch now than wait for ffmpeg or someone to try
>>> > to
>>> > use static_assert and then have to hustle to get this into clang. Many
>>> > many
>>> > C projects have COMPILE_ASSERT macros that are just a small change away
>>> > from
>>> > relying on (_S|s)tatic_assert.
>>>
>>> I don't find "someone might rely on this bug" to be compelling. Put
>>> another way, why do we lower the bar for this bug but not others given
>>> that no large projects appear to need this behavior?
>>
>>
>> Very few people are using clang-cl with C code.  I'm sure it would come up a
>> lot more often if it were better tested.
>
> Fair, but again, if people aren't hitting it, it's not a problem (yet). ;-)
>
>>
>>>
>>>
>>> >>
>>> >> Agreed, we have a way forward if we need it. I mostly just want to
>>> >> avoid the burden of supporting that because this is sufficiently weird
>>> >> (being a non-conforming keyword).
>>> >
>>> >
>>> > It's not conforming, but it's not that weird to define our own keywords.
>>> > The
>>> > C++ committee chose the keyword "static_assert" because it was unlikely
>>> > to
>>> > conflict with existing code. MSVC has made this a keyword in C mode and
>>> > the
>>> > world hasn't burned.
>>>
>>> Correct, we have a way around it, I am just not convinced that we
>>> should put forth the effort of supporting another compiler's bug
>>> without a compelling use-case.
>>
>>
>> A compelling use case is that users who wish to use static_assert in a
>> conforming C program can do so.
>> Today, it is impossible to do this with clang-cl & MSVC's CRT.

If someone is trying to use clang and they hit this problem, they can
add -Dstatic_assert=_Static_assert to their command line -- or they
can fix the code to not rely on non-standard features (or, as the MS
engineer suggested, compiler bugs).

> Thank you for this explanation, it finally triggered enough of my
> brain cells to explain the desire for working around this MSVC bug.
>
> I think the keyword approach is better than interposing on assert.h in
> terms of maintenance, but I would like to see use of static_assert as
> a compatibility keyword be ill-formed if the user does not include
> assert.h. This gives users a way forward, but helps direct them
> towards conforming uses of static_assert so that when MSVC does fix it
> and we stop enabling the compatibility mode for it, their code
> continues to just work. This isn't 100% compatibility with MSVC (since
> they don't require you to include assert.h), but I feel it's
> reasonable to tell users to include assert.h to get the behavior they
> desire (which continues to work with cl or clang-cl).
>
> Thoughts?

I don't think it's a significant maintenance problem to interpose
assert.h; we do that for several other libc headers. That seems a lot
simpler than trying to track if assert.h was ever included (especially
across PCH / modules).

I'm unconvinced we need any change here. In my searching, I've only
found one case of someone trying to use this MSVC extension in
widespread / popular open source code:

  https://bugzilla.mozilla.org/show_bug.cgi?id=713531

... and they wrote code that would have worked fine for clang-cl
regardless (and ended up not using the MSVC static_assert anyway).