[CMake] Properly test if compiler supports compiler flags

https://bugs.webkit.org/show_bug.cgi?id=174490

Reviewed by Konstantin Tokarev.

.:

This turned out to be a massive pain. I didn't want to merely check options before using
them: I also wanted to organize the code to avoid setting similar flags in different places.
Right now we set a bunch of global flags in OptionsCommon.cmake, and a bunch more flags in
WEBKIT_SET_EXTRA_COMPILER_FLAGS on a per-target basis.

Setting flags per-target seems better in general, e.g. because it makes it very easy to
disable warnings for particular ThirdParty targets. But it turns out that all the flags set
on a per-target basis get passed to both the C compiler and the C++ compiler, so it's
impossible to pass C++-only flags there. That's terrible. It's possible to make the flags
language-conditional using generator expressions, but that doesn't work for the Visual
Studio backend, so we would have to drop support for that (not going to happen). The CMake
documentation suggests that C and C++ files ought to be built in separate targets to avoid
this. It's a mess, basically.

So I've wound up removing WEBKIT_SET_EXTRA_COMPILER_FLAGS and adding most of those flags to
CMAKE_C_FLAGS and CMAKE_CXX_FLAGS instead. Really the only disadvantage of this is we now
have to suppress individual warnings when building ANGLESupport in WebCore. That's not the
end of the world. The only remaining useful feature of WEBKIT_SET_EXTRA_COMPILER_FLAGS was
to add -fPIC to static library targets, but turns out CMake does that for us if we just set
the variable CMAKE_POSITION_INDEPENDENT_CODE, so we can get rid of it completely.

Of course there are also macros for setting target-specific compiler flags, which we
frequently need in order to suppress specific warnings, particularly warnings coming from
third-party libraries like ANGLE and gtest. But remember the footgun: these macros will test
the flag against only one compiler, but must work with both C and C++ compilers unless the
build target exclusively contains targets built with just one of those compilers. Yuck.

* CMakeLists.txt:
* Source/CMakeLists.txt:
* Source/PlatformGTK.cmake:
* Source/cmake/OptionsCommon.cmake:
* Source/cmake/WebKitCommon.cmake:
* Source/cmake/WebKitCompilerFlags.cmake: Added.
* Source/cmake/WebKitMacros.cmake:

Source/JavaScriptCore:

* API/tests/PingPongStackOverflowTest.cpp:
(testPingPongStackOverflow):
* API/tests/testapi.c:
* b3/testb3.cpp:
(JSC::B3::testPatchpointLotsOfLateAnys):

Source/ThirdParty:

* brotli/CMakeLists.txt:
* gtest/CMakeLists.txt:
* woff2/CMakeLists.txt:
* xdgmime/CMakeLists.txt:

Source/WebCore:

* CMakeLists.txt:
* PlatformGTK.cmake:
* PlatformWPE.cmake:

Source/WebDriver:

* WebDriverService.cpp:
(WebDriver::WebDriverService::run):
* glib/SessionHostGlib.cpp:

Source/WebKit:

* CMakeLists.txt:
* PlatformGTK.cmake:

Source/WTF:

* wtf/Compiler.h:

Tools:

