Bug 206637 – `FALLTHROUGH` macro isn't properly defined when building Objective-C files using Clang

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Template Framework

[Conrad Shultz]

In the non-C++ case, FALLTHROUGH is defined only when using GCC.

[Conrad Shultz]

<rdar://problem/58807878>

[Radar WebKit Bug Importer]

<rdar://problem/58820324>

[Conrad Shultz]

<rdar://problem/58807878>

[Conrad Shultz]

Created attachment 388506 [details]
Patch

[Darin Adler]

Comment on attachment 388506 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=388506&action=review

> Source/WTF/wtf/Compiler.h:207
> -#if COMPILER(GCC)
> +#if COMPILER(GCC) || (COMPILER(CLANG) && __has_attribute(fallthrough))

Why doesn’t the section above with __has_cpp_attribute work? I believe that’s what’s supposed to cover this.

[Darin Adler]

Comment on attachment 388506 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=388506&action=review

>> Source/WTF/wtf/Compiler.h:207
>> +#if COMPILER(GCC) || (COMPILER(CLANG) && __has_attribute(fallthrough))
> 
> Why doesn’t the section above with __has_cpp_attribute work? I believe that’s what’s supposed to cover this.

Oh, non-C++, I see

[Darin Adler]

Comment on attachment 388506 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=388506&action=review

>>> Source/WTF/wtf/Compiler.h:207
>>> +#if COMPILER(GCC) || (COMPILER(CLANG) && __has_attribute(fallthrough))
>> 
>> Why doesn’t the section above with __has_cpp_attribute work? I believe that’s what’s supposed to cover this.
> 
> Oh, non-C++, I see

Since all the versions of CLANG we compile with have this support, I suggest just writing:

    #if COMPILE(GCC_COMPATIBLE)

[Conrad Shultz]

(In reply to Darin Adler from comment #7)
> Comment on attachment 388506 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=388506&action=review
> 
> >>> Source/WTF/wtf/Compiler.h:207
> >>> +#if COMPILER(GCC) || (COMPILER(CLANG) && __has_attribute(fallthrough))
> >> 
> >> Why doesn’t the section above with __has_cpp_attribute work? I believe that’s what’s supposed to cover this.
> > 
> > Oh, non-C++, I see
> 
> Since all the versions of CLANG we compile with have this support, I suggest
> just writing:
> 
>     #if COMPILE(GCC_COMPATIBLE)

In my testing, I found this wasn't always true, so I'd prefer to leave the `__has_attribute` check.

Thanks for the review, will sort out what's going on with Windows before landing.

[Conrad Shultz]

Created attachment 389056 [details]
Patch

[Conrad Shultz]

Created attachment 389274 [details]
Patch

[WebKit Commit Bot]

The commit-queue encountered the following flaky tests while processing attachment 389274 [details]:

editing/spelling/spellcheck-async-remove-frame.html bug 158401 (authors: [e-mail], [e-mail], and [e-mail])
The commit-queue is continuing to process your patch.

[WebKit Commit Bot]

Comment on attachment 389274 [details]
Patch

Clearing flags on attachment: 389274

Committed r255467: <https://trac.webkit.org/changeset/255467>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.