Bug 232998 - Non-unified build fixes, early November 2021 edition, bis
Summary: Non-unified build fixes, early November 2021 edition, bis
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Adrian Perez
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2021-11-11 07:58 PST by Adrian Perez
Modified: 2021-11-12 06:25 PST (History)
28 users (show)

See Also:

Attachments
Patch (24.29 KB, patch)
2021-11-11 08:53 PST, Adrian Perez 		no flags Details | Formatted Diff | Diff
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Adrian Perez 2021-11-11 07:58:01 PST 
👺️
Comment 1 Adrian Perez 2021-11-11 08:53:33 PST 
Created attachment 443951 [details]
Patch
Comment 2 EWS 2021-11-11 13:29:07 PST 
Committed r285654 (244152@main): <https://commits.webkit.org/244152@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 443951 [details].
Comment 3 Radar WebKit Bug Importer 2021-11-11 13:30:20 PST 
<rdar://problem/85313167>
Comment 4 Darin Adler 2021-11-11 13:32:31 PST 
Comment on attachment 443951 [details]
Patch

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

> Source/WebCore/ChangeLog:35
> +        * platform/gamepad/GamepadConstants.h: Add missing WTF:: namespace prefix to uses of
> +        WTF::String.

This was an incorrect change. Could you tell me more about why the compile failed without it?

> Source/WebCore/platform/gamepad/GamepadConstants.h:67
> -const String& standardGamepadMappingString();
> +const WTF::String& standardGamepadMappingString();
>  #if ENABLE(WEBXR)
> -const String& xrStandardGamepadMappingString();
> +const WTF::String& xrStandardGamepadMappingString();
>  #endif

This is not the correct idiom. Instead this header should include <wtf/Forward.h> and remove the declaration of the String class above.
Comment 5 Adrian Perez 2021-11-12 06:25:36 PST 
(In reply to Darin Adler from comment #4)
> Comment on attachment 443951 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=443951&action=review
> 
> > Source/WebCore/ChangeLog:35
> > +        * platform/gamepad/GamepadConstants.h: Add missing WTF:: namespace prefix to uses of
> > +        WTF::String.
> 
> This was an incorrect change. Could you tell me more about why the compile
> failed without it?
> 
> > Source/WebCore/platform/gamepad/GamepadConstants.h:67
> > -const String& standardGamepadMappingString();
> > +const WTF::String& standardGamepadMappingString();
> >  #if ENABLE(WEBXR)
> > -const String& xrStandardGamepadMappingString();
> > +const WTF::String& xrStandardGamepadMappingString();
> >  #endif
> 
> This is not the correct idiom. Instead this header should include
> <wtf/Forward.h> and remove the declaration of the String class above.

Fixed in r285715 — thanks for commenting on this :)