* DumpRenderTree/TestNetscapePlugIn/CMakeLists.txt:
* MiniBrowser/gtk/CMakeLists.txt:
* TestRunnerShared/Bindings/JSWrapper.cpp:
(WTR::JSWrapper::initialize):
* TestWebKitAPI/CMakeLists.txt:
* TestWebKitAPI/PlatformGTK.cmake:
* TestWebKitAPI/Tests/WTF/CheckedArithmeticOperations.cpp:
(TestWebKitAPI::CheckedArithmeticTester::run):
* TestWebKitAPI/Tests/WebKitGLib/TestAutomationSession.cpp:
* TestWebKitAPI/Tests/WebKitGLib/TestWebExtensions.cpp:
* TestWebKitAPI/Tests/WebKitGLib/WebExtensionTest.cpp:
(formControlsAssociatedCallback):
* TestWebKitAPI/glib/CMakeLists.txt:
* TestWebKitAPI/glib/WebKitGLib/TestMain.h:
(Test::getResourcesDir):
* WebKitTestRunner/CMakeLists.txt:
* WebKitTestRunner/InjectedBundle/EventSendingController.cpp:
(WTR::menuItemClickCallback):
(WTR::staticConvertMenuItemToType):
* WebKitTestRunner/InjectedBundle/TestRunner.cpp:
(WTR::TestRunner::setUseDashboardCompatibilityMode):
* WebKitTestRunner/InjectedBundle/atk/AccessibilityNotificationHandlerAtk.cpp:
(WTR::AccessibilityNotificationHandler::disconnectAccessibilityCallbacks):
* WebKitTestRunner/InjectedBundle/atk/AccessibilityUIElementAtk.cpp:
(WTR::AccessibilityUIElement::helpText const):
(WTR::AccessibilityUIElement::attributedStringForRange):
* WebKitTestRunner/gtk/EventSenderProxyGtk.cpp:
(WTR::EventSenderProxy::updateTouchPoint):
(WTR::EventSenderProxy::releaseTouchPoint):

git-svn-id: http://svn.webkit.org/repository/webkit/trunk@220403 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Author: [email protected]

CMakeLists.txt

@@ -126,6 +126,8 @@ set(WebKit_LIBRARY_TYPE SHARED)
 set(WebKit2_LIBRARY_TYPE SHARED)
 set(WebCoreTestSupport_LIBRARY_TYPE STATIC)
 
+set(CMAKE_POSITION_INDEPENDENT_CODE True)
+
 # -----------------------------------------------------------------------------
 # Install JavaScript shell
 # -----------------------------------------------------------------------------

ChangeLog

@@ -1,3 +1,45 @@
+2017-08-08  Michael Catanzaro  <[email protected]>
+
+        [CMake] Properly test if compiler supports compiler flags
+        https://bugs.webkit.org/show_bug.cgi?id=174490
+
+        Reviewed by Konstantin Tokarev.
+
+        This turned out to be a massive pain. I didn't want to merely check options before using
+        them: I also wanted to organize the code to avoid setting similar flags in different places.
+        Right now we set a bunch of global flags in OptionsCommon.cmake, and a bunch more flags in
+        WEBKIT_SET_EXTRA_COMPILER_FLAGS on a per-target basis.
+
+        Setting flags per-target seems better in general, e.g. because it makes it very easy to
+        disable warnings for particular ThirdParty targets. But it turns out that all the flags set
+        on a per-target basis get passed to both the C compiler and the C++ compiler, so it's
+        impossible to pass C++-only flags there. That's terrible. It's possible to make the flags
+        language-conditional using generator expressions, but that doesn't work for the Visual
+        Studio backend, so we would have to drop support for that (not going to happen). The CMake
+        documentation suggests that C and C++ files ought to be built in separate targets to avoid
+        this. It's a mess, basically.
+
+        So I've wound up removing WEBKIT_SET_EXTRA_COMPILER_FLAGS and adding most of those flags to
+        CMAKE_C_FLAGS and CMAKE_CXX_FLAGS instead. Really the only disadvantage of this is we now
+        have to suppress individual warnings when building ANGLESupport in WebCore. That's not the
+        end of the world. The only remaining useful feature of WEBKIT_SET_EXTRA_COMPILER_FLAGS was
+        to add -fPIC to static library targets, but turns out CMake does that for us if we just set
+        the variable CMAKE_POSITION_INDEPENDENT_CODE, so we can get rid of it completely.
+
+        Of course there are also macros for setting target-specific compiler flags, which we
+        frequently need in order to suppress specific warnings, particularly warnings coming from
+        third-party libraries like ANGLE and gtest. But remember the footgun: these macros will test
+        the flag against only one compiler, but must work with both C and C++ compilers unless the
+        build target exclusively contains targets built with just one of those compilers. Yuck.
+
+        * CMakeLists.txt:
+        * Source/CMakeLists.txt:
+        * Source/PlatformGTK.cmake:
+        * Source/cmake/OptionsCommon.cmake:
+        * Source/cmake/WebKitCommon.cmake:
+        * Source/cmake/WebKitCompilerFlags.cmake: Added.
+        * Source/cmake/WebKitMacros.cmake:
+
 2017-08-07  Brian Burg  <[email protected]>
 
         Remove CANVAS_PATH compilation guard

