RESOLVED FIXED 119932
WebCore fails to build with trunk clang: error: 'register' storage class specifier is deprecated [-Werror,-Wdeprecated-register]
https://bugs.webkit.org/show_bug.cgi?id=119932
Summary WebCore fails to build with trunk clang: error: 'register' storage class spec...
David Kilzer (:ddkilzer)
Reported 2013-08-16 21:43:41 PDT
WebCore fails to build with trunk clang. Errors are due to the 'register' keyword being deprecated. The output of gperf uses 'register' keyword.
Attachments
Patch v1 (3.02 KB, patch)
2013-08-16 21:54 PDT, David Kilzer (:ddkilzer)
no flags
Patch v2 (3.28 KB, patch)
2013-08-16 23:29 PDT, David Kilzer (:ddkilzer)
no flags
Patch v3 (4.17 KB, patch)
2013-08-17 09:12 PDT, David Kilzer (:ddkilzer)
no flags
David Kilzer (:ddkilzer)
Comment 1 2013-08-16 21:51:49 PDT
David Kilzer (:ddkilzer)
Comment 2 2013-08-16 21:54:28 PDT
Created attachment 208975 [details] Patch v1
Build Bot
Comment 3 2013-08-16 22:21:56 PDT
Comment on attachment 208975 [details] Patch v1 Attachment 208975 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.appspot.com/results/1345336
Benjamin Poulain
Comment 4 2013-08-16 22:30:51 PDT
Can't we just remove the "register" specifier in all 3 files?
Build Bot
Comment 5 2013-08-16 22:34:46 PDT
David Kilzer (:ddkilzer)
Comment 6 2013-08-16 22:45:38 PDT
(In reply to comment #4) > Can't we just remove the "register" specifier in all 3 files? No, it's generated from the gperf command.
David Kilzer (:ddkilzer)
Comment 7 2013-08-16 22:49:12 PDT
(In reply to comment #5) > (From update of attachment 208975 [details]) > Attachment 208975 [details] did not pass mac-ews (mac): > Output: http://webkit-queues.appspot.com/results/1486009 WebCore/platform/ColorData.gperf:7:34: error: unknown warning group '-Wdeprecated-register', ignored [-Werror,-Wunknown-pragmas] #pragma clang diagnostic ignored "-Wdeprecated-register" ^ 1 error generated.
David Kilzer (:ddkilzer)
Comment 8 2013-08-16 23:29:37 PDT
Created attachment 208981 [details] Patch v2
Darin Adler
Comment 9 2013-08-17 05:15:32 PDT
Comment on attachment 208981 [details] Patch v2 View in context: https://bugs.webkit.org/attachment.cgi?id=208981&action=review > Source/WebCore/css/makeprop.pl:81 > +#if defined(__clang__) > +#pragma clang diagnostic push > +#pragma clang diagnostic ignored \"-Wunknown-pragmas\" > +#pragma clang diagnostic ignored \"-Wdeprecated-register\" > +#endif This is the wrong way to fix it. We should remove the "register" keywords instead. They are doing no good.
Darin Adler
Comment 10 2013-08-17 05:17:04 PDT
Comment on attachment 208981 [details] Patch v2 Are you sure the register are generated by gperf? I see "register" in these source files, and we should definitely remove those even if we also turn off the diagnostic. If gperf adds additional "register" then I suppose it's OK to ignore the warning.
David Kilzer (:ddkilzer)
Comment 11 2013-08-17 07:11:59 PDT
Comment on attachment 208981 [details] Patch v2 Marking patch obsolete to remove other instances of 'register' in function parameters.
David Kilzer (:ddkilzer)
Comment 12 2013-08-17 07:30:03 PDT
(In reply to comment #10) > (From update of attachment 208981 [details]) > Are you sure the register are generated by gperf? Yes, I am sure. Here are the error messages; the code below doesn't appear in the source *.gperf files (this is from a single file, but the errors are all the same): WebCore/platform/ColorData.gperf:57:3: error: 'register' storage class specifier is deprecated [-Werror,-Wdeprecated-register] register int hval = (int)len; ^~~~~~~~~ WebCore/platform/ColorData.gperf:250:7: error: 'register' storage class specifier is deprecated [-Werror,-Wdeprecated-register] register int key = colordata_hash_function (str, len); ^~~~~~~~~ WebCore/platform/ColorData.gperf:254:11: error: 'register' storage class specifier is deprecated [-Werror,-Wdeprecated-register] register int index = lookup[key]; ^~~~~~~~~ WebCore/platform/ColorData.gperf:258:15: error: 'register' storage class specifier is deprecated [-Werror,-Wdeprecated-register] register const char *s = wordlist[index].name; ^~~~~~~~~ 4 errors generated. > I see "register" in these source files, and we should definitely remove those even if we also turn off the diagnostic. If gperf adds additional "register" then I suppose it's OK to ignore the warning. Thanks for catching the additional 'register' keyword use. It wasn't flagged as an error when building with trunk clang.
David Kilzer (:ddkilzer)
Comment 13 2013-08-17 08:00:49 PDT
(In reply to comment #12) > (In reply to comment #10) > > (From update of attachment 208981 [details] [details]) > > I see "register" in these source files, and we should definitely remove those even if we also turn off the diagnostic. If gperf adds additional "register" then I suppose it's OK to ignore the warning. > > Thanks for catching the additional 'register' keyword use. It wasn't flagged as an error when building with trunk clang. To follow up, the non-generated code that used 'register' keyword was doing it to match the generated code it called. For ColorData.gperf, here is the manual code: const struct NamedColor* findColor(register const char* str, register unsigned int len) { return ColorDataHash::findColorImpl(str, len); } The method it called was defined thusly: const struct NamedColor * ColorDataHash::findColorImpl (register const char *str, register unsigned int len)
David Kilzer (:ddkilzer)
Comment 14 2013-08-17 09:12:45 PDT
Created attachment 208999 [details] Patch v3
WebKit Commit Bot
Comment 15 2013-08-18 20:04:19 PDT
Comment on attachment 208999 [details] Patch v3 Clearing flags on attachment: 208999 Committed r154259: <http://trac.webkit.org/changeset/154259>
WebKit Commit Bot
Comment 16 2013-08-18 20:04:23 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.