Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Remove oldest ES6 feature switches #6

Closed
ianwjhalliday opened this issue Jan 6, 2016 · 6 comments
Closed

Remove oldest ES6 feature switches #6

ianwjhalliday opened this issue Jan 6, 2016 · 6 comments
Assignees
@tcare
Labels
Suggestion
Milestone
Backlog

Comments

@ianwjhalliday
Copy link
Collaborator

Here's my reasoning:

  • switches were provided for allowing disabling unstable features for engineering infrastructure and RI
  • old features are now stable and any bugs in them are not going to cause us to disable the feature
  • I cannot think of any reason else why we would want to disable a given feature
  • oldest switches do not necessarily even work anymore due to future changes requiring the older changes
    • e.g. let/const changed how we defer parse in a way that other features have taken a dependency on
  • old unused switches are thus now useless visual cruft

Switches that I believe are reasonable to remove with no risk at this time:

  • proto
  • Map
  • Set
  • WeakMap
  • ES6WeakSet
  • DefineGetterSetter
  • LetConst
  • ES6Lambda
  • ES6Math
  • ES6Object
  • ES6Number
  • ES6String

Switches that I think are reasonable to remove but there might still be low risk in the feature having bugs or causing issues:

  • ES6Iterators
  • ES6NumericLiterals
  • ES6Promise
  • ES6StringTemplate
  • ES6Symbol
  • ES6Unscopables
@pleath
Copy link
Contributor

pleath commented Jan 6, 2016

+1 with an exclamation point

I’d argue for getting rid of switches that enable any feature that is not experimental in Chakra and is well-baked in the language – meaning that we’re not going to find ourselves in a world without feature x, even if there are kinks to work out in the spec. So, for instance, I’d get rid of ES6StringTemplate, too. Maybe everything on the list. This goes double for switches that enable syntax, because without the switch you can’t run a test that has the syntax in it, anyway, so such a switch is almost always useless noise.

@ianwjhalliday ianwjhalliday mentioned this issue Jan 13, 2016
Closed
ianwjhalliday pushed a commit to ianwjhalliday/ChakraCore that referenced this issue Jan 14, 2016
Remove -Map -Set -WeakMap and -ES6WeakSet feature switches
7861d98 
Re issue chakra-core#6

Also add ThreadConfigFlagsList.h to the VS solution.
ianwjhalliday pushed a commit to ianwjhalliday/ChakraCore that referenced this issue Jan 14, 2016
Remove -DefineGetterSetter feature switch
52de067 
Re issue chakra-core#6
ianwjhalliday pushed a commit to ianwjhalliday/ChakraCore that referenced this issue Jan 14, 2016
Remove -__proto__ feature switch
87f2325 
Re issue chakra-core#6
ianwjhalliday pushed a commit to ianwjhalliday/ChakraCore that referenced this issue Jan 14, 2016
Remove -ES6Lambda feature switch
505999d 
Re issue chakra-core#6
ianwjhalliday pushed a commit to ianwjhalliday/ChakraCore that referenced this issue Jan 14, 2016
Remove ES6 API extension feature switches
4b44541 
Re issue chakra-core#6

Removes -ES6Math -ES6Number -ES6Object and -ES6String API extension
feature switches.

Also removed unused ScriptContext function SupportsES3.
ianwjhalliday pushed a commit to ianwjhalliday/ChakraCore that referenced this issue Jan 14, 2016
Remove -ES6Iterators feature switch
c4f0159 
Re issue chakra-core#6

