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

Optional chaining #6973

Draft
wants to merge 33 commits into
base: master
Choose a base branch
from
Draft

Optional chaining #6973

Changes from 1 commit
Commits
Show all changes
33 commits
Select commit Hold shift + click to select a range
b2f5f1b
Basic parsing
ShortDevelopment Apr 13, 2024
8e9abda
Config flag
ShortDevelopment Apr 13, 2024
2f84eb1
Parse call
ShortDevelopment Apr 13, 2024
6538ef0
Basic byte-code emission
ShortDevelopment Apr 13, 2024
a33799e
Check for `null` not truthy
ShortDevelopment Apr 15, 2024
caccbc6
Fix copyright
ShortDevelopment Apr 15, 2024
2cde756
Use less branches
ShortDevelopment Apr 15, 2024
f833213
More copyright fixes
ShortDevelopment Apr 15, 2024
d2377a4
Don't break ternary with decimal numbers
ShortDevelopment Apr 15, 2024
6a2c0ec
Honor `buildAST`
ShortDevelopment Apr 15, 2024
e3820ec
Tagged template in optional chain is syntax error
ShortDevelopment Apr 15, 2024
cc54764
short-circuit indexer expressions
ShortDevelopment Apr 15, 2024
9e11b34
Simple method call
ShortDevelopment Apr 18, 2024
2498b2d
Comments and review
ShortDevelopment Apr 18, 2024
63093df
Merge branch 'master' into feat-optional-chaining
ShortDevelopment Apr 18, 2024
6e7d935
Fix optChain right before function call
ShortDevelopment Apr 20, 2024
9369845
Fix `this` propagation
ShortDevelopment Apr 21, 2024
ba32362
Basic tests
ShortDevelopment Apr 24, 2024
c8297ec
Fix jit `_ReuseLoc`
ShortDevelopment May 4, 2024
8baa9e6
Don't use `LdMethodFld`
ShortDevelopment May 4, 2024
8c4eddf
Add call tests + Split into multiple files
ShortDevelopment May 7, 2024
f48dc1b
Treat `eval?.()` as indirect `eval`
ShortDevelopment May 10, 2024
05790ae
Started optional-deletion
ShortDevelopment May 28, 2024
3b5d8d5
Merge branch 'master' into feat-optional-chaining
ShortDevelopment Jul 4, 2024
a18431e
Test optional-call in eval
ShortDevelopment Jul 6, 2024
d96b76b
Ensure `isUsed` is set if `MustProduceValue`
ShortDevelopment Jul 6, 2024
0abf9c2
Copy `isUsed` for robustness
ShortDevelopment Jul 6, 2024
a2709e7
Test for opt-call of root function
ShortDevelopment Jul 6, 2024
8210f62
Async tests
ShortDevelopment Jul 6, 2024
629cb8a
Add test for indirect eval
ShortDevelopment Oct 13, 2024
55dea9f
Add tests for jit
ShortDevelopment Oct 13, 2024
2aaa1c9
Test opt-chain arguments
ShortDevelopment Oct 13, 2024
835a4fa
Copy `isNullPropagating` in `CopyPnode`
ShortDevelopment Oct 13, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
137 changes: 79 additions & 58 deletions lib/Runtime/ByteCode/ByteCodeEmitter.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,12 +60,13 @@ static void EmitOptionalChainWrapper(ParseNodeUni *pnodeOptChain, ByteCodeGenera
// Copy values from wrapper to inner expression
ParseNodePtr innerNode = pnodeOptChain->pnode1;
innerNode->isUsed = pnodeOptChain->isUsed;
innerNode->location = funcInfo->AcquireLoc(pnodeOptChain);
innerNode->location = pnodeOptChain->location;
ShortDevelopment marked this conversation as resolved.
Show resolved Hide resolved

// emit chain expression
// Every `?.` node will call `EmitNullPropagation`
// `EmitNullPropagation` short-circuits to `skipLabel` in case of a nullish value
emitChainContent(innerNode);
pnodeOptChain->location = innerNode->location;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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;


Js::ByteCodeLabel doneLabel = Js::Constants::NoRegister;
if (pnodeOptChain->isUsed)
Expand All @@ -89,6 +90,7 @@ static void EmitOptionalChainWrapper(ParseNodeUni *pnodeOptChain, ByteCodeGenera
byteCodeGenerator->Writer()->Reg2(Js::OpCode::Ld_A_ReuseLoc, pnodeOptChain->location, funcInfo->undefinedConstantRegister);
byteCodeGenerator->Writer()->MarkLabel(doneLabel);
}

funcInfo->currentOptionalChainSkipLabel = previousSkipLabel;
}

