Optional chaining

[ShortDevelopment]

This PR aims to add support for the stage-4 proposal optional-chaining.
It's inspired by the work of @rhuanjl but uses a more hacky approach to parsing.

Goals

• Minimize amount of changes
• (Hopefully) Performance

ToDo

• Add tests
  • Unused expression result
  • Return expression
  • Root optional-call (e.g. eval?.('a'))
  • Scoped method load (e.g. inside eval)
  • delete
• implement optional-deletion
• optional call invoked on eval function should be indirect eval
• Simple function calls
• Preserve this
(a.b)().c should be equivalent to a.b().c
• short circuit embedded expressions like a?.[++x]
++x should not be evaluated if a is null or undefined
• Don't tokenize a?.3:0 (ternary) as tkOptChain (?.)
• Tagged templates are not allowed in optional chain
• fix tmp-renaming for eval result
  eval("foo?.()") or eval("foo?.property")
• Works with optimizations
  • What about the apply call optimization?
  • Loop inversion (AST cloning)
    Only triggered for 2 nested for loops with assignment
    

    ChakraCore/test/loop/loopinversion.js

    Lines 60 to 66 in e26c81f

    	for (var j = 0; j < 1; ++j) {
    	for (var i = 0; i < 1; ++i) {
    	1;
    	c = 2;
    	1;
    	}
    	};

Out of scope

• Remove EmitInvoke (seems unused)
• Better error message for a call on a non-function

---

Spec: Optional Chains
Fixes #6349

[rhuanjl]

I think this looks neater than my version was, though struggling to follow how it handles every branch. What happens if you do a function call in an optional chain does it crash?

(Not going to merge until we have some significant test coverage)

[ShortDevelopment]

I'm currently working on function calls.
At the moment it would just be handled as if no optional-chaining would be used
⇾ crash in js land

Edit: Function call should work now but this is not always propagated correctly (yet)
Edit: Function calls should now work

[ShortDevelopment]

The jitted code currently crashes at this assertion (causing the test-failures)

ChakraCore/lib/Backend/GlobOpt.cpp

Line 11384 in 1f6e17c

Assert(block->globOptData.IsTypeSpecialized(varSym));

[ShortDevelopment]

We "copy" the value from the knopOptChain node to the knopDelete node

[ShortDevelopment]

Instead of acquiring a tmp I copy the location of pnodeOptChain to innerNode
(In case the caller has already aquired a tmp)

ChakraCore/lib/Runtime/ByteCode/ByteCodeEmitter.cpp

Line 63 in 05790ae

innerNode->location = pnodeOptChain->location;

And copy it back after emitting innerNode
(In case the location was NoRegister and the emission of innerNode aquired a tmp)

ChakraCore/lib/Runtime/ByteCode/ByteCodeEmitter.cpp

Line 69 in 05790ae

pnodeOptChain->location = innerNode->location;

[ShortDevelopment]

@rhuanjl Just in time for the first aniversary (🎉) of this PR, I'll mark it as ready for review!

• All feature work should be complete
• I'm likely missing some test cases (Do you have ideas?)
• You had some special handling for apply calls but I did not manage to come up with a working test case
• The pipeline is failing again due to a now unsupported CMake version 😢
  [Build]: MacOS ci does not support CMake < 3.5 #7031

[sapdragon]

I see that there has been more activity in the repository. Is there any chance of merging this PR?

[rhuanjl]

I see that there has been more activity in the repository. Is there any chance of merging this PR?

I'll take a look in the next week, back when we were last working on this I was worried about a couple of potentially untested code paths.

FYI: the only people who contribute to this repository now do it as a hobby and have unrelated jobs hence the glacial progress. I'm happy to give a bit of time to help any new contributor learn the ropes if you're aware of interest it would be good to see more happening here.