[WHLSL] Parsing and lexing the standard library is slow

• Component: WebGPU
• Status: RESOLVED FIXED

[Myles C. Maxfield]

Parsing the standard library is slow

[Radar WebKit Bug Importer]

<rdar://problem/50746335>

[Robin Morisset]

Created attachment 370292 [details]
WIP

[Robin Morisset]

Created attachment 370799 [details]
Patch

[Robin Morisset]

Comment on attachment 370799 [details]
Patch

The patch does not apply cleanly for some reason.. I will send it for review again once it is fixed.

[Robin Morisset]

Created attachment 370805 [details]
Patch

Properly rebased

[EWS Watchlist]

Attachment 370805 [details] did not pass style-queue:


ERROR: Source/WebCore/Modules/webgpu/WHLSL/WHLSLParser.cpp:61:  Missing space before ( in switch(  [whitespace/parens] [5]
ERROR: Source/WebCore/Modules/webgpu/WHLSL/WHLSLParser.cpp:475:  A case label should not be indented, but line up with its switch statement.  [whitespace/indent] [4]
ERROR: Source/WebCore/Modules/webgpu/WHLSL/WHLSLParser.cpp:507:  A case label should not be indented, but line up with its switch statement.  [whitespace/indent] [4]
ERROR: Source/WebCore/Modules/webgpu/WHLSL/WHLSLParser.cpp:681:  Missing space before ( in switch(  [whitespace/parens] [5]
ERROR: Source/WebCore/Modules/webgpu/WHLSL/WHLSLParser.cpp:930:  This { should be at the end of the previous line  [whitespace/braces] [4]
ERROR: Source/WebCore/Modules/webgpu/WHLSL/WHLSLParser.cpp:981:  Missing space before ( in switch(  [whitespace/parens] [5]
ERROR: Source/WebCore/Modules/webgpu/WHLSL/WHLSLParser.cpp:1202:  Missing space before ( in switch(  [whitespace/parens] [5]
ERROR: Source/WebCore/Modules/webgpu/WHLSL/WHLSLParser.cpp:1253:  An else statement can be removed when the prior "if" concludes with a return, break, continue or goto statement.  [readability/control_flow] [4]
ERROR: Source/WebCore/Modules/webgpu/WHLSL/WHLSLParser.cpp:1469:  This { should be at the end of the previous line  [whitespace/braces] [4]
ERROR: Source/WebCore/Modules/webgpu/WHLSL/WHLSLParser.cpp:1493:  This { should be at the end of the previous line  [whitespace/braces] [4]
ERROR: Source/WebCore/Modules/webgpu/WHLSL/WHLSLParser.cpp:1567:  Multi line control clauses should use braces.  [whitespace/braces] [4]
ERROR: Source/WebCore/Modules/webgpu/WHLSL/WHLSLParser.cpp:1602:  This { should be at the end of the previous line  [whitespace/braces] [4]
ERROR: Source/WebCore/Modules/webgpu/WHLSL/WHLSLParser.cpp:1654:  This { should be at the end of the previous line  [whitespace/braces] [4]
ERROR: Source/WebCore/Modules/webgpu/WHLSL/WHLSLParser.cpp:1709:  This { should be at the end of the previous line  [whitespace/braces] [4]
ERROR: Source/WebCore/Modules/webgpu/WHLSL/WHLSLParser.cpp:1743:  This { should be at the end of the previous line  [whitespace/braces] [4]
ERROR: Source/WebCore/Modules/webgpu/WHLSL/WHLSLParser.cpp:1777:  This { should be at the end of the previous line  [whitespace/braces] [4]
Total errors found: 16 in 4 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Myles C. Maxfield]

Comment on attachment 370805 [details]
Patch

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

> Source/WebCore/ChangeLog:1767
> [phone]  Robin Morisset  <[e-mail]>

Changelogs should go at the top of the file.

> Source/WebCore/Modules/webgpu/WHLSL/WHLSLLexer.h:184
> +        m_ringBufferIndex = !m_ringBufferIndex;

Nit: this could be made more clear by saying (m_ringBufferIndex + 1) % 2. Maybe one day we'll want to have 3 tokens of lookahead.

> Source/WebCore/Modules/webgpu/WHLSL/WHLSLLexer.h:195
> +        return m_ringBuffer[!m_ringBufferIndex];

Ditto

> Source/WebCore/Modules/webgpu/WHLSL/WHLSLLexer.h:205
>      State state() const

We should add a FIXME with a link to a bugzilla bug about removing the last of the backtracking, at which point we can delete these functions. The FIXME could go here or it could go where the backtracking occurs (or both).

> Source/WebCore/Modules/webgpu/WHLSL/WHLSLParser.cpp:58
> +        auto token = m_lexer.peek();

Can't you just call peek( ) instead of going through the parser? Having fewer funnel points is better.

> Source/WebCore/Modules/webgpu/WHLSL/WHLSLParser.cpp:87
> +            ASSERT(m_mode == Mode::StandardLibrary);

Shouldn't this be a parse error, rather than an ASSERT()? We can't stop authors from putting the token "native" in their source.

> Source/WebCore/Modules/webgpu/WHLSL/WHLSLParser.cpp:365
> +    if (m_lexer.peek()->type != Lexer::Token::Type::Identifier || m_lexer.peekFurther()->type == Lexer::Token::Type::FullStop) {

Did you mechanically generate these tokens using a tool? Or did you figure it out by looking through the grammar yourself?

If we add a new type of token, are we going to have to modify lots of places throughout the parser?

> Source/WebCore/Modules/webgpu/WHLSL/WHLSLParser.cpp:382
> +    auto greaterThanSign = tryType(Lexer::Token::Type::GreaterThanSign);
> +    if (greaterThanSign)
> +        return typeArguments;

Should there be a macro for this pattern too? It's used in quite a few places.

> Source/WebCore/Modules/webgpu/WHLSL/WHLSLParser.cpp:703
> +    while (auto next = tryType(Lexer::Token::Type::Qualifier)) {

The rare "variable declaration in a while loop condition"! I like it!

> Source/WebCore/Modules/webgpu/WHLSL/WHLSLParser.cpp:744
>          auto structureElement = backtrackingScope<Expected<AST::StructureElement, Error>>([&]() {

😔

> Source/WebCore/Modules/webgpu/WHLSL/WHLSLParser.cpp:986
> +        return parseVertexFragmentFunctionDeclaration();

Nit: VertexOrFragment

> Source/WebCore/Modules/webgpu/WHLSL/WHLSLParser.cpp:1091
>      auto parseRemainder = [&](Variant<AST::VariableDeclarationsStatement, UniqueRef<AST::Expression>>&& initialization) -> Expected<AST::ForLoop, Error> {

With the new design, is it possible to get rid of this lambda and put its contents directly into parseForLoop()?

> Source/WebCore/Modules/webgpu/WHLSL/WHLSLParser.cpp:1335
> +    auto origin = peek();
> +    if (!origin)
> +        return Unexpected<Error>(origin.error());

Can be macro-ized?

> Source/WebCore/Modules/webgpu/WHLSL/WHLSLParser.cpp:1667
> +        Vector<UniqueRef<AST::Expression>> callArguments;
> +        callArguments.append(WTFMove(previous));
> +        callArguments.append(WTFMove(*next));

Can we say "callArguments { WTFMove(previous), WTFMove(*next) }"?

> Source/WebCore/Modules/webgpu/WHLSL/WHLSLParser.cpp:1718
> +        Vector<UniqueRef<AST::Expression>> callArguments;
> +        callArguments.append(WTFMove(previous));
> +        callArguments.append(WTFMove(*next));

Ditto

> Source/WebCore/Modules/webgpu/WHLSL/WHLSLParser.cpp:1752
> +        Vector<UniqueRef<AST::Expression>> callArguments;
> +        callArguments.append(WTFMove(previous));
> +        callArguments.append(WTFMove(*next));

Ditto

> Source/WebCore/Modules/webgpu/WHLSL/WHLSLParser.cpp:1787
> +        Vector<UniqueRef<AST::Expression>> callArguments;
> +        callArguments.append(WTFMove(previous));
> +        callArguments.append(WTFMove(*next));

Ditto

> Source/WebCore/Modules/webgpu/WHLSL/WHLSLParser.cpp:1827
> +            if (isEffectful)
> +                *isEffectful = true;

If possible, output values should be part of the return type, not out-params.

> Source/WebCore/Modules/webgpu/WHLSL/WHLSLParser.cpp:1913
> +            return fail("Could not read next token in the callExpression case of parsePossibleSuffix()"_str, TryToPeek::No);

Isn't a better error message "the term isn't effectful" or "need a semicolon" or something like that?

> Source/WebCore/Modules/webgpu/WHLSL/WHLSLParser.h:123
> +        void dump(PrintStream& out) const
> +        {
> +            out.print(error);
> +        }

Is this ever called? Maybe it should be behind #ifndef NDEBUG

Also, possibly rename to dumpError()

[Robin Morisset]

Thank you for the review!

(In reply to Myles C. Maxfield from comment #7)
> Comment on attachment 370805 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=370805&action=review
> 
> > Source/WebCore/ChangeLog:1767
> > [phone]  Robin Morisset  <[e-mail]>
> 
> Changelogs should go at the top of the file.

will fix

> > Source/WebCore/Modules/webgpu/WHLSL/WHLSLLexer.h:184
> > +        m_ringBufferIndex = !m_ringBufferIndex;
> 
> Nit: this could be made more clear by saying (m_ringBufferIndex + 1) % 2.
> Maybe one day we'll want to have 3 tokens of lookahead.

will fix.

> > Source/WebCore/Modules/webgpu/WHLSL/WHLSLLexer.h:205
> >      State state() const
> 
> We should add a FIXME with a link to a bugzilla bug about removing the last
> of the backtracking, at which point we can delete these functions. The FIXME
> could go here or it could go where the backtracking occurs (or both).

Will do.

> > Source/WebCore/Modules/webgpu/WHLSL/WHLSLParser.cpp:58
> > +        auto token = m_lexer.peek();
> 
> Can't you just call peek( ) instead of going through the parser? Having
> fewer funnel points is better.

I will try to fix it.

> > Source/WebCore/Modules/webgpu/WHLSL/WHLSLParser.cpp:87
> > +            ASSERT(m_mode == Mode::StandardLibrary);
> 
> Shouldn't this be a parse error, rather than an ASSERT()? We can't stop
> authors from putting the token "native" in their source.

Ah, I forgot to open a bug about that. The lexer should only treat native as a special keyword while parsing the standard library. It should be a normal identifier in user code.

> > Source/WebCore/Modules/webgpu/WHLSL/WHLSLParser.cpp:365
> > +    if (m_lexer.peek()->type != Lexer::Token::Type::Identifier || m_lexer.peekFurther()->type == Lexer::Token::Type::FullStop) {
> 
> Did you mechanically generate these tokens using a tool? Or did you figure
> it out by looking through the grammar yourself?
> 
> If we add a new type of token, are we going to have to modify lots of places
> throughout the parser?

I figured them out by looking myself.. I agree it is ugly as hell, but supporting some new tool in the build system would be a pain, and most tools have some restriction that could make it harder to extend the language.

> > Source/WebCore/Modules/webgpu/WHLSL/WHLSLParser.cpp:382
> > +    auto greaterThanSign = tryType(Lexer::Token::Type::GreaterThanSign);
> > +    if (greaterThanSign)
> > +        return typeArguments;
> 
> Should there be a macro for this pattern too? It's used in quite a few
> places.

maybe, but I have not seen it in that many places, and I don't want to make the code harder to read by using rare macros.

> > Source/WebCore/Modules/webgpu/WHLSL/WHLSLParser.cpp:986
> > +        return parseVertexFragmentFunctionDeclaration();
> 
> Nit: VertexOrFragment

Will fix.

> 
> > Source/WebCore/Modules/webgpu/WHLSL/WHLSLParser.cpp:1091
> >      auto parseRemainder = [&](Variant<AST::VariableDeclarationsStatement, UniqueRef<AST::Expression>>&& initialization) -> Expected<AST::ForLoop, Error> {
> 
> With the new design, is it possible to get rid of this lambda and put its
> contents directly into parseForLoop()?

Sadly not yet because there is still ambiguity between variable declarations and effectfulExpr, which will require a change to the array syntax to fix.

> > Source/WebCore/Modules/webgpu/WHLSL/WHLSLParser.cpp:1335
> > +    auto origin = peek();
> > +    if (!origin)
> > +        return Unexpected<Error>(origin.error());
> 
> Can be macro-ized?

I will look.

> > Source/WebCore/Modules/webgpu/WHLSL/WHLSLParser.cpp:1667
> > +        Vector<UniqueRef<AST::Expression>> callArguments;
> > +        callArguments.append(WTFMove(previous));
> > +        callArguments.append(WTFMove(*next));
> 
> Can we say "callArguments { WTFMove(previous), WTFMove(*next) }"?

Will do.

> > Source/WebCore/Modules/webgpu/WHLSL/WHLSLParser.cpp:1827
> > +            if (isEffectful)
> > +                *isEffectful = true;
> 
> If possible, output values should be part of the return type, not out-params.

The problem is that this is not meaningful out of the parser, so I don't want to put it in the expression ast node. I could make this function return a std::pair<AST::Expression, bool>, but it would make all places that call it (including some that don't care about isEffectful) more complicated. I can do it if you think it really matters, but an out param seemed the simplest way.

> > Source/WebCore/Modules/webgpu/WHLSL/WHLSLParser.cpp:1913
> > +            return fail("Could not read next token in the callExpression case of parsePossibleSuffix()"_str, TryToPeek::No);
> 
> Isn't a better error message "the term isn't effectful" or "need a
> semicolon" or something like that?

Yeah, this error message is terrible. Will fix.

> > Source/WebCore/Modules/webgpu/WHLSL/WHLSLParser.h:123
> > +        void dump(PrintStream& out) const
> > +        {
> > +            out.print(error);
> > +        }
> 
> Is this ever called? Maybe it should be behind #ifndef NDEBUG
> 
> Also, possibly rename to dumpError()

It must be called dump(), so it can be called implicitly by dataLog and friends. And I frequently need dataLog while in Release mode. It is not currently called because I removed all the dataLog I had generously sprinkled throughout the parser, but it will be useful if we ever need to debug anything in the parser again.

[Robin Morisset]

Created attachment 370907 [details]
Patch for landing

[WebKit Commit Bot]

Comment on attachment 370907 [details]
Patch for landing

Rejecting attachment 370907 [details] from commit-queue.

Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.webkit.org', '--bot-id=webkit-cq-02', 'apply-attachment', '--no-update', '--non-interactive', 370907, '--port=mac']" exit_code: 2 cwd: /Volumes/Data/EWS/WebKit

Logging in as [e-mail]...
Fetching: https://bugs.webkit.org/attachment.cgi?id=370907&action=edit
Fetching: https://bugs.webkit.org/show_bug.cgi?id=192890&ctype=xml&excludefield=attachmentdata
Processing 1 patch from 1 bug.
Processing patch 370907 from bug 192890.
Fetching: https://bugs.webkit.org/attachment.cgi?id=370907
Failed to run "[u'/Volumes/Data/EWS/WebKit/Tools/Scripts/svn-apply', '--force']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit

Parsed 4 diffs from patch file(s).
patching file Source/WebCore/ChangeLog
Hunk #1 succeeded at 1 with fuzz 3.
patching file Source/WebCore/Modules/webgpu/WHLSL/WHLSLLexer.h
patching file Source/WebCore/Modules/webgpu/WHLSL/WHLSLParser.cpp
Hunk #1 FAILED at 38.
Hunk #19 FAILED at 871.
2 out of 43 hunks FAILED -- saving rejects to file Source/WebCore/Modules/webgpu/WHLSL/WHLSLParser.cpp.rej
patching file Source/WebCore/Modules/webgpu/WHLSL/WHLSLParser.h

Failed to run "[u'/Volumes/Data/EWS/WebKit/Tools/Scripts/svn-apply', '--force']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit

Full output: https://webkit-queues.webkit.org/results/12323670

[Robin Morisset]

Created attachment 370913 [details]
Patch for landing

Picking the right patch from the right directory this time.

[WebKit Commit Bot]

Comment on attachment 370913 [details]
Patch for landing

Clearing flags on attachment: 370913

Committed r245883: <https://trac.webkit.org/changeset/245883>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Truitt Savell]

The changes in https://trac.webkit.org/changeset/245883/webkit

appears to have broken 6 test on WK2:
webgpu/whlsl-arbitrary-vertex-attribute-locations.html
webgpu/whlsl-dereference-pointer-should-type-check.html
webgpu/whlsl-dont-crash-parsing-enum.html
webgpu/whlsl-dot-expressions.html
webgpu/whlsl-nested-dot-expression-rvalue.html
webgpu/whlsl.html

I reproduced this by running these test on 245883 which all failed, and on 245882 which all passed. Can this be resolved soon? 

Results:
https://build.webkit.org/results/Apple%20Mojave%20Release%20WK2%20(Tests)/r245884%20(4380)/results.html

[Truitt Savell]

Reverted r245883 for reason:

Caused 6 webgpu/ layout test failures.

Committed r245897: <https://trac.webkit.org/changeset/245897>

[EWS Watchlist]

Comment on attachment 370913 [details]
Patch for landing

Attachment 370913 [details] did not pass mac-wk2-ews (mac-wk2):
Output: https://webkit-queues.webkit.org/results/12330696

New failing tests:
webgpu/whlsl-dereference-pointer-should-type-check.html
webgpu/whlsl-dot-expressions.html
webgpu/whlsl-arbitrary-vertex-attribute-locations.html
webgpu/whlsl.html
webgpu/whlsl-dont-crash-parsing-enum.html
webgpu/whlsl-nested-dot-expression-rvalue.html

[EWS Watchlist]

Created attachment 370971 [details]
Archive of layout-test-results from ews105 for mac-highsierra-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews105  Port: mac-highsierra-wk2  Platform: Mac OS X 10.13.6

[Ryan Haddad]

These tests were also failing an assertion on debug bots:
https://build.webkit.org/results/Apple%20High%20Sierra%20Debug%20WK2%20(Tests)/r245892%20(7962)/results.html

ASSERTION FAILED: !failure
./Modules/webgpu/WHLSL/WHLSLPrepare.cpp(101) : Optional<WebCore::WHLSL::Program> WebCore::WHLSL::prepareShared(WTF::String &)
1   0x39aec8c09 WTFCrash
2   0x38800739b WTFCrashWithInfo(int, char const*, char const*, int)
3   0x389d58ca5 WebCore::WHLSL::prepareShared(WTF::String&)
4   0x389d589d7 WebCore::WHLSL::prepare(WTF::String&, WebCore::WHLSL::RenderPipelineDescriptor&)
5   0x3888c4779 WebCore::trySetWHLSLFunctionsForPipelineDescriptor(char const*, MTLRenderPipelineDescriptor*, WebCore::GPURenderPipelineDescriptor const&, WTF::String, WebCore::GPUDevice const&)
6   0x3888c3496 WebCore::trySetFunctionsForPipelineDescriptor(char const*, MTLRenderPipelineDescriptor*, WebCore::GPURenderPipelineDescriptor const&, WebCore::GPUDevice const&)
7   0x3888bf091 WebCore::tryCreateMtlRenderPipelineState(char const*, WebCore::GPURenderPipelineDescriptor const&, WebCore::GPUDevice const&)
8   0x3888be83b WebCore::GPURenderPipeline::tryCreate(WebCore::GPUDevice const&, WebCore::GPURenderPipelineDescriptor const&)
9   0x38b4b02b4 WebCore::GPUDevice::tryCreateRenderPipeline(WebCore::GPURenderPipelineDescriptor const&) const
10  0x389c6f280 WebCore::WebGPUDevice::createRenderPipeline(WebCore::WebGPURenderPipelineDescriptor const&) const
11  0x3895403c6 WebCore::jsWebGPUDevicePrototypeFunctionCreateRenderPipelineBody(JSC::ExecState*, WebCore::JSWebGPUDevice*, JSC::ThrowScope&)
12  0x389535150 long long WebCore::IDLOperation<WebCore::JSWebGPUDevice>::call<&(WebCore::jsWebGPUDevicePrototypeFunctionCreateRenderPipelineBody(JSC::ExecState*, WebCore::JSWebGPUDevice*, JSC::ThrowScope&)), (WebCore::CastedThisErrorBehavior)0>(JSC::ExecState&, char const*)
13  0x389534e3c WebCore::jsWebGPUDevicePrototypeFunctionCreateRenderPipeline(JSC::ExecState*)
14  0x28b308d316b
15  0x39b3e7781 llint_entry
16  0x39b3e76e2 llint_entry
17  0x39b3e76e2 llint_entry
18  0x39b3e7781 llint_entry
19  0x39b3d42d3 vmEntryToJavaScript
20  0x39c05503e JSC::JITCode::execute(JSC::VM*, JSC::ProtoCallFrame*)
21  0x39c05566f JSC::Interpreter::executeCall(JSC::ExecState*, JSC::JSObject*, JSC::CallType, JSC::CallData const&, JSC::JSValue, JSC::ArgList const&)
22  0x39c32f47c JSC::call(JSC::ExecState*, JSC::JSValue, JSC::CallType, JSC::CallData const&, JSC::JSValue, JSC::ArgList const&)
23  0x39c32f75b JSC::profiledCall(JSC::ExecState*, JSC::ProfilingReason, JSC::JSValue, JSC::CallType, JSC::CallData const&, JSC::JSValue, JSC::ArgList const&)
24  0x39c4b7e9d JSC::JSMicrotask::run(JSC::ExecState*)
25  0x389f91f27 WebCore::JSExecState::runTask(JSC::ExecState*, JSC::Microtask&)
26  0x389fa37f0 WebCore::JSMicrotaskCallback::call()
27  0x389fa366d WebCore::JSDOMWindowBase::queueTaskToEventLoop(JSC::JSGlobalObject&, WTF::Ref<JSC::Microtask, WTF::DumbPtrTraits<JSC::Microtask> >&&)::$_1::operator()()
28  0x389fa35c9 WTF::Detail::CallableWrapper<WebCore::JSDOMWindowBase::queueTaskToEventLoop(JSC::JSGlobalObject&, WTF::Ref<JSC::Microtask, WTF::DumbPtrTraits<JSC::Microtask> >&&)::$_1, void>::call()
29  0x388013b3d WTF::Function<void ()>::operator()() const
30  0x38a438cb7 WebCore::ActiveDOMCallbackMicrotask::run()
31  0x38a5eaa6d WebCore::MicrotaskQueue::performMicrotaskCheckpoint()
LEAK: 3 WebPageProxy

[Robin Morisset]

Created attachment 371228 [details]
Patch for landing

Found the problem: when applying some of Myles suggestion, I accidentally used m_lexer.peek() in the definition of Parser::peekFurther, instead of m_lexer.peekFurther()

[WebKit Commit Bot]

Comment on attachment 371228 [details]
Patch for landing

Clearing flags on attachment: 371228

Committed r246052: <https://trac.webkit.org/changeset/246052>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Truitt Savell]

The changes in https://trac.webkit.org/changeset/246052/webkit

has caused 3 webgpu/ tests to fail:
webgpu/whlsl-ensure-proper-variable-lifetime-3.html 
webgpu/whlsl-ensure-proper-variable-lifetime.html 
webgpu/whlsl-ensure-proper-variable-lifetime-2.html

History:
http://webkit-test-results.webkit.org/dashboards/flakiness_dashboard.html#showAllRuns=true&tests=webgpu%2Fwhlsl-ensure-proper-variable-lifetime-3.html%20%20webgpu%2Fwhlsl-ensure-proper-variable-lifetime.html%20%20webgpu%2Fwhlsl-ensure-proper-variable-lifetime-2.html

Results:
https://build.webkit.org/results/Apple%20Mojave%20Release%20WK2%20(Tests)/r246059%20(4469)/results.html

[Truitt Savell]

Reverted r246052 for reason:

Caused 3 webgpu/ failures.

Committed r246108: <https://trac.webkit.org/changeset/246108>

[Robin Morisset]

Created attachment 371452 [details]
Patch for landing

Found two bugs:
- We were only peeking in the first iteration of the loop in parseSwitchStatement instead of every iteration.
- parsePossibleTernaryConditional was consuming a token it should only peek at.

Fixed both, and now all the tests that were failing pass.

I also removed a bit of dead code, and slightly improved the error message when we fail to parse.

[WebKit Commit Bot]

Comment on attachment 371452 [details]
Patch for landing

Clearing flags on attachment: 371452

Committed r246135: <https://trac.webkit.org/changeset/246135>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.