Source/CMakeLists.txt

@@ -43,29 +43,3 @@ if (ENABLE_WEBDRIVER)
 endif ()
 
 WEBKIT_INCLUDE_CONFIG_FILES_IF_EXISTS()
-
-# -----------------------------------------------------------------------------
-# Set compiler flags for all targets
-# -----------------------------------------------------------------------------
-if (NOT USE_SYSTEM_MALLOC)
-    WEBKIT_SET_EXTRA_COMPILER_FLAGS(bmalloc ${ADDITIONAL_COMPILER_FLAGS})
-endif ()
-WEBKIT_SET_EXTRA_COMPILER_FLAGS(WTF ${ADDITIONAL_COMPILER_FLAGS})
-WEBKIT_SET_EXTRA_COMPILER_FLAGS(JavaScriptCore ${ADDITIONAL_COMPILER_FLAGS})
-
-if (ENABLE_WEBCORE)
-    WEBKIT_SET_EXTRA_COMPILER_FLAGS(PAL ${ADDITIONAL_COMPILER_FLAGS})
-    WEBKIT_SET_EXTRA_COMPILER_FLAGS(WebCoreTestSupport ${ADDITIONAL_COMPILER_FLAGS})
-    WEBKIT_SET_EXTRA_COMPILER_FLAGS(WebCore ${ADDITIONAL_COMPILER_FLAGS})
-    WEBKIT_SET_EXTRA_COMPILER_FLAGS(WebCoreDerivedSources ${ADDITIONAL_COMPILER_FLAGS})
-endif ()
-
-if (ENABLE_WEBKIT_LEGACY)
-    # FIXME: Rename this target to WebKitLegacy.
-    WEBKIT_SET_EXTRA_COMPILER_FLAGS(WebKit ${ADDITIONAL_COMPILER_FLAGS})
-endif ()
-
-if (ENABLE_WEBKIT)
-    # FIXME: Rename this target to WebKit.
-    WEBKIT_SET_EXTRA_COMPILER_FLAGS(WebKit2 ${ADDITIONAL_COMPILER_FLAGS})
-endif ()

Source/JavaScriptCore/API/tests/PingPongStackOverflowTest.cpp

@@ -142,7 +142,6 @@ int testPingPongStackOverflow()
         "PingPongStackOverflowObject.__proto__ = undefined;" \
         "undefined instanceof PingPongStackOverflowObject;";
 
-    JSValueRef scriptResult = nullptr;
     JSValueRef exception = nullptr;
     JSStringRef script = JSStringCreateWithUTF8CString(scriptString);
 
@@ -161,7 +160,7 @@ int testPingPongStackOverflow()
     Options::maxPerThreadStackUsage() = stackSize + Options::softReservedZoneSize();
 
     exception = nullptr;
-    scriptResult = JSEvaluateScript(context, script, nullptr, nullptr, 1, &exception);
+    JSEvaluateScript(context, script, nullptr, nullptr, 1, &exception);
 
     if (!exception) {
         printf("FAIL: PingPongStackOverflowError not thrown in PingPongStackOverflow test\n");

Source/JavaScriptCore/API/tests/testapi.c

@@ -1134,13 +1134,20 @@ static bool globalContextNameTest()
     return result;
 }
 
+#if COMPILER(GCC)
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Wunused-but-set-variable"
+#endif
 static void checkConstnessInJSObjectNames()
 {
     JSStaticFunction fun;
     fun.name = "something";
     JSStaticValue val;
     val.name = "something";
 }