Expand Down Expand Up @@ -11209,6 +11211,81 @@ void TrackGlobalIntAssignments(ParseNodePtr pnode, ByteCodeGenerator * byteCodeG
}
}

static void EmitDelete(ParseNode *pnode, ParseNode *pexpr, ByteCodeGenerator *byteCodeGenerator, FuncInfo *funcInfo) {
switch (pexpr->nop)
{
case knopOptChain:
EmitOptionalChainWrapper(pexpr->AsParseNodeUni(), byteCodeGenerator, funcInfo, [&](ParseNode *innerNode) {
EmitDelete(innerNode, innerNode, byteCodeGenerator, funcInfo);
});
pnode->location = pexpr->location;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

break;
case knopName:
{
ParseNodeName *pnodeName = pexpr->AsParseNodeName();
if (pnodeName->IsSpecialName())
{
funcInfo->AcquireLoc(pnode);
byteCodeGenerator->Writer()->Reg1(Js::OpCode::LdTrue, pnode->location);
}
else
{
funcInfo->AcquireLoc(pnode);
byteCodeGenerator->EmitPropDelete(pnode->location, pnodeName->sym, pnodeName->pid, funcInfo);
}
break;
}
case knopDot:
{
ParseNodeBin *pnodeDot = pexpr->AsParseNodeBin();
ParseNode *pnode1 = pnodeDot->pnode1;
ParseNode *pnode2 = pnodeDot->pnode2;

if (ByteCodeGenerator::IsSuper(pnode1))
{
byteCodeGenerator->Writer()->W1(Js::OpCode::RuntimeReferenceError, SCODE_CODE(JSERR_DeletePropertyWithSuper));

funcInfo->AcquireLoc(pnode);
byteCodeGenerator->Writer()->Reg1(Js::OpCode::LdUndef, pnode->location);
}
else
{
Emit(pnode1, byteCodeGenerator, funcInfo, false);
EmitNullPropagation(pnode1->location, byteCodeGenerator, funcInfo, pnodeDot->isNullPropagating);

funcInfo->ReleaseLoc(pnode1);
Js::PropertyId propertyId = pnode2->AsParseNodeName()->PropertyIdFromNameNode();
funcInfo->AcquireLoc(pnode);
byteCodeGenerator->Writer()->Property(Js::OpCode::DeleteFld, pnode->location, pnode1->location,
funcInfo->FindOrAddReferencedPropertyId(propertyId), byteCodeGenerator->forceStrictModeForClassComputedPropertyName);
}

break;
}
case knopIndex:
{
ParseNodeBin *pnodeIndex = pexpr->AsParseNodeBin();
ParseNode *pnode1 = pnodeIndex->pnode1;
ParseNode *pnode2 = pnodeIndex->pnode2;

EmitBinaryOpnds(pnode1, pnode2, byteCodeGenerator, funcInfo, Js::Constants::NoRegister, pnodeIndex->isNullPropagating);
funcInfo->ReleaseLoc(pnode2);
funcInfo->ReleaseLoc(pnode1);
funcInfo->AcquireLoc(pnode);
byteCodeGenerator->Writer()->Element(Js::OpCode::DeleteElemI_A, pnode->location, pnode1->location, pnode2->location);
break;
}
default:
{
Emit(pexpr, byteCodeGenerator, funcInfo, false);
funcInfo->ReleaseLoc(pexpr);
byteCodeGenerator->Writer()->Reg2(
Js::OpCode::Delete_A, funcInfo->AcquireLoc(pnode), pexpr->location);
break;
}
Comment on lines +11282 to +11289
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

JavascriptOperators::Delete does only return true. Why can't we just emit bytecode to load true?
Is this because of getter/setter side-effects?

ChakraCore/lib/Runtime/Language/JavascriptOperators.cpp

Lines 534 to 539 in 05790ae

Var JavascriptOperators::Delete(Var var, ScriptContext* scriptContext)
{
JIT_HELPER_NOT_REENTRANT_NOLOCK_HEADER(Op_Delete);
return scriptContext->GetLibrary()->GetTrue();
JIT_HELPER_END(Op_Delete);
}

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for the delay, I was confused when I first looked at this; it appears this OpCode is used only in this one location in the bytecodeemitter and is for a no_op delete, as far as I can see we could indeed replace it with LdTrue and delete the logic for OpCode:Delete_A and the relevant code paths.

}
}

