Bug 56337 - Enable -Werror on ANGLE
Summary: Enable -Werror on ANGLE
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebGL (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: David Kilzer (:ddkilzer)
URL:
Keywords:
Depends on: 56396
Blocks:
  Show dependency treegraph
 
Reported: 2011-03-14 14:38 PDT by David Kilzer (:ddkilzer)
Modified: 2011-05-04 11:14 PDT (History)
7 users (show)

See Also:


Attachments
Patch 5/5: Enable -Werror on ANGLE (1.75 KB, patch)
2011-03-14 16:09 PDT, David Kilzer (:ddkilzer)
abarth: review+
Details | Formatted Diff | Diff
Patch 1/5: Fix clang warning: ConstantUnion.h:56:17: warning: 7 enumeration values not handled in switch (2.95 KB, patch)
2011-03-14 16:09 PDT, David Kilzer (:ddkilzer)
no flags Details | Formatted Diff | Diff
Patch 2/5: Fix clang warning: ParseHelper.cpp:264:13: warning: 8 enumeration values not handled in switch (2.62 KB, patch)
2011-03-14 16:10 PDT, David Kilzer (:ddkilzer)
no flags Details | Formatted Diff | Diff
Patch 3/5: Fix clang warnings: Intermediate.cpp: warning: '&&' within '||' (8.27 KB, patch)
2011-03-14 16:10 PDT, David Kilzer (:ddkilzer)
no flags Details | Formatted Diff | Diff
Patch 4/5: Fix clang warnings in PoolAlloc.cpp (4.65 KB, patch)
2011-03-14 16:11 PDT, David Kilzer (:ddkilzer)
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description David Kilzer (:ddkilzer) 2011-03-14 14:38:18 PDT
We should enable -Werror on ANGLE to catch compiler issues.

There are a few fixes for clang that must be addressed first.
Comment 1 David Kilzer (:ddkilzer) 2011-03-14 16:09:04 PDT
Created attachment 85736 [details]
Patch 5/5: Enable -Werror on ANGLE

Posting the last one first to make sure gcc-4.2/llvm-gcc-4.2 still builds with this change.
Comment 2 David Kilzer (:ddkilzer) 2011-03-14 16:09:44 PDT
Created attachment 85737 [details]
Patch 1/5: Fix clang warning: ConstantUnion.h:56:17: warning: 7 enumeration values not handled in switch
Comment 3 David Kilzer (:ddkilzer) 2011-03-14 16:10:15 PDT
Created attachment 85738 [details]
Patch 2/5: Fix clang warning: ParseHelper.cpp:264:13: warning: 8 enumeration values not handled in switch
Comment 4 David Kilzer (:ddkilzer) 2011-03-14 16:10:44 PDT
Created attachment 85739 [details]
Patch 3/5: Fix clang warnings: Intermediate.cpp: warning: '&&' within '||'
Comment 5 David Kilzer (:ddkilzer) 2011-03-14 16:11:14 PDT
Created attachment 85740 [details]
Patch 4/5: Fix clang warnings in PoolAlloc.cpp
Comment 6 WebKit Review Bot 2011-03-14 16:14:50 PDT
Attachment 85739 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/ThirdParty/ANGLE/ChangeLog', u'Sour..." exit_code: 1

Source/ThirdParty/ANGLE/src/compiler/Intermediate.cpp:1008:  Boolean expressions that span multiple lines should have their operators on the left side of the line instead of the right side.  [whitespace/operators] [4]
Source/ThirdParty/ANGLE/src/compiler/Intermediate.cpp:1020:  Boolean expressions that span multiple lines should have their operators on the left side of the line instead of the right side.  [whitespace/operators] [4]
Total errors found: 2 in 2 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 7 Zhenyao Mo 2011-03-14 16:30:55 PDT
This should be fixed upstream ANGLE.  If you commit it here, after a ANGLE roll in the future, your change will be lost.

I see you already filed a bug to ANGLE.   Could you upload your patch there?  Once it's reviewed and landed upstream, we can do another ANGLE roll in webkit.
Comment 8 David Kilzer (:ddkilzer) 2011-03-14 20:15:42 PDT
(In reply to comment #7)
> This should be fixed upstream ANGLE.  If you commit it here, after a ANGLE roll in the future, your change will be lost.
> 
> I see you already filed a bug to ANGLE.   Could you upload your patch there?  Once it's reviewed and landed upstream, we can do another ANGLE roll in webkit.

So you don't merge changes from ANGLE into WebKit--you just land new code and delete whatever local changes were made?
Comment 9 David Kilzer (:ddkilzer) 2011-03-14 20:19:47 PDT
(In reply to comment #7)
> I see you already filed a bug to ANGLE.   Could you upload your patch there?  Once it's reviewed and landed upstream, we can do another ANGLE roll in webkit.

I'll upload the patches upstream as well.
Comment 10 David Kilzer (:ddkilzer) 2011-03-14 20:24:23 PDT
Comment on attachment 85737 [details]
Patch 1/5: Fix clang warning: ConstantUnion.h:56:17: warning: 7 enumeration values not handled in switch

Upstream bug: <http://code.google.com/p/angleproject/issues/detail?id=124>
Comment 11 David Kilzer (:ddkilzer) 2011-03-14 20:24:38 PDT
Comment on attachment 85738 [details]
Patch 2/5: Fix clang warning: ParseHelper.cpp:264:13: warning: 8 enumeration values not handled in switch

Upstream bug: <http://code.google.com/p/angleproject/issues/detail?id=125>
Comment 12 David Kilzer (:ddkilzer) 2011-03-14 20:24:53 PDT
Comment on attachment 85739 [details]
Patch 3/5: Fix clang warnings: Intermediate.cpp: warning: '&&' within '||'

Upstream bug: <http://code.google.com/p/angleproject/issues/detail?id=126>
Comment 13 David Kilzer (:ddkilzer) 2011-03-14 20:25:06 PDT
Comment on attachment 85740 [details]
Patch 4/5: Fix clang warnings in PoolAlloc.cpp

Upstream bug: <http://code.google.com/p/angleproject/issues/detail?id=127>
Comment 14 David Kilzer (:ddkilzer) 2011-03-14 20:26:37 PDT
Patch 5/5 (Attachment 85736 [details]) is now blocked on the upstream patches landing, then ANGLE being updated.
Comment 15 WebKit Review Bot 2011-03-14 21:46:56 PDT
Attachment 85736 [details] did not build on mac:
Build output: http://queues.webkit.org/results/8175479
Comment 16 Adam Barth 2011-03-15 02:00:24 PDT
Comment on attachment 85736 [details]
Patch 5/5: Enable -Werror on ANGLE

I support this product and or service (assuming it builds).
Comment 17 Zhenyao Mo 2011-03-15 09:18:59 PDT
(In reply to comment #8)
> (In reply to comment #7)
> > This should be fixed upstream ANGLE.  If you commit it here, after a ANGLE roll in the future, your change will be lost.
> > 
> > I see you already filed a bug to ANGLE.   Could you upload your patch there?  Once it's reviewed and landed upstream, we can do another ANGLE roll in webkit.
> 
> So you don't merge changes from ANGLE into WebKit--you just land new code and delete whatever local changes were made?

Yes.  That's at least what we've done so far.
Comment 18 David Kilzer (:ddkilzer) 2011-03-15 11:59:18 PDT
(In reply to comment #14)
> Patch 5/5 (Attachment 85736 [details]) is now blocked on the upstream patches landing, then ANGLE being updated.

Filed Bug 56396 to update ANGLE to at least r575, which will pick up the necessary fixes to enable -Werror.
Comment 19 David Kilzer (:ddkilzer) 2011-05-04 11:13:45 PDT
Part 1 of 2:

Committed r85761: <http://trac.webkit.org/changeset/85761>
Comment 20 David Kilzer (:ddkilzer) 2011-05-04 11:14:31 PDT
Part 2 of 2:

Committed r85762: <http://trac.webkit.org/changeset/85762>