Bug 60380 - Fix compile with GCC 4.6.0
Summary: Fix compile with GCC 4.6.0
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC Linux
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-05-06 10:21 PDT by Dawit A.
Modified: 2011-05-20 05:57 PDT (History)
5 users (show)

See Also:


Attachments
proposed patch v1 (1.36 KB, patch)
2011-05-06 10:24 PDT, Dawit A.
no flags Details | Formatted Diff | Diff
proposed patch v2 (1.36 KB, patch)
2011-05-06 10:32 PDT, Dawit A.
akling: review-
Details | Formatted Diff | Diff
proposed patch v3 (3.92 KB, patch)
2011-05-06 11:33 PDT, Dawit A.
ap: review-
Details | Formatted Diff | Diff
proposed patch v4 (690 bytes, patch)
2011-05-06 17:34 PDT, Dawit A.
dbates: review-
Details | Formatted Diff | Diff
proposed patch v5 (1.30 KB, patch)
2011-05-07 15:30 PDT, Dawit A.
dbates: review+
dbates: commit-queue-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Dawit A. 2011-05-06 10:21:47 PDT
Attempting to compile webkit trunk on my box (ArchLinux x86-64) stops the following error:

Source/JavaScriptCore/runtime/StringPrototype.cpp: In function ‘void* JSC::stringProtoFuncMatch(JSC::ExecState*)’:
Source/JavaScriptCore/runtime/StringPrototype.cpp:631:14: error: variable ‘lastIndex’ set but not used [-Werror=unused-but-set-variable]

The attached patch comments of the offending set but unused variable to prevent this compile breakage.
Comment 1 Dawit A. 2011-05-06 10:24:49 PDT
Created attachment 92599 [details]
proposed patch v1
Comment 2 WebKit Review Bot 2011-05-06 10:26:46 PDT
Attachment 92599 [details] did not pass style-queue:

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

Source/JavaScriptCore/runtime/StringPrototype.cpp:631:  Should have a space between // and comment  [whitespace/comments] [4]
Source/JavaScriptCore/runtime/StringPrototype.cpp:634:  Should have a space between // and comment  [whitespace/comments] [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 3 Dawit A. 2011-05-06 10:32:30 PDT
Created attachment 92600 [details]
proposed patch v2

Fixed the style problems...
Comment 4 Andreas Kling 2011-05-06 10:54:18 PDT
Comment on attachment 92600 [details]
proposed patch v2

If the variable is unused, it should be removed, not commented out.
Comment 5 Dawit A. 2011-05-06 11:33:14 PDT
Created attachment 92614 [details]
proposed patch v3

Removed the set but unused variables instead of commenting them out.
Comment 6 Alexey Proskuryakov 2011-05-06 17:15:34 PDT
Comment on attachment 92614 [details]
proposed patch v3

I saw (at least parts of) this is someone else's patch today, r- since this will conflict with it.
Comment 7 Alexey Proskuryakov 2011-05-06 17:16:37 PDT
Specifically, bug 60370.
Comment 8 Dawit A. 2011-05-06 17:27:39 PDT
(In reply to comment #7)
> Specifically, bug 60370.

Greate, but the patch in that report does not fix the JavascriptCore which was  originally proposed in patch v2.
Comment 9 Dawit A. 2011-05-06 17:34:02 PDT
Created attachment 92663 [details]
proposed patch v4

Removed fixes that have already been applied through bug# 60370.
Comment 10 Daniel Bates 2011-05-07 14:35:25 PDT
Comment on attachment 92663 [details]
proposed patch v4

OK.
Comment 11 Daniel Bates 2011-05-07 14:35:54 PDT
Comment on attachment 92663 [details]
proposed patch v4

r- you're missing a change log.
Comment 12 Dawit A. 2011-05-07 15:30:08 PDT
Created attachment 92698 [details]
proposed patch v5

Added missing changelog information.
Comment 13 Daniel Bates 2011-05-07 15:55:15 PDT
Comment on attachment 92698 [details]
proposed patch v5

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

As far as I can tell you're not a committer. So, I will make these changes for you and land your patch.

> Source/JavaScriptCore/ChangeLog:8
> +        Remove unused local variable from code
> +
> +        Fix compile with gcc 4.6.0
> +        https://bugs.webkit.org/show_bug.cgi?id=60380

This change log entry does not have the right format. The format is bug title, bug url, then description. Also, GCC is an acronym so "gcc" should be "GCC" (although I can see from the diff that we landed change log entries with "gcc").

Also, we use complete sentences.
Comment 14 Daniel Bates 2011-05-07 15:59:22 PDT
Committed r86008: <http://trac.webkit.org/changeset/86008>
Comment 15 Dawit A. 2011-05-07 16:11:07 PDT
(In reply to comment #13)
> (From update of attachment 92698 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=92698&action=review
> 
> As far as I can tell you're not a committer. So, I will make these changes for you and land your patch.
> 
> > Source/JavaScriptCore/ChangeLog:8
> > +        Remove unused local variable from code
> > +
> > +        Fix compile with gcc 4.6.0
> > +        https://bugs.webkit.org/show_bug.cgi?id=60380
> 
> This change log entry does not have the right format. The format is bug title, bug url, then description. Also, GCC is an acronym so "gcc" should be "GCC" (although I can see from the diff that we landed change log entries with "gcc").

For the record, the changelog message was prepared using 

./Tools/Scripts/prepare-ChangeLog -b 60380 -g <git-commit-number>

So the format is as generated by the script one is supposed to use to generate the changelog...
Comment 16 Ademar (unprivileged account) 2011-05-17 06:42:25 PDT
Revision r86008 cherry-picked into qtwebkit-2.2 with commit 8620c0c <http://gitorious.org/webkit/qtwebkit/commit/8620c0c>
Comment 17 Dawit A. 2011-05-19 18:19:27 PDT
(In reply to comment #16)
> Revision r86008 cherry-picked into qtwebkit-2.2 with commit 8620c0c <http://gitorious.org/webkit/qtwebkit/commit/8620c0c>

Need to cherry pick the patch in bug# 60370 as well. See comment #8.
http://trac.webkit.org/changeset/85973
Comment 18 Ademar Reis 2011-05-20 05:20:26 PDT
(In reply to comment #17)
> (In reply to comment #16)
> > Revision r86008 cherry-picked into qtwebkit-2.2 with commit 8620c0c <http://gitorious.org/webkit/qtwebkit/commit/8620c0c>
> 
> Need to cherry pick the patch in bug# 60370 as well. See comment #8.
> http://trac.webkit.org/changeset/85973

Done, thanks!
Comment 19 Ademar Reis 2011-05-20 05:48:35 PDT
Dawit: bugs should block the master bug until they're cherry-picked. Once they're included in the branch they're removed from the blocking list.
Comment 20 Dawit A. 2011-05-20 05:57:53 PDT
(In reply to comment #19)
> Dawit: bugs should block the master bug until they're cherry-picked. Once they're included in the branch they're removed from the blocking list.

Ahhh.... right. The reason I made this bug block the master bug is because not all the patches from this bug, actually the one from bug 60370, were cherry-picked. Since I did not create the 60370 bug and I could not change it to block master, I simply added a comment on this one and made it the blocker instead.