Bug 113147 - Fix the build with GCC 4.8
Summary: Fix the build with GCC 4.8
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Template Framework (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Andras Becsi
URL:
Keywords:
Depends on:
Blocks: 110211
  Show dependency treegraph
 
Reported: 2013-03-23 22:44 PDT by Jonathan Liu
Modified: 2013-04-05 04:08 PDT (History)
13 users (show)

See Also:


Attachments
Patch (1.33 KB, patch)
2013-03-31 01:17 PDT, Jonathan Liu
no flags Details | Formatted Diff | Diff
Patch (3.21 KB, patch)
2013-04-04 08:59 PDT, Andras Becsi
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jonathan Liu 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);
Comment 1 Allan Sandfeld Jensen 2013-03-26 06:43:02 PDT
Compile_assert should probably be implemented using static_assert when available.
Comment 2 Kai Koehne 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).
Comment 3 Allan Sandfeld Jensen 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
Comment 4 Alexey Proskuryakov 2013-03-26 11:02:30 PDT
Does this bug still track anything useful, given that bug 113308 will fix the symptom?
Comment 5 Allan Sandfeld Jensen 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.
Comment 6 Zan Dobersek 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?
Comment 7 Andras Becsi 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.
Comment 8 Allan Sandfeld Jensen 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.
Comment 9 Zan Dobersek 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
Comment 10 Jonathan Liu 2013-03-31 01:17:12 PDT
Created attachment 195890 [details]
Patch
Comment 11 WebKit Review Bot 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.
Comment 12 Raphael Kubo da Costa (:rakuco) 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?
Comment 13 Anders Carlsson 2013-04-03 07:54:16 PDT
This doesn't seem like a helpful warning, let's just turn it off.
Comment 14 Raphael Kubo da Costa (:rakuco) 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?
Comment 15 Allan Sandfeld Jensen 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.
Comment 16 Raphael Kubo da Costa (:rakuco) 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.
Comment 17 Andras Becsi 2013-04-04 08:59:04 PDT
Created attachment 196483 [details]
Patch
Comment 18 Allan Sandfeld Jensen 2013-04-04 09:32:15 PDT
Comment on attachment 196483 [details]
Patch

LGTM
Comment 19 Andras Becsi 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>
Comment 20 Andras Becsi 2013-04-04 09:39:12 PDT
All reviewed patches have been landed.  Closing bug.