Also fixed up initialization of Array.prototype.values function.
ianwjhalliday pushed a commit to ianwjhalliday/ChakraCore that referenced this issue Jan 14, 2016
Remove -ES6StringTemplate feature switch
9342443 
Re issue chakra-core#6
ianwjhalliday pushed a commit to ianwjhalliday/ChakraCore that referenced this issue Jan 14, 2016
Remove -ES6Symbol feature switch
97df5d5 
Re issue chakra-core#6
ianwjhalliday pushed a commit to ianwjhalliday/ChakraCore that referenced this issue Jan 14, 2016
Remove -ES6NumericLiteral feature switch
b0c9623 
Re issue chakra-core#6
ianwjhalliday pushed a commit to ianwjhalliday/ChakraCore that referenced this issue Jan 14, 2016
Remove -ES6Promise feature switch
1c914ed 
Re issue chakra-core#6
ianwjhalliday pushed a commit to ianwjhalliday/ChakraCore that referenced this issue Jan 14, 2016
Remove -ES6Unscopables feature switch
7039393 
Re issue chakra-core#6
ianwjhalliday pushed a commit to ianwjhalliday/ChakraCore that referenced this issue Jan 14, 2016
Remove -LetConst feature switch
38605fa 
Re issue chakra-core#6
@ianwjhalliday ianwjhalliday mentioned this issue Jan 14, 2016
Merged
ianwjhalliday pushed a commit to ianwjhalliday/ChakraCore that referenced this issue Jan 25, 2016
Remove -Map -Set -WeakMap and -ES6WeakSet feature switches
5fdc088 
Re issue chakra-core#6

Also add ThreadConfigFlagsList.h to the VS solution.
ianwjhalliday pushed a commit to ianwjhalliday/ChakraCore that referenced this issue Jan 25, 2016
Remove -DefineGetterSetter feature switch
965cfc4 
Re issue chakra-core#6
ianwjhalliday pushed a commit to ianwjhalliday/ChakraCore that referenced this issue Jan 25, 2016
Remove -__proto__ feature switch
9f00b44 
Re issue chakra-core#6
ianwjhalliday pushed a commit to ianwjhalliday/ChakraCore that referenced this issue Jan 25, 2016
Remove -ES6Lambda feature switch
489eabf 
Re issue chakra-core#6
ianwjhalliday pushed a commit to ianwjhalliday/ChakraCore that referenced this issue Jan 25, 2016
Remove -ES6Iterators feature switch
13f7aba 
Re issue chakra-core#6