+#if COMPILER(GCC)
+#pragma GCC diagnostic pop
+#endif
 
 #ifdef __cplusplus
 extern "C" {

Source/JavaScriptCore/ChangeLog

@@ -1,3 +1,16 @@
+2017-08-08  Michael Catanzaro  <[email protected]>
+
+        [CMake] Properly test if compiler supports compiler flags
+        https://bugs.webkit.org/show_bug.cgi?id=174490
+
+        Reviewed by Konstantin Tokarev.
+
+        * API/tests/PingPongStackOverflowTest.cpp:
+        (testPingPongStackOverflow):
+        * API/tests/testapi.c:
+        * b3/testb3.cpp:
+        (JSC::B3::testPatchpointLotsOfLateAnys):
+
 2017-08-06  Yusuke Suzuki  <[email protected]>
 
         [Linux] Clear WasmMemory with madvice instead of memset

Source/JavaScriptCore/b3/testb3.cpp

@@ -8481,7 +8481,7 @@ void testPatchpointLotsOfLateAnys()
         });
     root->appendNewControlValue(proc, Return, Origin(), patchpoint);
 
-    CHECK(compileAndRun<int>(proc) == (things.size() * (things.size() - 1)) / 2);
+    CHECK(static_cast<size_t>(compileAndRun<int>(proc)) == (things.size() * (things.size() - 1)) / 2);
 }
 
 void testPatchpointAnyImm(ValueRep rep)

Source/PlatformGTK.cmake

@@ -24,9 +24,10 @@ macro(ADD_GTKDOC_GENERATOR _stamp_name _extra_args)
     add_custom_command(
         OUTPUT "${CMAKE_BINARY_DIR}/${_stamp_name}"
         DEPENDS ${DocumentationDependencies}
-        COMMAND CC=${CMAKE_C_COMPILER} CFLAGS=${CMAKE_C_FLAGS} ${CMAKE_SOURCE_DIR}/Tools/gtk/generate-gtkdoc ${_extra_args}
+        COMMAND ${CMAKE_COMMAND} -E env "CC=${CMAKE_C_COMPILER}" "CFLAGS=${CMAKE_C_FLAGS} -Wno-unused-parameter" ${CMAKE_SOURCE_DIR}/Tools/gtk/generate-gtkdoc ${_extra_args}
         COMMAND touch ${_stamp_name}
         WORKING_DIRECTORY "${CMAKE_BINARY_DIR}"
+        VERBATIM
     )
 endmacro()
 

Source/ThirdParty/ChangeLog

@@ -1,3 +1,15 @@
+2017-08-08  Michael Catanzaro  <[email protected]>
+
+        [CMake] Properly test if compiler supports compiler flags
+        https://bugs.webkit.org/show_bug.cgi?id=174490
+
+        Reviewed by Konstantin Tokarev.
+
+        * brotli/CMakeLists.txt:
+        * gtest/CMakeLists.txt:
+        * woff2/CMakeLists.txt:
+        * xdgmime/CMakeLists.txt:
+
 2017-07-17  Michael Catanzaro  <[email protected]>
 
         [CMake] Macros in WebKitMacros.cmake should be prefixed with WEBKIT_ namespace

Source/ThirdParty/brotli/CMakeLists.txt

@@ -15,8 +15,8 @@ set(BROTLI_SOURCES
 include_directories("${BROTLI_INCLUDE_DIRECTORIES}")
 add_definitions(-DBROTLI_BUILD_PORTABLE)
 add_library(brotli STATIC ${BROTLI_SOURCES})
-WEBKIT_SET_EXTRA_COMPILER_FLAGS(brotli)
 
 if (COMPILER_IS_GCC_OR_CLANG)
-    WEBKIT_ADD_TARGET_PROPERTIES(brotli COMPILE_FLAGS "-Wno-cast-align -Wno-implicit-fallthrough")
+    WEBKIT_ADD_TARGET_C_FLAGS(brotli -Wno-cast-align
+                                     -Wno-implicit-fallthrough)
 endif ()