Bug 190637 - delete expression should not throw without a reference
Summary: delete expression should not throw without a reference
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Ross Kirsling
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2018-10-16 13:42 PDT by Leo Balter
Modified: 2018-10-18 10:32 PDT (History)
11 users (show)

See Also:


Attachments
Patch (5.15 KB, patch)
2018-10-17 15:11 PDT, Ross Kirsling
no flags Details | Formatted Diff | Diff
Patch for landing (6.04 KB, patch)
2018-10-18 09:55 PDT, Ross Kirsling
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Leo Balter 2018-10-16 13:42:04 PDT
This test is wrong:

https://github.com/WebKit/webkit/blob/master/LayoutTests/js/script-tests/basic-strict-mode.js#L153

```
"use strict";
var a = { b: 42 };
delete void a.b;
```

The delete expression should not throw, but it throws a syntax error requiring a reference expression.

The specs says the expression should return true when the right hand expression is not a reference:

https://tc39.github.io/ecma262/#sec-delete-operator-runtime-semantics-evaluation

the void expression will normally return undefined and this value is not a reference.

This bug is only seen on JSC. Other major engines are showing the expected behavior.

```
eshost -x '"use strict"; var a = { b: 42 }; delete void a.b'
#### v8-harmony


#### v8


#### jsc
SyntaxError: The delete operator requires a reference expression.

#### spidermonkey


#### ch


#### node


```

The same happens if the expression is `delete void 0` or `delete typeof 0` but changing the expression from `delete (void 0)` gets the correct behavior.


I'm sending some tests to Test262 to support this and prevent it from other occurrences.
Comment 1 Ross Kirsling 2018-10-17 15:11:16 PDT
Created attachment 352650 [details]
Patch
Comment 2 Ross Kirsling 2018-10-17 15:30:34 PDT
This is really interesting -- looks like that SyntaxError has been in JSC's parser forever, yet even looking through old editions of ECMA-262, it doesn't seem like it was ever spec-compliant.

I don't believe there are any test cases to be added here, since we're just removing a strict-mode check that seemingly ought to have never existed.
Comment 3 Yusuke Suzuki 2018-10-18 00:02:37 PDT
Comment on attachment 352650 [details]
Patch

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

r=me. Could you update test262 expectations.yaml too?

> LayoutTests/js/script-tests/basic-strict-mode.js:-153
> -shouldBeSyntaxError("'use strict'; if (0) delete +a.b");
> -shouldBeSyntaxError("'use strict'; if (0) delete ++a.b");
> -shouldBeSyntaxError("'use strict'; if (0) delete void a.b");

Lets'
Comment 4 Yusuke Suzuki 2018-10-18 00:03:22 PDT
Comment on attachment 352650 [details]
Patch

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

>> LayoutTests/js/script-tests/basic-strict-mode.js:-153
>> -shouldBeSyntaxError("'use strict'; if (0) delete void a.b");
> 
> Lets'

Oops.

Let's keep these tests as follows. `shouldBeXXX(...)`
Comment 5 Ross Kirsling 2018-10-18 09:55:09 PDT
Created attachment 352705 [details]
Patch for landing
Comment 6 WebKit Commit Bot 2018-10-18 10:31:30 PDT
Comment on attachment 352705 [details]
Patch for landing

Clearing flags on attachment: 352705

Committed r237259: <https://trac.webkit.org/changeset/237259>
Comment 7 WebKit Commit Bot 2018-10-18 10:31:31 PDT
All reviewed patches have been landed.  Closing bug.
Comment 8 Radar WebKit Bug Importer 2018-10-18 10:32:41 PDT
<rdar://problem/45375223>