Also fixed up initialization of Array.prototype.values function.
ianwjhalliday pushed a commit to ianwjhalliday/ChakraCore that referenced this issue Jan 25, 2016
Remove -ES6StringTemplate feature switch
9d6e9cb 
Re issue chakra-core#6
ianwjhalliday pushed a commit to ianwjhalliday/ChakraCore that referenced this issue Jan 25, 2016
Remove -ES6Symbol feature switch
c5ef002 
Re issue chakra-core#6
ianwjhalliday pushed a commit to ianwjhalliday/ChakraCore that referenced this issue Jan 25, 2016
Remove -ES6NumericLiteral feature switch
33e8062 
Re issue chakra-core#6
ianwjhalliday pushed a commit to ianwjhalliday/ChakraCore that referenced this issue Jan 25, 2016
Remove -LetConst feature switch
7e78cfe 
Re issue chakra-core#6
ianwjhalliday pushed a commit to ianwjhalliday/ChakraCore that referenced this issue Jan 25, 2016
Remove -ES6Classes feature switch
cc01610 
Re issue chakra-core#6
ianwjhalliday pushed a commit to ianwjhalliday/ChakraCore that referenced this issue Jan 25, 2016
Remove -ES6ObjectLiterals feature switch
161b09b 
Re issue chakra-core#6
ianwjhalliday pushed a commit to ianwjhalliday/ChakraCore that referenced this issue Jan 25, 2016
Remove -ES6Generators feature switch
855146a 
Re issue chakra-core#6
ianwjhalliday pushed a commit to ianwjhalliday/ChakraCore that referenced this issue Jan 25, 2016
Remove -ES6Rest and -ES6Spread feature switches
73fb8a0 
Re issue chakra-core#6
ianwjhalliday pushed a commit to ianwjhalliday/ChakraCore that referenced this issue Feb 11, 2016
Remove -ES6NumericLiteral feature switch
70ede9a 
Re issue chakra-core#6
ianwjhalliday pushed a commit to ianwjhalliday/ChakraCore that referenced this issue Feb 11, 2016
Remove -LetConst feature switch
94632ec 
Re issue chakra-core#6
chakrabot pushed a commit that referenced this issue Feb 11, 2016
[MERGE #86] Remove old feature switches
dd8c63b 
Merge pull request #86 from ianwjhalliday:rmfeatureswitches
Addresses #6.  Though there are a few more switches remain that I believe would be good to remove.

Removes the following switches:
 - `LetConst`
 - `Map`
 - `Set`
 - `WeakMap`
 - `ES6WeakSet`
 - `DefineGetterSetter`
 - `__proto__`
 - `ES6Iterators`
 - `ES6Lambda`
 - `ES6NumericLiteral`
 - `ES6StringTemplate`
 - `ES6Symbol`
 - `TDZ`

With the removal of `-LetConst` a couple of predicates become always true and have been removed. They are:
 - `BindDeferredPidRefs()`
 - `IsBlockScopeEnabled()`

Parser binding had specific logic to handle catch scopes in non-block scope mode to support legacy ES5 behavior.  This has also been removed.

Finally `Parser::CheckStrictModeFncDeclNotSourceElement()` no longer did anything so it has also been removed.
ianwjhalliday pushed a commit to ianwjhalliday/ChakraCore that referenced this issue Feb 13, 2017
Remove ES6 API extension feature switches
d20b4c2 
Re issue chakra-core#6

Removes -ES6Math -ES6Number -ES6Object and -ES6String API extension
feature switches.

Also removed unused ScriptContext function SupportsES3.
ianwjhalliday pushed a commit to ianwjhalliday/ChakraCore that referenced this issue Feb 13, 2017
Remove -ES6Promise feature switch
b855430 
Re issue chakra-core#6
ianwjhalliday pushed a commit to ianwjhalliday/ChakraCore that referenced this issue Feb 13, 2017
Remove -ES6Unscopables feature switch
b142f22 
Re issue chakra-core#6
ianwjhalliday pushed a commit to ianwjhalliday/ChakraCore that referenced this issue Feb 13, 2017
Remove -ES6Classes feature switch
b5bd41c 
Re issue chakra-core#6
ianwjhalliday pushed a commit to ianwjhalliday/ChakraCore that referenced this issue Feb 13, 2017
Remove -ES6ObjectLiterals feature switch
a6aebc4 
Re issue chakra-core#6
ianwjhalliday pushed a commit to ianwjhalliday/ChakraCore that referenced this issue Feb 13, 2017
Remove -ES6Generators feature switch
f1f21f2 
Re issue chakra-core#6
ianwjhalliday pushed a commit to ianwjhalliday/ChakraCore that referenced this issue Feb 13, 2017
Remove -ES6Rest and -ES6Spread feature switches
7acf693 
Re issue chakra-core#6
@curtisman curtisman assigned tcare and unassigned ianwjhalliday May 18, 2017
@curtisman curtisman added this to the Backlog milestone May 18, 2017
@litian2025 litian2025 mentioned this issue Jun 29, 2017
Closed
@rajatd rajatd mentioned this issue Jul 26, 2017
Closed
@dilijev
Copy link
Contributor

dilijev commented Feb 24, 2018 •
edited
Loading

This suggestion is something we will gradually make progress on regardless of whether there is an open issue and will never be, in spirit, "done" because there will always be more feature flags. Therefore, I'm closing this issue. /cc @kfarnung @agarwal-sandeep

@dilijev dilijev closed this as completed Feb 24, 2018
@zhunki zhunki mentioned this issue Jul 26, 2018
Closed
@crdumoul crdumoul mentioned this issue Aug 7, 2018
Closed
boingoing added a commit to boingoing/ChakraCore that referenced this issue Feb 8, 2019
@boingoing
Class members which capture super via dot (super.constructor) end up …
b1cb079 
…getting the wrong value

We were using the wrong bytecode register when emitting LdHomeObjProto in the case of a lambda capturing super via a dot reference. Consider this Javascript:

```javascript
class A { }
class B extends A {
    foo() {
        const _s = () => super.constructor;
        console.log(_s().name);
    }
}
```
The member function foo correctly stashes this and super but the lambda needs to load them from the environment and it was loading them into the same register. Here's the incorrect bytecode:
```
Function _s ( (chakra-core#1.5), chakra-core#6) (In0) (size: 8 [8])
      5 locals (2 temps from R3), 1 inline cache
    Constant Table:
    ======== =====
     R1 LdRoot

  Line  13: super.constructor
  Col   26: ^
    0000   ProfiledLdEnvSlot    R3 = [1][2]  <0>
    0006   ProfiledLdEnvSlot    R3 = [1][3]  <1>
    000c   LdHomeObjProto       R4  R3
    0011   ProfiledLdSuperFld   R0 = R4(this=R3). #0 <0>
    0019   Br                   x:001e (   2)
    001c   LdUndef              R0
```
With the fix in this PR, here's the bytecode we get for _s:
```
Function _s ( (chakra-core#1.5), chakra-core#6) (In0) (size: 8 [8])
      6 locals (3 temps from R3), 1 inline cache
    Constant Table:
    ======== =====
     R1 LdRoot

  Line  56: super.constructor
  Col   26: ^
    0000   ProfiledLdEnvSlot    R3 = [1][2]  <0>
    0006   ProfiledLdEnvSlot    R4 = [1][3]  <1>
    000c   LdHomeObjProto       R5  R3
    0011   ProfiledLdSuperFld   R0 = R5(this=R4). #0 <0>
    0019   Br                   x:001e (   2)
    001c   LdUndef              R0
```
chakrabot pushed a commit that referenced this issue Feb 8, 2019
@boingoing
[MERGE #5914 @boingoing] Class members which capture super via dot (s…
87249a1 
…uper.constructor) end up getting the wrong value

Merge pull request #5914 from boingoing:super_capture_ts

Class members which capture super via dot (super.constructor) end up getting the wrong value

    We were using the wrong bytecode register when emitting LdHomeObjProto in the case of a lambda capturing super via a dot reference. Consider this Javascript:

    ```javascript
    class A { }
    class B extends A {
        foo() {
            const _s = () => super.constructor;
            console.log(_s().name);
        }
    }
    ```
    The member function foo correctly stashes this and super but the lambda needs to load them from the environment and it was loading them into the same register. Here's the incorrect bytecode:
    ```
    Function _s ( (#1.5), #6) (In0) (size: 8 [8])
          5 locals (2 temps from R3), 1 inline cache
        Constant Table:
        ======== =====
         R1 LdRoot

      Line  13: super.constructor
      Col   26: ^
        0000   ProfiledLdEnvSlot    R3 = [1][2]  <0>
        0006   ProfiledLdEnvSlot    R3 = [1][3]  <1>
        000c   LdHomeObjProto       R4  R3
        0011   ProfiledLdSuperFld   R0 = R4(this=R3). #0 <0>
        0019   Br                   x:001e (   2)
        001c   LdUndef              R0
    ```
    With the fix in this PR, here's the bytecode we get for _s:
    ```
    Function _s ( (#1.5), #6) (In0) (size: 8 [8])
          6 locals (3 temps from R3), 1 inline cache
        Constant Table:
        ======== =====
         R1 LdRoot

      Line  56: super.constructor
      Col   26: ^
        0000   ProfiledLdEnvSlot    R3 = [1][2]  <0>
        0006   ProfiledLdEnvSlot    R4 = [1][3]  <1>
        000c   LdHomeObjProto       R5  R3
        0011   ProfiledLdSuperFld   R0 = R5(this=R4). #0 <0>
        0019   Br                   x:001e (   2)
        001c   LdUndef              R0
    ```
@ghost ghost mentioned this issue Mar 21, 2019
Closed
@bird8693 bird8693 mentioned this issue Mar 17, 2021
Open
This was referenced Dec 14, 2021
Open
Closed
Open
Open
Open
Open
@wangyue6761 wangyue6761 mentioned this issue Feb 20, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@tcare tcare

Labels
Suggestion
Projects
None yet
Milestone
Backlog
Development

No branches or pull requests
5 participants
@dilijev @tcare @ianwjhalliday @curtisman @pleath