Bug 137084

Summary: Web Replay: code generator shouldn't complain about enums without a storage type if they are in an enclosing scope
Product: WebKit Reporter: Brian Burg <burg>
Component: Tools / TestsAssignee: Brian Burg <burg>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, joepeck, kling
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
WIP
none
Patch joepeck: review+

Description Brian Burg 2014-09-24 16:18:26 PDT
.
Comment 1 Brian Burg 2014-09-24 16:19:15 PDT
Created attachment 238622 [details]
WIP
Comment 2 Joseph Pecoraro 2014-09-25 11:40:28 PDT
Comment on attachment 238622 [details]
WIP

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

> Source/JavaScriptCore/replay/scripts/tests/expected/generate-enum-encoding-helpers.json-TestReplayInputs.cpp:181
> +        if (enumString == "Mouse")
> +            enumValue = static_cast<WebCore::PlatformEvent::Type>(enumValue | WebCore::PlatformEvent::Mouse);
> +        if (enumString == "Key")

Nit: Should be an else/if chain.
Comment 3 Brian Burg 2014-09-26 16:30:04 PDT
Created attachment 238746 [details]
Patch
Comment 4 WebKit Commit Bot 2014-09-26 16:32:45 PDT
This patch modifies the WEB_REPLAY inputs generator. Please ensure that you have rebaselined any generator test results (i.e., by running `Tools/Scripts/run-input-generator-tests --reset-results`)
Comment 5 Joseph Pecoraro 2014-10-16 16:53:14 PDT
Comment on attachment 238746 [details]
Patch

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

r=me

> Source/JavaScriptCore/ChangeLog:11
> +        Change the generator to not require an explicit size if an enum is declared inside a struct or class definition. In that case, it must pull in headers since scoped enums can't be

Style: wrap line earlier.

> Source/JavaScriptCore/replay/scripts/tests/expected/generate-enum-encoding-helpers.json-TestReplayInputs.cpp:178
> +    for (String enumString : enumStrings) {

Nit: Unrelated to your changes, but I wonder if "for (const String& : ...)" would be better.
Comment 6 Brian Burg 2014-10-18 11:13:04 PDT
Committed r174863: <http://trac.webkit.org/changeset/174863>
Comment 7 Brian Burg 2014-10-29 20:10:14 PDT
*** Bug 136296 has been marked as a duplicate of this bug. ***