void Emit(ParseNode* pnode, ByteCodeGenerator* byteCodeGenerator, FuncInfo* funcInfo, BOOL fReturnValue, bool isConstructorCall, bool isTopLevel)
{
if (pnode == nullptr)
Expand Down Expand Up @@ -11570,63 +11647,7 @@ void Emit(ParseNode* pnode, ByteCodeGenerator* byteCodeGenerator, FuncInfo* func
{
ParseNode *pexpr = pnode->AsParseNodeUni()->pnode1;
byteCodeGenerator->StartStatement(pnode);
switch (pexpr->nop)
{
case knopName:
{
ParseNodeName * pnodeName = pexpr->AsParseNodeName();
if (pnodeName->IsSpecialName())
{
funcInfo->AcquireLoc(pnode);
byteCodeGenerator->Writer()->Reg1(Js::OpCode::LdTrue, pnode->location);
}
else
{
funcInfo->AcquireLoc(pnode);
byteCodeGenerator->EmitPropDelete(pnode->location, pnodeName->sym, pnodeName->pid, funcInfo);
}
break;
}
case knopDot:
{
if (ByteCodeGenerator::IsSuper(pexpr->AsParseNodeBin()->pnode1))
{
byteCodeGenerator->Writer()->W1(Js::OpCode::RuntimeReferenceError, SCODE_CODE(JSERR_DeletePropertyWithSuper));

funcInfo->AcquireLoc(pnode);
byteCodeGenerator->Writer()->Reg1(Js::OpCode::LdUndef, pnode->location);
}
else
{
Emit(pexpr->AsParseNodeBin()->pnode1, byteCodeGenerator, funcInfo, false);

funcInfo->ReleaseLoc(pexpr->AsParseNodeBin()->pnode1);
Js::PropertyId propertyId = pexpr->AsParseNodeBin()->pnode2->AsParseNodeName()->PropertyIdFromNameNode();
funcInfo->AcquireLoc(pnode);
byteCodeGenerator->Writer()->Property(Js::OpCode::DeleteFld, pnode->location, pexpr->AsParseNodeBin()->pnode1->location,
funcInfo->FindOrAddReferencedPropertyId(propertyId), byteCodeGenerator->forceStrictModeForClassComputedPropertyName);
}

break;
}
case knopIndex:
{
EmitBinaryOpnds(pexpr->AsParseNodeBin()->pnode1, pexpr->AsParseNodeBin()->pnode2, byteCodeGenerator, funcInfo);
funcInfo->ReleaseLoc(pexpr->AsParseNodeBin()->pnode2);
funcInfo->ReleaseLoc(pexpr->AsParseNodeBin()->pnode1);
funcInfo->AcquireLoc(pnode);
byteCodeGenerator->Writer()->Element(Js::OpCode::DeleteElemI_A, pnode->location, pexpr->AsParseNodeBin()->pnode1->location, pexpr->AsParseNodeBin()->pnode2->location);
break;
}
default:
{
Emit(pexpr, byteCodeGenerator, funcInfo, false);
funcInfo->ReleaseLoc(pexpr);
byteCodeGenerator->Writer()->Reg2(
Js::OpCode::Delete_A, funcInfo->AcquireLoc(pnode), pexpr->location);
break;
}
}
EmitDelete(pnode, pexpr, byteCodeGenerator, funcInfo);
byteCodeGenerator->EndStatement(pnode);
break;
}
Expand Down
Loading