Bug 113147

Summary: Fix the build with GCC 4.8
Product: WebKit Reporter: Jonathan Liu <net147>
Component: Web Template FrameworkAssignee: Andras Becsi <abecsi>
Status: RESOLVED FIXED    
Severity: Normal CC: abecsi, allan.jensen, andersca, ap, benjamin, cmarcelo, hausmann, jdiggs, kai.koehne, ojan.autocc, rakuco, webkit.review.bot, zan
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 110211    
Attachments:
Description Flags
Patch
none
Patch none

Jonathan Liu
Reported 2013-03-23 22:44:44 PDT
Compiling WebKit with GCC 4.8 generates lots of warnings due to COMPILE_ASSERT defining an unused typedef. Example: generated/InspectorTypeBuilder.h: In member function 'WebCore::TypeBuilder::Network::CachedResource::Builder<STATE>::operator WTF::RefPtr<WebCore::TypeBuilder::Network::CachedResource>&()': ..\WTF/wtf/Assertions.h:328:47: warning: typedef 'dummyresult_is_not_ready' locally defined but not used [-Wunused-local-typedefs] #define COMPILE_ASSERT(exp, name) typedef int dummy##name [(exp) ? 1 : -1] ^ generated/InspectorTypeBuilder.h:2932:13: note: in expansion of macro 'COMPILE_ASSERT' COMPILE_ASSERT(STATE == AllFieldsSet, result_is_not_ready);
Attachments
Patch (1.33 KB, patch)
2013-03-31 01:17 PDT, Jonathan Liu
no flags
Patch (3.21 KB, patch)
2013-04-04 08:59 PDT, Andras Becsi
no flags
Allan Sandfeld Jensen
Comment 1 2013-03-26 06:43:02 PDT
Compile_assert should probably be implemented using static_assert when available.
Kai Koehne
Comment 2 2013-03-26 06:57:21 PDT
We could also just diable the warning: #pragma GCC diagnostic ignored "-Wunused-local-typedefs" (Note that you can't disable it 'locally' with pragma push, pragma pop though cause it's part of a #define).
Allan Sandfeld Jensen
Comment 3 2013-03-26 07:25:02 PDT
(In reply to comment #1) > Compile_assert should probably be implemented using static_assert when available. Uploaded that to bug 113308
Alexey Proskuryakov
Comment 4 2013-03-26 11:02:30 PDT
Does this bug still track anything useful, given that bug 113308 will fix the symptom?
Allan Sandfeld Jensen
Comment 5 2013-03-26 11:09:31 PDT
(In reply to comment #4) > Does this bug still track anything useful, given that bug 113308 will fix the symptom? C++11 support may not be enabled in GCC. In fact it is default off.
Zan Dobersek
Comment 6 2013-03-27 03:42:45 PDT
(In reply to comment #5) > (In reply to comment #4) > > Does this bug still track anything useful, given that bug 113308 will fix the symptom? > > C++11 support may not be enabled in GCC. In fact it is default off. The GTK port only enforces the C++11 standard when building the WebKit2 framework. This means that when building WTF, WebCore etc. static_assert is still not available and thus the same warnings are still being spat out. Is this true for the Qt port as well?
Andras Becsi
Comment 7 2013-03-27 03:55:05 PDT
(In reply to comment #6) > (In reply to comment #5) > > (In reply to comment #4) > > > Does this bug still track anything useful, given that bug 113308 will fix the symptom? > > > > C++11 support may not be enabled in GCC. In fact it is default off. > > The GTK port only enforces the C++11 standard when building the WebKit2 framework. This means that when building WTF, WebCore etc. static_assert is still not available and thus the same warnings are still being spat out. > > Is this true for the Qt port as well? As far as I know Qt does not enforce C++11 on any framework yet.
Allan Sandfeld Jensen
Comment 8 2013-03-27 03:59:56 PDT
(In reply to comment #6) > (In reply to comment #5) > > (In reply to comment #4) > > > Does this bug still track anything useful, given that bug 113308 will fix the symptom? > > > > C++11 support may not be enabled in GCC. In fact it is default off. > > The GTK port only enforces the C++11 standard when building the WebKit2 framework. This means that when building WTF, WebCore etc. static_assert is still not available and thus the same warnings are still being spat out. > > Is this true for the Qt port as well? Let me put it this way: WebCore doesn't build with C++11 enabled, see bug 111579.
Zan Dobersek
Comment 9 2013-03-27 04:10:12 PDT
Right, understood. The warnings that the dummy COMPILE_ASSERT implementation is throwing out can be suppressed using the __attribute__((unused)) directive. It seems to work with GCC 4.7 and 4.8 and with Clang 3.2, though there are other compilers that people use that don't necessary support the directive. https://svn.boost.org/trac/boost/ticket/7242
Jonathan Liu
Comment 10 2013-03-31 01:17:12 PDT
WebKit Review Bot
Comment 11 2013-03-31 16:04:21 PDT
Attachment 195890 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WTF/ChangeLog', u'Source/WTF/wtf/Assertions.h']" exit_code: 1 Source/WTF/wtf/Assertions.h:354: Extra space before [ [whitespace/braces] [5] Total errors found: 1 in 2 files If any of these errors are false positives, please file a bug against check-webkit-style.
Raphael Kubo da Costa (:rakuco)
Comment 12 2013-04-03 05:05:47 PDT
There are also warnings generated by Source/WTF/wtf/dtoa/utils.h:294:22: error: typedef ‘VerifySizesAreEqual’ locally defined but not used [-Werror=unused-local-typedefs] typedef char VerifySizesAreEqual[sizeof(Dest) == sizeof(Source) ? 1 : -1]; Source/WTF/wtf/HashTable.h:1302:84: warning: typedef ‘HashTableType’ locally defined but not used [-Wunused-local-typedefs] typedef HashTable<Key, Value, Extractor, HashFunctions, Traits, KeyTraits> HashTableType; ^ Source/WTF/wtf/HashTable.h:1303:97: warning: typedef ‘const_iterator’ locally defined but not used [-Wunused-local-typedefs] typedef HashTableConstIterator<Key, Value, Extractor, HashFunctions, Traits, KeyTraits> const_iterator; ^ Doesn't it make more sense to add a macro to Compiler.h that's defined when we have either gcc or clang and use it in all those declarations?
Anders Carlsson
Comment 13 2013-04-03 07:54:16 PDT
This doesn't seem like a helpful warning, let's just turn it off.
Raphael Kubo da Costa (:rakuco)
Comment 14 2013-04-03 08:03:02 PDT
(In reply to comment #13) > This doesn't seem like a helpful warning, let's just turn it off. Hmm, do you mean leaving it up to each build system to detect the compiler and its version and pass -Wno-unused-local-typedefs accordingly?
Allan Sandfeld Jensen
Comment 15 2013-04-04 02:20:32 PDT
(In reply to comment #14) > (In reply to comment #13) > > This doesn't seem like a helpful warning, let's just turn it off. > > Hmm, do you mean leaving it up to each build system to detect the compiler and its version and pass -Wno-unused-local-typedefs accordingly? I think it would be better to disable it in source code than to try to rely on specific compiler flags.
Raphael Kubo da Costa (:rakuco)
Comment 16 2013-04-04 04:20:42 PDT
(In reply to comment #15) > (In reply to comment #14) > > (In reply to comment #13) > > > This doesn't seem like a helpful warning, let's just turn it off. > > > > Hmm, do you mean leaving it up to each build system to detect the compiler and its version and pass -Wno-unused-local-typedefs accordingly? > > I think it would be better to disable it in source code than to try to rely on specific compiler flags. I agree, and I think this was the direction we were moving towards, but Anders' comment sounded like he had a different opinion.
Andras Becsi
Comment 17 2013-04-04 08:59:04 PDT
Allan Sandfeld Jensen
Comment 18 2013-04-04 09:32:15 PDT
Comment on attachment 196483 [details] Patch LGTM
Andras Becsi
Comment 19 2013-04-04 09:39:05 PDT
Comment on attachment 196483 [details] Patch Clearing flags on attachment: 196483 Committed r147640: <http://trac.webkit.org/changeset/147640>
Andras Becsi
Comment 20 2013-04-04 09:39:12 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.