Summary: | Web Replay: code generator should take supplemental specifications and allow cross-framework references | ||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Brian Burg <burg> | ||||||||||||||||
Component: | WebCore Misc. | Assignee: | Brian Burg <burg> | ||||||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||||||
Severity: | Normal | CC: | bfulgham, commit-queue, joepeck | ||||||||||||||||
Priority: | P2 | ||||||||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||||||||
Hardware: | Unspecified | ||||||||||||||||||
OS: | Unspecified | ||||||||||||||||||
Bug Depends on: | |||||||||||||||||||
Bug Blocks: | 136294, 136297 | ||||||||||||||||||
Attachments: |
|
Description
Brian Burg
2014-08-27 14:44:44 PDT
*** Bug 136311 has been marked as a duplicate of this bug. *** Created attachment 244879 [details]
Patch
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`) Attachment 244879 [details] did not pass style-queue:
ERROR: Source/JavaScriptCore/replay/scripts/tests/expected/generate-enum-with-guard.json-TestReplayInputs.h:71: The parameter name "phase" adds no information, so it should be removed. [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/replay/scripts/CodeGeneratorReplayInputs.py:795: whitespace before '}' [pep8/E202] [5]
ERROR: Source/JavaScriptCore/replay/scripts/tests/expected/generate-enum-encoding-helpers-with-guarded-values.json-TestReplayInputs.h:60: The parameter name "button" adds no information, so it should be removed. [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/replay/scripts/tests/expected/generate-enum-encoding-helpers.json-TestReplayInputs.h:81: The parameter name "button" adds no information, so it should be removed. [readability/parameter_name] [5]
Total errors found: 4 in 41 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 244922 [details]
Patch (w/ Windows fix)
Comment on attachment 244922 [details] Patch (w/ Windows fix) View in context: https://bugs.webkit.org/attachment.cgi?id=244922&action=review r=me > Source/WebCore/ChangeLog:11 > + * replay/WebInputs.json: Moved common types to JSINputs.json. Typo: "JSINputs.json" > Source/JavaScriptCore/replay/scripts/CodeGeneratorReplayInputs.py:787 > + if len(self.setting('exportMacro')) > 0: Would just "if len(...):" do? > Source/JavaScriptCore/replay/scripts/CodeGeneratorReplayInputs.py:981 > + if len(input_filepaths) == 0: This could also be "if !len(...):". Do what you think is clearer. > Source/WebCore/DerivedSources.make:1164 > INPUT_GENERATOR_SPECIFICATIONS = \ > $(WebCore)/replay/WebInputs.json \ > + $(WebReplayScripts)/JSInputs.json \ > # I wonder if we should split this up into two variables so one day someone doesn't try to sort these in a different order and inadvertently break things since the order here is important. Nothing to do here unless you want to. Comment on attachment 244922 [details] Patch (w/ Windows fix) View in context: https://bugs.webkit.org/attachment.cgi?id=244922&action=review Will rebase and EWS one more time before landing. >> Source/JavaScriptCore/replay/scripts/CodeGeneratorReplayInputs.py:787 >> + if len(self.setting('exportMacro')) > 0: > > Would just "if len(...):" do? There are only a few places in WebKit python code that use such implicit conversions with len(), so I'm shy to add more. >> Source/WebCore/DerivedSources.make:1164 >> # > > I wonder if we should split this up into two variables so one day someone doesn't try to sort these in a different order and inadvertently break things since the order here is important. > > Nothing to do here unless you want to. The order is *not* important. Specifications are not type-checked (i.e., try to lookup all type references between files) until all input files are parsed. Maybe I should put that into the changelog. Created attachment 244924 [details]
Patch (for EWS)
Created attachment 244925 [details]
Patch (for EWS)
Attachment 244925 [details] did not pass style-queue:
ERROR: Source/JavaScriptCore/replay/scripts/tests/expected/generate-enum-with-guard.json-TestReplayInputs.h:71: The parameter name "phase" adds no information, so it should be removed. [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/replay/scripts/CodeGeneratorReplayInputs.py:806: whitespace before '}' [pep8/E202] [5]
ERROR: Source/JavaScriptCore/replay/scripts/tests/expected/generate-enum-encoding-helpers-with-guarded-values.json-TestReplayInputs.h:60: The parameter name "button" adds no information, so it should be removed. [readability/parameter_name] [5]
ERROR: Source/JavaScriptCore/replay/scripts/tests/expected/generate-enum-encoding-helpers.json-TestReplayInputs.h:81: The parameter name "button" adds no information, so it should be removed. [readability/parameter_name] [5]
Total errors found: 4 in 42 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 244927 [details]
Patch (always Windows...)
Created attachment 244935 [details]
Patch (for EWS, 5)
Created attachment 244969 [details]
Patch (EWS)
Committed r178714: <http://trac.webkit.org/changeset/178714> (In reply to comment #14) > Committed r178714: <http://trac.webkit.org/changeset/178714> Based on the reviewed patch with fix for windows (locally confirmed to fix known build issue). Landed manually since I was unable to convince EWS to apply changes to copy-files.cmd, which address the windows issue. I'll be watching in case Windows still has problems. |