Bug 189292

Summary: isAsyncGeneratorMethodParseMode() should check for SourceParseMode::AsyncGeneratorWrapperMethodMode.
Product: WebKit Reporter: Mark Lam <mark.lam>
Component: JavaScriptCoreAssignee: Mark Lam <mark.lam>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, fpizlo, keith_miller, msaboff, rmorisset, saam, tzagallo, webkit-bug-importer, ysuzuki
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
proposed patch.
saam: review+, saam: commit-queue-
patch for landing. none

Description Mark Lam 2018-09-04 17:55:13 PDT
Currently, it's checking for SourceParseMode::AsyncGeneratorWrapperFunctionMode instead.

<rdar://problem/38907433>
Comment 1 Mark Lam 2018-09-04 18:04:01 PDT
Created attachment 348881 [details]
proposed patch.
Comment 2 Saam Barati 2018-09-04 21:49:09 PDT
Comment on attachment 348881 [details]
proposed patch.

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

r=me

Did this make any test262 tests pass?

> Source/JavaScriptCore/ChangeLog:9
> +        Previously, it's checking for SourceParseMode::AsyncGeneratorWrapperFunctionMode instead.

You need to rewrite this sentence

> Source/JavaScriptCore/parser/ParserModes.h:192
> +        SourceParseMode::AsyncGeneratorWrapperMethodMode).contains(parseMode);

Why on two lines? The indentation here looks weird (though perhaps that’s a bugzilla bug)
Comment 3 Mark Lam 2018-09-04 21:55:49 PDT
Comment on attachment 348881 [details]
proposed patch.

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

Thanks for the review.

>> Source/JavaScriptCore/parser/ParserModes.h:192
>> +        SourceParseMode::AsyncGeneratorWrapperMethodMode).contains(parseMode);
> 
> Why on two lines? The indentation here looks weird (though perhaps that’s a bugzilla bug)

I'll put it back on 1 line.
Comment 4 Mark Lam 2018-09-04 22:17:35 PDT
(In reply to Saam Barati from comment #2)
> Did this make any test262 tests pass?

I'm running test262 now.
Comment 5 Mark Lam 2018-09-04 23:14:40 PDT
(In reply to Mark Lam from comment #4)
> (In reply to Saam Barati from comment #2)
> > Did this make any test262 tests pass?
> 
> I'm running test262 now.

The test262 results:

NEW PASS test/intl402/PluralRules/prototype/resolvedOptions/pluralCategories.js (strict mode)
NEW PASS test/intl402/PluralRules/prototype/resolvedOptions/pluralCategories.js (default)
NEW PASS test/intl402/NumberFormat/prototype/formatToParts/length.js (strict mode)
NEW PASS test/intl402/NumberFormat/prototype/formatToParts/formatToParts.js (strict mode)
NEW PASS test/intl402/NumberFormat/prototype/formatToParts/default-parameter.js (strict mode)
NEW PASS test/intl402/NumberFormat/prototype/formatToParts/name.js (strict mode)
NEW PASS test/intl402/NumberFormat/prototype/formatToParts/return-abrupt-tonumber.js (strict mode)
NEW PASS test/intl402/NumberFormat/prototype/formatToParts/main.js (strict mode)
NEW PASS test/intl402/NumberFormat/prototype/formatToParts/length.js (default)
NEW PASS test/intl402/NumberFormat/prototype/formatToParts/name.js (default)
NEW PASS test/intl402/NumberFormat/prototype/formatToParts/formatToParts.js (default)
NEW PASS test/intl402/NumberFormat/prototype/formatToParts/default-parameter.js (default)
NEW PASS test/intl402/NumberFormat/prototype/formatToParts/return-abrupt-tonumber.js (default)
NEW PASS test/intl402/NumberFormat/prototype/formatToParts/main.js (default)

49176 tests run
5031 test files skipped
2375 tests failed in total
0 tests newly fail
14 tests newly pass

These 14 new passes are all in the intl402 spec.  I don't think they have anything to do with my patch.
Comment 6 Mark Lam 2018-09-04 23:17:20 PDT
Created attachment 348896 [details]
patch for landing.
Comment 7 WebKit Commit Bot 2018-09-05 01:14:51 PDT
Comment on attachment 348896 [details]
patch for landing.

Clearing flags on attachment: 348896

Committed r235662: <https://trac.webkit.org/changeset/235662>
Comment 8 WebKit Commit Bot 2018-09-05 01:14:53 PDT
All reviewed patches have been landed.  Closing bug.