Bug 119932 – WebCore fails to build with trunk clang: error: 'register' storage class specifier is deprecated [-Werror,-Wdeprecated-register]

• Component: WebKit Misc.
• Status: RESOLVED FIXED

[David Kilzer (:ddkilzer)]

WebCore fails to build with trunk clang.  Errors are due to the 'register' keyword being deprecated.  The output of gperf uses 'register' keyword.

[David Kilzer (:ddkilzer)]

<rdar://problem/14764085>

[David Kilzer (:ddkilzer)]

Created attachment 208975 [details]
Patch v1

[Build Bot]

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]

Can't we just remove the "register" specifier in all 3 files?

[Build Bot]

Comment on attachment 208975 [details]
Patch v1

Attachment 208975 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/1486009

[David Kilzer (:ddkilzer)]

(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)]

(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)]

Created attachment 208981 [details]
Patch v2

[Darin Adler]

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 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 on attachment 208981 [details]
Patch v2

Marking patch obsolete to remove other instances of 'register' in function parameters.

[David Kilzer (:ddkilzer)]

(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)]

(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)]

Created attachment 208999 [details]
Patch v3

[WebKit Commit Bot]

Comment on attachment 208999 [details]
Patch v3

Clearing flags on attachment: 208999

Committed r154259: <http://trac.webkit.org/changeset/154259>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.