Bug 189733

Summary: Rename WTF_COMPILER_GCC_OR_CLANG to WTF_COMPILER_GCC_COMPATIBLE
Product: WebKit Reporter: Fujii Hironori <Hironori.Fujii>
Component: Web Template FrameworkAssignee: Fujii Hironori <Hironori.Fujii>
Status: RESOLVED FIXED    
Severity: Normal CC: achristensen, benjamin, bfulgham, cdumez, cmarcelo, dbates, dino, ews-watchlist, keith_miller, kondapallykalyan, mark.lam, mcatanzaro, msaboff, pvollan, saam, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 171618    
Attachments:
Description Flags
Patch
none
Patch
none
Patch to land none

Description Fujii Hironori 2018-09-18 21:48:26 PDT
[Win][Clang] Can't compile IGNORE_CLANG_WARNINGS_BEGIN

While doing Bug 171618, following compilation error is reported.

> [772/6184] Building CXX object Source\WTF\wtf\CMakeFiles\WTF.dir\StackBounds.cpp.obj
> FAILED: Source/WTF/wtf/CMakeFiles/WTF.dir/StackBounds.cpp.obj
> C:\tools\llvm\bin\clang-cl.exe (...) -c ..\..\Source\WTF\wtf\StackBounds.cpp
> In file included from ..\..\Source\WTF\wtf\StackBounds.cpp:21:
> In file included from ..\..\Source\WTF\config.h:31:
> In file included from ..\..\Source\WTF\wtf/FastMalloc.h:26:
> In file included from ..\..\Source\WTF\wtf/StdLibExtras.h:33:
> ..\..\Source\WTF\wtf/Assertions.h(605,1):  error: C++ requires a type specifier for all declarations
> IGNORE_CLANG_WARNINGS_BEGIN("missing-noreturn")
> ^
> ..\..\Source\WTF\wtf/Compiler.h(434,46):  note: expanded from macro 'IGNORE_CLANG_WARNINGS_BEGIN'
> #define IGNORE_CLANG_WARNINGS_BEGIN(warning) IGNORE_WARNINGS_BEGIN_IMPL(clang, warning)
>                                              ^
> In file included from ..\..\Source\WTF\wtf\StackBounds.cpp:21:
> In file included from ..\..\Source\WTF\config.h:31:
> In file included from ..\..\Source\WTF\wtf/FastMalloc.h:26:
> In file included from ..\..\Source\WTF\wtf/StdLibExtras.h:33:
> ..\..\Source\WTF\wtf/Assertions.h(605,1):  error: use of undeclared identifier 'clang'
> ..\..\Source\WTF\wtf/Compiler.h(434,73):  note: expanded from macro 'IGNORE_CLANG_WARNINGS_BEGIN'
> #define IGNORE_CLANG_WARNINGS_BEGIN(warning) IGNORE_WARNINGS_BEGIN_IMPL(clang, warning)
>                                                                         ^
> In file included from ..\..\Source\WTF\wtf\StackBounds.cpp:21:
> In file included from ..\..\Source\WTF\config.h:31:
> In file included from ..\..\Source\WTF\wtf/FastMalloc.h:26:
> In file included from ..\..\Source\WTF\wtf/StdLibExtras.h:33:
> ..\..\Source\WTF\wtf/Assertions.h(605,48):  error: expected ';' after top level declarator
> IGNORE_CLANG_WARNINGS_BEGIN("missing-noreturn")
>                                                ^

See Also:
Bug 188996 – Add IGNORE_WARNING_.* macros
Comment 1 Fujii Hironori 2018-09-18 22:50:48 PDT
Created attachment 350094 [details]
Patch
Comment 2 Alex Christensen 2018-09-19 13:50:31 PDT
Comment on attachment 350094 [details]
Patch

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

> Source/WTF/ChangeLog:16
> +        * wtf/Compiler.h: Replaced "COMPILER(GCC_OR_CLANG)" with "COMPILER(GCC) || COMPILER(CLANG)" of IGNORE_WARNINGS_* macros.

Will we need to do this for other uses of GCC_OR_CLANG?
Comment 3 Fujii Hironori 2018-09-19 13:59:30 PDT
Comment on attachment 350094 [details]
Patch

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

>> Source/WTF/ChangeLog:16
>> +        * wtf/Compiler.h: Replaced "COMPILER(GCC_OR_CLANG)" with "COMPILER(GCC) || COMPILER(CLANG)" of IGNORE_WARNINGS_* macros.
> 
> Will we need to do this for other uses of GCC_OR_CLANG?

Just only this one because I want to enable GCC and Clang warning macros even for Clang for Windows.
Comment 4 Fujii Hironori 2018-09-20 15:49:40 PDT
R?
Comment 5 Michael Catanzaro 2018-09-20 20:48:29 PDT
It's messed up if COMPILER(GCC_OR_CLANG) returns false when using Clang.

I think we need to replace COMPILER(GCC_OR_CLANG) with something like COMPILER(GCC_COMPATIBLE).
Comment 6 Fujii Hironori 2018-09-21 02:57:15 PDT
(In reply to Michael Catanzaro from comment #5)
> It's messed up if COMPILER(GCC_OR_CLANG) returns false when using Clang.
> 
> I think we need to replace COMPILER(GCC_OR_CLANG) with something like
> COMPILER(GCC_COMPATIBLE).

It sounds good. I will do it.
Comment 7 Fujii Hironori 2018-09-21 03:17:40 PDT
Created attachment 350346 [details]
Patch
Comment 8 Michael Catanzaro 2018-09-21 04:23:53 PDT
LGTM.

I see the CMake COMPILER_IS_GCC_OR_CLANG is already true for Clang, so that's good.
Comment 9 Michael Catanzaro 2018-09-21 04:24:24 PDT
Comment on attachment 350346 [details]
Patch

Please wait one day before committing this is case anyone has other naming suggestions.
Comment 10 Fujii Hironori 2018-09-24 19:06:13 PDT
Created attachment 350728 [details]
Patch to land

Thank you for the review.
* Rebased onto Tot.
Comment 11 Fujii Hironori 2018-09-24 20:06:09 PDT
Comment on attachment 350728 [details]
Patch to land

Clearing flags on attachment: 350728

Committed r236450: <https://trac.webkit.org/changeset/236450>
Comment 12 Fujii Hironori 2018-09-24 20:06:13 PDT
All reviewed patches have been landed.  Closing bug.
Comment 13 Radar WebKit Bug Importer 2018-09-24 20:07:23 PDT
<rdar://problem/44750032>