From b2f5f1b96a4db9576badd85a735640190253a2ef Mon Sep 17 00:00:00 2001 From: Lukas Kurz Date: Sat, 13 Apr 2024 11:23:01 +0200 Subject: [PATCH 01/27] Basic parsing --- lib/Parser/Parse.cpp | 24 +++++++++++++++--------- lib/Parser/Parse.h | 6 +++--- lib/Parser/Scan.cpp | 7 +++++++ lib/Parser/kwd-lsc.h | 2 ++ lib/Parser/ptlist.h | 2 ++ lib/Parser/ptree.cpp | 3 ++- lib/Parser/ptree.h | 3 ++- 7 files changed, 33 insertions(+), 14 deletions(-) diff --git a/lib/Parser/Parse.cpp b/lib/Parser/Parse.cpp index d5adfb40cf2..c4b1629accd 100644 --- a/lib/Parser/Parse.cpp +++ b/lib/Parser/Parse.cpp @@ -309,6 +309,7 @@ LPCWSTR Parser::GetTokenString(tokens token) case tkLParen: return _u("("); case tkLBrack: return _u("["); case tkDot: return _u("."); + case tkOptChain: return _u("?."); default: return _u("unknown token"); @@ -894,13 +895,13 @@ ParseNodeUni * Parser::CreateUniNode(OpCode nop, ParseNodePtr pnode1, charcount_ } // Create ParseNodeBin -ParseNodeBin * Parser::StaticCreateBinNode(OpCode nop, ParseNodePtr pnode1, ParseNodePtr pnode2, ArenaAllocator* alloc, charcount_t ichMin, charcount_t ichLim) +ParseNodeBin * Parser::StaticCreateBinNode(OpCode nop, ParseNodePtr pnode1, ParseNodePtr pnode2, ArenaAllocator* alloc, charcount_t ichMin, charcount_t ichLim, bool isNullPropagating) { DebugOnly(VerifyNodeSize(nop, sizeof(ParseNodeBin))); - return Anew(alloc, ParseNodeBin, nop, ichMin, ichLim, pnode1, pnode2); + return Anew(alloc, ParseNodeBin, nop, ichMin, ichLim, pnode1, pnode2, isNullPropagating); } -ParseNodeBin * Parser::CreateBinNode(OpCode nop, ParseNodePtr pnode1, ParseNodePtr pnode2) +ParseNodeBin * Parser::CreateBinNode(OpCode nop, ParseNodePtr pnode1, ParseNodePtr pnode2, bool isNullPropagating) { Assert(!this->m_deferringAST); charcount_t ichMin; @@ -937,15 +938,15 @@ ParseNodeBin * Parser::CreateBinNode(OpCode nop, ParseNodePtr pnode1, ParseNodeP } } - return CreateBinNode(nop, pnode1, pnode2, ichMin, ichLim); + return CreateBinNode(nop, pnode1, pnode2, ichMin, ichLim, isNullPropagating); } ParseNodeBin * Parser::CreateBinNode(OpCode nop, ParseNodePtr pnode1, - ParseNodePtr pnode2, charcount_t ichMin, charcount_t ichLim) + ParseNodePtr pnode2, charcount_t ichMin, charcount_t ichLim, bool isNullPropagating) { Assert(!this->m_deferringAST); - ParseNodeBin * pnode = StaticCreateBinNode(nop, pnode1, pnode2, &m_nodeAllocator, ichMin, ichLim); + ParseNodeBin * pnode = StaticCreateBinNode(nop, pnode1, pnode2, &m_nodeAllocator, ichMin, ichLim, isNullPropagating); AddAstSize(sizeof(ParseNodeBin)); return pnode; } @@ -4163,12 +4164,17 @@ ParseNodePtr Parser::ParsePostfixOperators( } } break; - + + case tkOptChain: case tkDot: { ParseNodePtr name = nullptr; OpCode opCode = knopDot; + bool isNullPropagating = tkOptChain == m_token.tk; + // We don't use a custom token but rather tell that knopDot is null-propagating + // opCode = knopOptChain; + this->GetScanner()->Scan(); if (!m_token.IsIdentifier()) { @@ -4189,7 +4195,7 @@ ParseNodePtr Parser::ParsePostfixOperators( if (buildAST) { - if (opCode == knopDot) + if (opCode == knopDot || opCode == knopOptChain) { name = CreateNameNode(m_token.GetIdentifier(this->GetHashTbl())); } @@ -4205,7 +4211,7 @@ ParseNodePtr Parser::ParsePostfixOperators( } else { - pnode = CreateBinNode(opCode, pnode, name); + pnode = CreateBinNode(opCode, pnode, name, isNullPropagating); } } else diff --git a/lib/Parser/Parse.h b/lib/Parser/Parse.h index bcbc9413c13..f72b0f9376a 100644 --- a/lib/Parser/Parse.h +++ b/lib/Parser/Parse.h @@ -371,7 +371,7 @@ class Parser return Anew(alloc, typename OpCodeTrait::ParseNodeType, nop, ichMin, ichLim); } - static ParseNodeBin * StaticCreateBinNode(OpCode nop, ParseNodePtr pnode1, ParseNodePtr pnode2, ArenaAllocator* alloc, charcount_t ichMin = 0, charcount_t ichLim = 0); + static ParseNodeBin * StaticCreateBinNode(OpCode nop, ParseNodePtr pnode1, ParseNodePtr pnode2, ArenaAllocator* alloc, charcount_t ichMin = 0, charcount_t ichLim = 0, bool isNullPropagating = false); static ParseNodeBlock * StaticCreateBlockNode(ArenaAllocator* alloc, charcount_t ichMin = 0, charcount_t ichLim = 0, int blockId = -1, PnodeBlockType blockType = PnodeBlockType::Regular); static ParseNodeVar * StaticCreateTempNode(ParseNode* initExpr, ArenaAllocator* alloc); static ParseNodeUni * StaticCreateTempRef(ParseNode* tempNode, ArenaAllocator* alloc); @@ -379,8 +379,8 @@ class Parser private: ParseNodeUni * CreateUniNode(OpCode nop, ParseNodePtr pnodeOp); ParseNodeUni * CreateUniNode(OpCode nop, ParseNodePtr pnode1, charcount_t ichMin, charcount_t ichLim); - ParseNodeBin * CreateBinNode(OpCode nop, ParseNodePtr pnode1, ParseNodePtr pnode2); - ParseNodeBin * CreateBinNode(OpCode nop, ParseNodePtr pnode1, ParseNodePtr pnode2, charcount_t ichMin, charcount_t ichLim); + ParseNodeBin * CreateBinNode(OpCode nop, ParseNodePtr pnode1, ParseNodePtr pnode2, bool isNullPropagating = false); + ParseNodeBin * CreateBinNode(OpCode nop, ParseNodePtr pnode1, ParseNodePtr pnode2, charcount_t ichMin, charcount_t ichLim, bool isNullPropagating = false); ParseNodeTri * CreateTriNode(OpCode nop, ParseNodePtr pnode1, ParseNodePtr pnode2, ParseNodePtr pnode3); ParseNodeTri * CreateTriNode(OpCode nop, ParseNodePtr pnode1, ParseNodePtr pnode2, ParseNodePtr pnode3, charcount_t ichMin, charcount_t ichLim); ParseNodeBlock * CreateBlockNode(PnodeBlockType blockType = PnodeBlockType::Regular); diff --git a/lib/Parser/Scan.cpp b/lib/Parser/Scan.cpp index f1b9efa71c3..22abfe46c9c 100644 --- a/lib/Parser/Scan.cpp +++ b/lib/Parser/Scan.cpp @@ -1,5 +1,6 @@ //------------------------------------------------------------------------------------------------------- // Copyright (C) Microsoft. All rights reserved. +// Copyright (c) ChakraCore Project Contributors. All rights reserved. // Licensed under the MIT license. See LICENSE.txt file in the project root for full license information. //------------------------------------------------------------------------------------------------------- #include "ParserPch.h" @@ -1780,6 +1781,12 @@ tokens Scanner::ScanCore(bool identifyKwds) token = tkCoalesce; break; } + else if (this->PeekFirst(p, last) == '.') // ToDo: Config flag for optional chaining?! + { + p++; + token = tkOptChain; + break; + } break; case '{': Assert(chType == _C_LC); token = tkLCurly; break; diff --git a/lib/Parser/kwd-lsc.h b/lib/Parser/kwd-lsc.h index dba31b1c5d9..0b913a6d38c 100644 --- a/lib/Parser/kwd-lsc.h +++ b/lib/Parser/kwd-lsc.h @@ -1,5 +1,6 @@ //------------------------------------------------------------------------------------------------------- // Copyright (C) Microsoft. All rights reserved. +// Copyright (c) ChakraCore Project Contributors. All rights reserved. // Licensed under the MIT license. See LICENSE.txt file in the project root for full license information. //------------------------------------------------------------------------------------------------------- #ifndef KEYWORD @@ -174,6 +175,7 @@ TOK_DCL(tkEllipsis , No, knopNone ,Spr, knopEllipsis ) // ... TOK_DCL(tkLParen , No, knopNone , No, knopNone ) // ( TOK_DCL(tkLBrack , No, knopNone , No, knopNone ) // [ TOK_DCL(tkDot , No, knopNone , No, knopNone ) // . +TOK_DCL(tkOptChain , No, knopNone , No, knopNone ) // ?. // String template tokens TOK_DCL(tkStrTmplBasic , No, knopNone , No, knopNone ) // `...` diff --git a/lib/Parser/ptlist.h b/lib/Parser/ptlist.h index a340d73f5bd..cb9f0a1fe0e 100644 --- a/lib/Parser/ptlist.h +++ b/lib/Parser/ptlist.h @@ -1,5 +1,6 @@ //------------------------------------------------------------------------------------------------------- // Copyright (C) Microsoft. All rights reserved. +// Copyright (c) ChakraCore Project Contributors. All rights reserved. // Licensed under the MIT license. See LICENSE.txt file in the project root for full license information. //------------------------------------------------------------------------------------------------------- /*****************************************************************************/ @@ -80,6 +81,7 @@ PTNODE(knopGe , ">=" , OP(Ge) , Bin , fnopBin|fn PTNODE(knopGt , ">" , OP(Gt) , Bin , fnopBin|fnopRel , "GreaterThanOper" ) PTNODE(knopCall , "()" , Nop , Call , fnopNone , "CallExpr" ) PTNODE(knopDot , "." , Nop , Bin , fnopBin , "DotOper" ) +PTNODE(knopOptChain , "?." , Nop , Bin , fnopBin , "OptChain" ) PTNODE(knopAsg , "=" , Nop , Bin , fnopBin|fnopAsg , "AssignmentOper" ) PTNODE(knopInstOf , "instanceof" , IsInst , Bin , fnopBin|fnopRel , "InstanceOfExpr" ) PTNODE(knopIn , "in" , IsIn , Bin , fnopBin|fnopRel , "InOper" ) diff --git a/lib/Parser/ptree.cpp b/lib/Parser/ptree.cpp index faa254f6564..1a85418ec40 100644 --- a/lib/Parser/ptree.cpp +++ b/lib/Parser/ptree.cpp @@ -301,7 +301,7 @@ ParseNodeUni::ParseNodeUni(OpCode nop, charcount_t ichMin, charcount_t ichLim, P this->pnode1 = pnode1; } -ParseNodeBin::ParseNodeBin(OpCode nop, charcount_t ichMin, charcount_t ichLim, ParseNode * pnode1, ParseNode * pnode2) +ParseNodeBin::ParseNodeBin(OpCode nop, charcount_t ichMin, charcount_t ichLim, ParseNode * pnode1, ParseNode * pnode2, bool isNullPropagating) : ParseNode(nop, ichMin, ichLim) { // Member name is either a string or a computed name @@ -313,6 +313,7 @@ ParseNodeBin::ParseNodeBin(OpCode nop, charcount_t ichMin, charcount_t ichLim, P this->pnode1 = pnode1; this->pnode2 = pnode2; + this->isNullPropagating = isNullPropagating; // Statically detect if the add is a concat if (!PHASE_OFF1(Js::ByteCodeConcatExprOptPhase)) diff --git a/lib/Parser/ptree.h b/lib/Parser/ptree.h index 82b56fef30c..a78ba960511 100644 --- a/lib/Parser/ptree.h +++ b/lib/Parser/ptree.h @@ -277,10 +277,11 @@ class ParseNodeUni : public ParseNode class ParseNodeBin : public ParseNode { public: - ParseNodeBin(OpCode nop, charcount_t ichMin, charcount_t ichLim, ParseNode * pnode1, ParseNode * pnode2); + ParseNodeBin(OpCode nop, charcount_t ichMin, charcount_t ichLim, ParseNode * pnode1, ParseNode * pnode2, bool isNullPropagating = false); ParseNodePtr pnode1; ParseNodePtr pnode2; + bool isNullPropagating; DISABLE_SELF_CAST(ParseNodeBin); }; From 8e9abda9a5bdddcacb065f86185914c710d9ee3f Mon Sep 17 00:00:00 2001 From: Lukas Kurz Date: Sat, 13 Apr 2024 13:21:30 +0200 Subject: [PATCH 02/27] Config flag --- lib/Common/ConfigFlagsList.h | 4 ++++ lib/Parser/Scan.cpp | 2 +- lib/Runtime/Base/ThreadConfigFlagsList.h | 1 + 3 files changed, 6 insertions(+), 1 deletion(-) diff --git a/lib/Common/ConfigFlagsList.h b/lib/Common/ConfigFlagsList.h index 406b24eb1dc..0566dae74f3 100644 --- a/lib/Common/ConfigFlagsList.h +++ b/lib/Common/ConfigFlagsList.h @@ -673,6 +673,7 @@ PHASE(All) #define DEFAULT_CONFIG_ESArrayFindFromLast (true) #define DEFAULT_CONFIG_ESPromiseAny (true) #define DEFAULT_CONFIG_ESNullishCoalescingOperator (true) +#define DEFAULT_CONFIG_ESOptionalChaining (true) #define DEFAULT_CONFIG_ESGlobalThis (true) // Jitting generators has not been tested on ARM @@ -1199,6 +1200,9 @@ FLAGR(Boolean, ESNumericSeparator, "Enable Numeric Separator flag", DEFAULT_CONF // ES Nullish coalescing operator support (??) FLAGR(Boolean, ESNullishCoalescingOperator, "Enable nullish coalescing operator", DEFAULT_CONFIG_ESNullishCoalescingOperator) +// ES Optional chaining operator support (?.) +FLAGR(Boolean, ESOptionalChaining, "Enable optional chaining operator", DEFAULT_CONFIG_ESOptionalChaining) + // ES Hashbang support for interpreter directive syntax FLAGR(Boolean, ESHashbang, "Enable Hashbang syntax", DEFAULT_CONFIG_ESHashbang) diff --git a/lib/Parser/Scan.cpp b/lib/Parser/Scan.cpp index 22abfe46c9c..72603dbe4b1 100644 --- a/lib/Parser/Scan.cpp +++ b/lib/Parser/Scan.cpp @@ -1781,7 +1781,7 @@ tokens Scanner::ScanCore(bool identifyKwds) token = tkCoalesce; break; } - else if (this->PeekFirst(p, last) == '.') // ToDo: Config flag for optional chaining?! + else if (m_scriptContext->GetConfig()->IsESOptionalChainingEnabled() && this->PeekFirst(p, last) == '.') { p++; token = tkOptChain; diff --git a/lib/Runtime/Base/ThreadConfigFlagsList.h b/lib/Runtime/Base/ThreadConfigFlagsList.h index 3026920fc9f..dee273547ce 100644 --- a/lib/Runtime/Base/ThreadConfigFlagsList.h +++ b/lib/Runtime/Base/ThreadConfigFlagsList.h @@ -45,6 +45,7 @@ FLAG_RELEASE(IsESBigIntEnabled, ESBigInt) FLAG_RELEASE(IsESNumericSeparatorEnabled, ESNumericSeparator) FLAG_RELEASE(IsESHashbangEnabled, ESHashbang) FLAG_RELEASE(IsESNullishCoalescingOperatorEnabled, ESNullishCoalescingOperator) +FLAG_RELEASE(IsESOptionalChainingEnabled, ESOptionalChaining) FLAG_RELEASE(IsESExportNsAsEnabled, ESExportNsAs) FLAG_RELEASE(IsESSymbolDescriptionEnabled, ESSymbolDescription) FLAG_RELEASE(IsESPromiseAnyEnabled, ESPromiseAny) From 2f84eb1ac2ecc5e0827351c8d7717f6eddd4e955 Mon Sep 17 00:00:00 2001 From: Lukas Kurz Date: Sat, 13 Apr 2024 13:28:51 +0200 Subject: [PATCH 03/27] Parse call --- lib/Parser/Parse.cpp | 43 +++++++++++++++++++++++++++++++++++++------ lib/Parser/perrors.h | 5 ++++- lib/Parser/ptree.cpp | 2 ++ lib/Parser/ptree.h | 2 ++ 4 files changed, 45 insertions(+), 7 deletions(-) diff --git a/lib/Parser/Parse.cpp b/lib/Parser/Parse.cpp index c4b1629accd..5b807f11013 100644 --- a/lib/Parser/Parse.cpp +++ b/lib/Parser/Parse.cpp @@ -3919,8 +3919,14 @@ ParseNodePtr Parser::ParsePostfixOperators( { AutoMarkInParsingArgs autoMarkInParsingArgs(this); + bool isNullPropagating = tkOptChain == this->GetScanner()->m_tkPrevious; if (fInNew) { + if (isNullPropagating) + { + Error(ERRInvalidOptChainInNew); + } + ParseNodePtr pnodeArgs = ParseArgList(&callOfConstants, &spreadArgCount, &count); if (buildAST) { @@ -3973,6 +3979,11 @@ ParseNodePtr Parser::ParsePostfixOperators( // Detect super() if (this->NodeIsSuperName(pnode)) { + if (isNullPropagating) + { + Error(ERRInvalidOptChainInSuper); + } + pnode = CreateSuperCallNode(pnode->AsParseNodeSpecialName(), pnodeArgs); Assert(pnode); @@ -4007,6 +4018,7 @@ ParseNodePtr Parser::ParsePostfixOperators( pnode->AsParseNodeCall()->isApplyCall = false; pnode->AsParseNodeCall()->isEvalCall = fCallIsEval; pnode->AsParseNodeCall()->hasDestructuring = m_hasDestructuringPattern; + pnode->AsParseNodeCall()->isNullPropagating = isNullPropagating; Assert(!m_hasDestructuringPattern || count > 0); pnode->AsParseNodeCall()->argCount = count; pnode->ichLim = this->GetScanner()->IchLimTok(); @@ -4042,7 +4054,7 @@ ParseNodePtr Parser::ParsePostfixOperators( } if (pfCanAssign) { - *pfCanAssign = fCanAssignToCallResult && + *pfCanAssign = !isNullPropagating && fCanAssignToCallResult && (m_sourceContextInfo ? !PHASE_ON_RAW(Js::EarlyErrorOnAssignToCallPhase, m_sourceContextInfo->sourceContextId, GetCurrentFunctionNode()->functionId) : !PHASE_ON1(Js::EarlyErrorOnAssignToCallPhase)); @@ -4055,6 +4067,8 @@ ParseNodePtr Parser::ParsePostfixOperators( } case tkLBrack: { + bool isNullPropagating = tkOptChain == this->GetScanner()->m_tkPrevious; + this->GetScanner()->Scan(); IdentToken tok; ParseNodePtr pnodeExpr = ParseExpr(0, FALSE, TRUE, FALSE, nullptr, nullptr, nullptr, &tok); @@ -4063,12 +4077,17 @@ ParseNodePtr Parser::ParsePostfixOperators( AnalysisAssert(pnodeExpr); if (pnode && pnode->nop == knopName && pnode->AsParseNodeName()->IsSpecialName() && pnode->AsParseNodeSpecialName()->isSuper) { + if (isNullPropagating) + { + Error(ERRInvalidOptChainInSuper); + } + pnode = CreateSuperReferenceNode(knopIndex, pnode->AsParseNodeSpecialName(), pnodeExpr); pnode->AsParseNodeSuperReference()->pnodeThis = ReferenceSpecialName(wellKnownPropertyPids._this, pnode->ichMin, pnode->ichLim, true); } else { - pnode = CreateBinNode(knopIndex, pnode, pnodeExpr); + pnode = CreateBinNode(knopIndex, pnode, pnodeExpr, isNullPropagating); } AnalysisAssert(pnode); @@ -4082,7 +4101,8 @@ ParseNodePtr Parser::ParsePostfixOperators( ChkCurTok(tkRBrack, ERRnoRbrack); if (pfCanAssign) { - *pfCanAssign = TRUE; + // optional assignment not permitted + *pfCanAssign = !isNullPropagating; } if (pfIsDotOrIndex) { @@ -4178,8 +4198,13 @@ ParseNodePtr Parser::ParsePostfixOperators( this->GetScanner()->Scan(); if (!m_token.IsIdentifier()) { - //allow reserved words in ES5 mode - if (!(m_token.IsReservedWord())) + if (isNullPropagating && (tkLParen == m_token.tk || tkLBrack == m_token.tk)) + { + // Continue to parse function or index (loop) + // Check previous token to check for null-propagation + continue; + } + else if (!(m_token.IsReservedWord())) //allow reserved words in ES5 mode { IdentifierExpectedError(m_token); } @@ -4206,6 +4231,11 @@ ParseNodePtr Parser::ParsePostfixOperators( } if (pnode && pnode->nop == knopName && pnode->AsParseNodeName()->IsSpecialName() && pnode->AsParseNodeSpecialName()->isSuper) { + if (isNullPropagating) + { + Error(ERRInvalidOptChainInSuper); + } + pnode = CreateSuperReferenceNode(opCode, pnode->AsParseNodeSpecialName(), name); pnode->AsParseNodeSuperReference()->pnodeThis = ReferenceSpecialName(wellKnownPropertyPids._this, pnode->ichMin, pnode->ichLim, true); } @@ -4222,7 +4252,8 @@ ParseNodePtr Parser::ParsePostfixOperators( if (pfCanAssign) { - *pfCanAssign = TRUE; + // optional assignment not permitted + *pfCanAssign = !isNullPropagating; } if (pfIsDotOrIndex) { diff --git a/lib/Parser/perrors.h b/lib/Parser/perrors.h index c4b93869ac3..4155fa81695 100644 --- a/lib/Parser/perrors.h +++ b/lib/Parser/perrors.h @@ -120,7 +120,10 @@ LSC_ERROR_MSG(1102, ERRInvalidAsgTarget, "Invalid left-hand side in assignment." LSC_ERROR_MSG(1103, ERRMissingFrom, "Expected 'from' after import or export clause.") // 1104 ERRsyntaxEOF -// 1105-1199 available for future use + +LSC_ERROR_MSG(1105, ERRInvalidOptChainInNew, "Invalid optional chain in new expression.") +LSC_ERROR_MSG(1106, ERRInvalidOptChainInSuper, "Invalid optional chain in call to 'super'.") +// 1107-1199 available for future use // Generic errors intended to be re-usable LSC_ERROR_MSG(1200, ERRKeywordAfter, "Unexpected keyword '%s' after '%s'") diff --git a/lib/Parser/ptree.cpp b/lib/Parser/ptree.cpp index 1a85418ec40..f9819f21478 100644 --- a/lib/Parser/ptree.cpp +++ b/lib/Parser/ptree.cpp @@ -1,5 +1,6 @@ //------------------------------------------------------------------------------------------------------- // Copyright (C) Microsoft. All rights reserved. +// Copyright (c) ChakraCore Project Contributors. All rights reserved. // Licensed under the MIT license. See LICENSE.txt file in the project root for full license information. //------------------------------------------------------------------------------------------------------- #include "ParserPch.h" @@ -496,6 +497,7 @@ ParseNodeCall::ParseNodeCall(OpCode nop, charcount_t ichMin, charcount_t ichLim, this->isEvalCall = false; this->isSuperCall = false; this->hasDestructuring = false; + this->isNullPropagating = false; } ParseNodeStmt::ParseNodeStmt(OpCode nop, charcount_t ichMin, charcount_t ichLim) diff --git a/lib/Parser/ptree.h b/lib/Parser/ptree.h index a78ba960511..5ad2be66c74 100644 --- a/lib/Parser/ptree.h +++ b/lib/Parser/ptree.h @@ -1,5 +1,6 @@ //------------------------------------------------------------------------------------------------------- // Copyright (C) Microsoft. All rights reserved. +// Copyright (c) ChakraCore Project Contributors. All rights reserved. // Licensed under the MIT license. See LICENSE.txt file in the project root for full license information. //------------------------------------------------------------------------------------------------------- #pragma once @@ -792,6 +793,7 @@ class ParseNodeCall : public ParseNode BYTE isEvalCall : 1; BYTE isSuperCall : 1; BYTE hasDestructuring : 1; + bool isNullPropagating; DISABLE_SELF_CAST(ParseNodeCall); }; From 6538ef091a653f04252957288600113e8fd2da3f Mon Sep 17 00:00:00 2001 From: Lukas Kurz Date: Sun, 14 Apr 2024 00:20:15 +0200 Subject: [PATCH 04/27] Basic byte-code emission --- lib/Parser/Parse.cpp | 18 +++++++--- lib/Parser/ptlist.h | 2 +- lib/Runtime/ByteCode/ByteCodeEmitter.cpp | 44 ++++++++++++++++++++++++ lib/Runtime/ByteCode/FuncInfo.cpp | 1 + lib/Runtime/ByteCode/FuncInfo.h | 9 +++++ 5 files changed, 69 insertions(+), 5 deletions(-) diff --git a/lib/Parser/Parse.cpp b/lib/Parser/Parse.cpp index 5b807f11013..39e4cec82b0 100644 --- a/lib/Parser/Parse.cpp +++ b/lib/Parser/Parse.cpp @@ -3910,6 +3910,7 @@ ParseNodePtr Parser::ParsePostfixOperators( *pfIsDotOrIndex = false; } + bool isOptionalChain = false; for (;;) { uint16 spreadArgCount = 0; @@ -4054,7 +4055,7 @@ ParseNodePtr Parser::ParsePostfixOperators( } if (pfCanAssign) { - *pfCanAssign = !isNullPropagating && fCanAssignToCallResult && + *pfCanAssign = !isOptionalChain && fCanAssignToCallResult && (m_sourceContextInfo ? !PHASE_ON_RAW(Js::EarlyErrorOnAssignToCallPhase, m_sourceContextInfo->sourceContextId, GetCurrentFunctionNode()->functionId) : !PHASE_ON1(Js::EarlyErrorOnAssignToCallPhase)); @@ -4102,7 +4103,7 @@ ParseNodePtr Parser::ParsePostfixOperators( if (pfCanAssign) { // optional assignment not permitted - *pfCanAssign = !isNullPropagating; + *pfCanAssign = !isOptionalChain; } if (pfIsDotOrIndex) { @@ -4192,6 +4193,10 @@ ParseNodePtr Parser::ParsePostfixOperators( OpCode opCode = knopDot; bool isNullPropagating = tkOptChain == m_token.tk; + if (isNullPropagating) + { + isOptionalChain = true; + } // We don't use a custom token but rather tell that knopDot is null-propagating // opCode = knopOptChain; @@ -4220,7 +4225,7 @@ ParseNodePtr Parser::ParsePostfixOperators( if (buildAST) { - if (opCode == knopDot || opCode == knopOptChain) + if (opCode == knopDot) { name = CreateNameNode(m_token.GetIdentifier(this->GetHashTbl())); } @@ -4253,7 +4258,7 @@ ParseNodePtr Parser::ParsePostfixOperators( if (pfCanAssign) { // optional assignment not permitted - *pfCanAssign = !isNullPropagating; + *pfCanAssign = !isOptionalChain; } if (pfIsDotOrIndex) { @@ -4295,6 +4300,11 @@ ParseNodePtr Parser::ParsePostfixOperators( break; } default: + if (isOptionalChain) + { + // Wrap as optional chain + return CreateUniNode(knopOptChain, pnode); + } return pnode; } } diff --git a/lib/Parser/ptlist.h b/lib/Parser/ptlist.h index cb9f0a1fe0e..e59c050aa31 100644 --- a/lib/Parser/ptlist.h +++ b/lib/Parser/ptlist.h @@ -81,7 +81,7 @@ PTNODE(knopGe , ">=" , OP(Ge) , Bin , fnopBin|fn PTNODE(knopGt , ">" , OP(Gt) , Bin , fnopBin|fnopRel , "GreaterThanOper" ) PTNODE(knopCall , "()" , Nop , Call , fnopNone , "CallExpr" ) PTNODE(knopDot , "." , Nop , Bin , fnopBin , "DotOper" ) -PTNODE(knopOptChain , "?." , Nop , Bin , fnopBin , "OptChain" ) +PTNODE(knopOptChain , "?." , Nop , Uni , fnopUni , "OptChain" ) PTNODE(knopAsg , "=" , Nop , Bin , fnopBin|fnopAsg , "AssignmentOper" ) PTNODE(knopInstOf , "instanceof" , IsInst , Bin , fnopBin|fnopRel , "InstanceOfExpr" ) PTNODE(knopIn , "in" , IsIn , Bin , fnopBin|fnopRel , "InOper" ) diff --git a/lib/Runtime/ByteCode/ByteCodeEmitter.cpp b/lib/Runtime/ByteCode/ByteCodeEmitter.cpp index 90745305b35..d9dedf22dbd 100644 --- a/lib/Runtime/ByteCode/ByteCodeEmitter.cpp +++ b/lib/Runtime/ByteCode/ByteCodeEmitter.cpp @@ -20,6 +20,25 @@ void EmitUseBeforeDeclaration(Symbol *sym, ByteCodeGenerator *byteCodeGenerator, void EmitUseBeforeDeclarationRuntimeError(ByteCodeGenerator *byteCodeGenerator, Js::RegSlot location); void VisitClearTmpRegs(ParseNode * pnode, ByteCodeGenerator * byteCodeGenerator, FuncInfo * funcInfo); +/** + * This function generates the common code for null-propagation / optional-chaining. + * This works similar to how the c# compiler emits byte-code for optional-chaining. + */ +static void EmitNullPropagation(Js::RegSlot targetObjectSlot, ByteCodeGenerator *byteCodeGenerator, FuncInfo* funcInfo, bool isNullPropagating) { + if (!isNullPropagating) + return; + + Assert(funcInfo->currentOptionalChain != 0); + + Js::ByteCodeLabel continueLabel = byteCodeGenerator->Writer()->DefineLabel(); + byteCodeGenerator->Writer()->BrReg1(Js::OpCode::BrTrue_A, continueLabel, targetObjectSlot); // if (targetObject) + { + byteCodeGenerator->Writer()->Reg1(Js::OpCode::LdUndef, funcInfo->currentOptionalChain->resultSlot); // result = undefined + byteCodeGenerator->Writer()->Br(funcInfo->currentOptionalChain->skipLabel); + } + byteCodeGenerator->Writer()->MarkLabel(continueLabel); +} + bool CallTargetIsArray(ParseNode *pnode) { return pnode->nop == knopName && pnode->AsParseNodeName()->PropertyIdFromNameNode() == Js::PropertyIds::Array; @@ -11596,12 +11615,35 @@ void Emit(ParseNode* pnode, ByteCodeGenerator* byteCodeGenerator, FuncInfo* func funcInfo->ReleaseLoc(pnode->AsParseNodeBin()->pnode1); funcInfo->AcquireLoc(pnode); + EmitNullPropagation(callObjLocation, byteCodeGenerator, funcInfo, pnode->AsParseNodeBin()->isNullPropagating); + byteCodeGenerator->Writer()->Element( Js::OpCode::LdElemI_A, pnode->location, protoLocation, pnode->AsParseNodeBin()->pnode2->location); + ENDSTATEMENET_IFTOPLEVEL(isTopLevel, pnode); break; } + + case knopOptChain: { + Js::ByteCodeLabel skipLabel = byteCodeGenerator->Writer()->DefineLabel(); + Js::RegSlot targetRegSlot = funcInfo->AcquireLoc(pnode); + + FuncInfo::OptionalChainInfo* previousChain = funcInfo->currentOptionalChain; + FuncInfo::OptionalChainInfo currentOptionalChain = FuncInfo::OptionalChainInfo(skipLabel, targetRegSlot); + funcInfo->currentOptionalChain = ¤tOptionalChain; + + ParseNodePtr innerNode = pnode->AsParseNodeUni()->pnode1; + Emit(innerNode, byteCodeGenerator, funcInfo, false); + + // Copy result + byteCodeGenerator->Writer()->Reg2(Js::OpCode::Ld_A, targetRegSlot, innerNode->location); + funcInfo->ReleaseLoc(innerNode); + + byteCodeGenerator->Writer()->MarkLabel(skipLabel); + funcInfo->currentOptionalChain = previousChain; + break; + } // this is MemberExpression as rvalue case knopDot: { @@ -11622,6 +11664,8 @@ void Emit(ParseNode* pnode, ByteCodeGenerator* byteCodeGenerator, FuncInfo* func Js::PropertyId propertyId = pnode->AsParseNodeBin()->pnode2->AsParseNodeName()->PropertyIdFromNameNode(); uint cacheId = funcInfo->FindOrAddInlineCacheId(protoLocation, propertyId, false, false); + EmitNullPropagation(callObjLocation, byteCodeGenerator, funcInfo, pnode->AsParseNodeBin()->isNullPropagating); + if (propertyId == Js::PropertyIds::length) { byteCodeGenerator->Writer()->PatchableProperty(Js::OpCode::LdLen_A, pnode->location, protoLocation, cacheId); diff --git a/lib/Runtime/ByteCode/FuncInfo.cpp b/lib/Runtime/ByteCode/FuncInfo.cpp index ca277e059cb..006fafc732e 100644 --- a/lib/Runtime/ByteCode/FuncInfo.cpp +++ b/lib/Runtime/ByteCode/FuncInfo.cpp @@ -33,6 +33,7 @@ FuncInfo::FuncInfo( outArgsCurrentExpr(0), innerScopeCount(0), currentInnerScopeIndex((uint)-1), + currentOptionalChain(nullptr), #if DBG outArgsDepth(0), #endif diff --git a/lib/Runtime/ByteCode/FuncInfo.h b/lib/Runtime/ByteCode/FuncInfo.h index ecc63f08b18..7297d39c20e 100644 --- a/lib/Runtime/ByteCode/FuncInfo.h +++ b/lib/Runtime/ByteCode/FuncInfo.h @@ -95,6 +95,15 @@ class FuncInfo Js::RegSlot outArgsCurrentExpr; // max number of out args accumulated in the current nested expression uint innerScopeCount; uint currentInnerScopeIndex; + struct OptionalChainInfo { + Js::ByteCodeLabel skipLabel; + Js::RegSlot resultSlot; + + OptionalChainInfo(Js::ByteCodeLabel skipLabel, Js::RegSlot resultSlot) { + this->skipLabel = skipLabel; + this->resultSlot = resultSlot; + } + } *currentOptionalChain; #if DBG uint32 outArgsDepth; // number of calls nested in an expression #endif From a33799e5964c954a530eb6fe0dce31ca12aeaddd Mon Sep 17 00:00:00 2001 From: Lukas Kurz Date: Mon, 15 Apr 2024 09:31:54 +0200 Subject: [PATCH 05/27] Check for `null` not truthy --- lib/Runtime/ByteCode/ByteCodeEmitter.cpp | 5 +++-- lib/Runtime/ByteCode/ByteCodeGenerator.cpp | 1 + 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/lib/Runtime/ByteCode/ByteCodeEmitter.cpp b/lib/Runtime/ByteCode/ByteCodeEmitter.cpp index d9dedf22dbd..a6b5250c282 100644 --- a/lib/Runtime/ByteCode/ByteCodeEmitter.cpp +++ b/lib/Runtime/ByteCode/ByteCodeEmitter.cpp @@ -24,14 +24,15 @@ void VisitClearTmpRegs(ParseNode * pnode, ByteCodeGenerator * byteCodeGenerator, * This function generates the common code for null-propagation / optional-chaining. * This works similar to how the c# compiler emits byte-code for optional-chaining. */ -static void EmitNullPropagation(Js::RegSlot targetObjectSlot, ByteCodeGenerator *byteCodeGenerator, FuncInfo* funcInfo, bool isNullPropagating) { +static void EmitNullPropagation(Js::RegSlot targetObjectSlot, ByteCodeGenerator *byteCodeGenerator, FuncInfo *funcInfo, bool isNullPropagating) { if (!isNullPropagating) return; Assert(funcInfo->currentOptionalChain != 0); Js::ByteCodeLabel continueLabel = byteCodeGenerator->Writer()->DefineLabel(); - byteCodeGenerator->Writer()->BrReg1(Js::OpCode::BrTrue_A, continueLabel, targetObjectSlot); // if (targetObject) + byteCodeGenerator->Writer()->BrReg2(Js::OpCode::BrNeq_A, continueLabel, targetObjectSlot, funcInfo->nullConstantRegister); + // if (targetObject == null) { byteCodeGenerator->Writer()->Reg1(Js::OpCode::LdUndef, funcInfo->currentOptionalChain->resultSlot); // result = undefined byteCodeGenerator->Writer()->Br(funcInfo->currentOptionalChain->skipLabel); diff --git a/lib/Runtime/ByteCode/ByteCodeGenerator.cpp b/lib/Runtime/ByteCode/ByteCodeGenerator.cpp index 1ed46444cf4..7e565fcb283 100644 --- a/lib/Runtime/ByteCode/ByteCodeGenerator.cpp +++ b/lib/Runtime/ByteCode/ByteCodeGenerator.cpp @@ -4962,6 +4962,7 @@ void AssignRegisters(ParseNode *pnode, ByteCodeGenerator *byteCodeGenerator) CheckMaybeEscapedUse(pnode->AsParseNodeUni()->pnode1, byteCodeGenerator); break; case knopCoalesce: + case knopOptChain: case knopObject: byteCodeGenerator->AssignNullConstRegister(); break; From caccbc6814f383ba76a389c9ba0ff2218a7746bf Mon Sep 17 00:00:00 2001 From: Lukas Kurz Date: Mon, 15 Apr 2024 11:27:27 +0200 Subject: [PATCH 06/27] Fix copyright --- lib/Runtime/ByteCode/ByteCodeGenerator.cpp | 2 +- lib/Runtime/ByteCode/FuncInfo.cpp | 1 + lib/Runtime/ByteCode/FuncInfo.h | 1 + 3 files changed, 3 insertions(+), 1 deletion(-) diff --git a/lib/Runtime/ByteCode/ByteCodeGenerator.cpp b/lib/Runtime/ByteCode/ByteCodeGenerator.cpp index 7e565fcb283..3193503457d 100644 --- a/lib/Runtime/ByteCode/ByteCodeGenerator.cpp +++ b/lib/Runtime/ByteCode/ByteCodeGenerator.cpp @@ -1,6 +1,6 @@ //------------------------------------------------------------------------------------------------------- // Copyright (C) Microsoft. All rights reserved. -// Copyright (c) 2021 ChakraCore Project Contributors. All rights reserved. +// Copyright (c) ChakraCore Project Contributors. All rights reserved. // Licensed under the MIT license. See LICENSE.txt file in the project root for full license information. //------------------------------------------------------------------------------------------------------- #include "RuntimeByteCodePch.h" diff --git a/lib/Runtime/ByteCode/FuncInfo.cpp b/lib/Runtime/ByteCode/FuncInfo.cpp index 006fafc732e..d5d8565cbd6 100644 --- a/lib/Runtime/ByteCode/FuncInfo.cpp +++ b/lib/Runtime/ByteCode/FuncInfo.cpp @@ -1,5 +1,6 @@ //------------------------------------------------------------------------------------------------------- // Copyright (C) Microsoft. All rights reserved. +// Copyright (c) ChakraCore Project Contributors. All rights reserved. // Licensed under the MIT license. See LICENSE.txt file in the project root for full license information. //------------------------------------------------------------------------------------------------------- #include "RuntimeByteCodePch.h" diff --git a/lib/Runtime/ByteCode/FuncInfo.h b/lib/Runtime/ByteCode/FuncInfo.h index 7297d39c20e..00b58a59ae3 100644 --- a/lib/Runtime/ByteCode/FuncInfo.h +++ b/lib/Runtime/ByteCode/FuncInfo.h @@ -1,5 +1,6 @@ //------------------------------------------------------------------------------------------------------- // Copyright (C) Microsoft. All rights reserved. +// Copyright (c) ChakraCore Project Contributors. All rights reserved. // Licensed under the MIT license. See LICENSE.txt file in the project root for full license information. //------------------------------------------------------------------------------------------------------- struct InlineCacheUnit From 2cde756c664a6aa79ea542e67dbe32b94b4921f6 Mon Sep 17 00:00:00 2001 From: Lukas Kurz Date: Mon, 15 Apr 2024 11:36:00 +0200 Subject: [PATCH 07/27] Use less branches --- lib/Runtime/ByteCode/ByteCodeEmitter.cpp | 27 ++++++++++++------------ lib/Runtime/ByteCode/FuncInfo.h | 4 +--- 2 files changed, 14 insertions(+), 17 deletions(-) diff --git a/lib/Runtime/ByteCode/ByteCodeEmitter.cpp b/lib/Runtime/ByteCode/ByteCodeEmitter.cpp index a6b5250c282..445929084b4 100644 --- a/lib/Runtime/ByteCode/ByteCodeEmitter.cpp +++ b/lib/Runtime/ByteCode/ByteCodeEmitter.cpp @@ -22,7 +22,6 @@ void VisitClearTmpRegs(ParseNode * pnode, ByteCodeGenerator * byteCodeGenerator, /** * This function generates the common code for null-propagation / optional-chaining. - * This works similar to how the c# compiler emits byte-code for optional-chaining. */ static void EmitNullPropagation(Js::RegSlot targetObjectSlot, ByteCodeGenerator *byteCodeGenerator, FuncInfo *funcInfo, bool isNullPropagating) { if (!isNullPropagating) @@ -30,14 +29,11 @@ static void EmitNullPropagation(Js::RegSlot targetObjectSlot, ByteCodeGenerator Assert(funcInfo->currentOptionalChain != 0); - Js::ByteCodeLabel continueLabel = byteCodeGenerator->Writer()->DefineLabel(); - byteCodeGenerator->Writer()->BrReg2(Js::OpCode::BrNeq_A, continueLabel, targetObjectSlot, funcInfo->nullConstantRegister); - // if (targetObject == null) - { - byteCodeGenerator->Writer()->Reg1(Js::OpCode::LdUndef, funcInfo->currentOptionalChain->resultSlot); // result = undefined - byteCodeGenerator->Writer()->Br(funcInfo->currentOptionalChain->skipLabel); - } - byteCodeGenerator->Writer()->MarkLabel(continueLabel); + // if (targetObject == null) goto chainEnd; + byteCodeGenerator->Writer()->BrReg2( + Js::OpCode::BrEq_A, funcInfo->currentOptionalChain->skipLabel, + targetObjectSlot, funcInfo->nullConstantRegister + ); } bool CallTargetIsArray(ParseNode *pnode) @@ -11627,18 +11623,21 @@ void Emit(ParseNode* pnode, ByteCodeGenerator* byteCodeGenerator, FuncInfo* func } case knopOptChain: { - Js::ByteCodeLabel skipLabel = byteCodeGenerator->Writer()->DefineLabel(); - Js::RegSlot targetRegSlot = funcInfo->AcquireLoc(pnode); + FuncInfo::OptionalChainInfo *previousChain = funcInfo->currentOptionalChain; - FuncInfo::OptionalChainInfo* previousChain = funcInfo->currentOptionalChain; - FuncInfo::OptionalChainInfo currentOptionalChain = FuncInfo::OptionalChainInfo(skipLabel, targetRegSlot); + Js::ByteCodeLabel skipLabel = byteCodeGenerator->Writer()->DefineLabel(); + FuncInfo::OptionalChainInfo currentOptionalChain = FuncInfo::OptionalChainInfo(skipLabel); funcInfo->currentOptionalChain = ¤tOptionalChain; + Js::RegSlot resultSlot = funcInfo->AcquireLoc(pnode); + byteCodeGenerator->Writer()->Reg1(Js::OpCode::LdUndef, resultSlot); // result = undefined + + // emit chain ParseNodePtr innerNode = pnode->AsParseNodeUni()->pnode1; Emit(innerNode, byteCodeGenerator, funcInfo, false); // Copy result - byteCodeGenerator->Writer()->Reg2(Js::OpCode::Ld_A, targetRegSlot, innerNode->location); + byteCodeGenerator->Writer()->Reg2(Js::OpCode::Ld_A, resultSlot, innerNode->location); funcInfo->ReleaseLoc(innerNode); byteCodeGenerator->Writer()->MarkLabel(skipLabel); diff --git a/lib/Runtime/ByteCode/FuncInfo.h b/lib/Runtime/ByteCode/FuncInfo.h index 00b58a59ae3..3f983069fae 100644 --- a/lib/Runtime/ByteCode/FuncInfo.h +++ b/lib/Runtime/ByteCode/FuncInfo.h @@ -98,11 +98,9 @@ class FuncInfo uint currentInnerScopeIndex; struct OptionalChainInfo { Js::ByteCodeLabel skipLabel; - Js::RegSlot resultSlot; - OptionalChainInfo(Js::ByteCodeLabel skipLabel, Js::RegSlot resultSlot) { + OptionalChainInfo(Js::ByteCodeLabel skipLabel) { this->skipLabel = skipLabel; - this->resultSlot = resultSlot; } } *currentOptionalChain; #if DBG From f833213eee87a9cdc533e7fb875dce2d8372edee Mon Sep 17 00:00:00 2001 From: Lukas Kurz Date: Mon, 15 Apr 2024 11:44:30 +0200 Subject: [PATCH 08/27] More copyright fixes --- lib/Parser/Parse.h | 1 + lib/Runtime/ByteCode/ByteCodeEmitter.cpp | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/Parser/Parse.h b/lib/Parser/Parse.h index f72b0f9376a..bc4f2a5cd5d 100644 --- a/lib/Parser/Parse.h +++ b/lib/Parser/Parse.h @@ -1,5 +1,6 @@ //------------------------------------------------------------------------------------------------------- // Copyright (C) Microsoft. All rights reserved. +// Copyright (c) ChakraCore Project Contributors. All rights reserved. // Licensed under the MIT license. See LICENSE.txt file in the project root for full license information. //------------------------------------------------------------------------------------------------------- #pragma once diff --git a/lib/Runtime/ByteCode/ByteCodeEmitter.cpp b/lib/Runtime/ByteCode/ByteCodeEmitter.cpp index 445929084b4..747fe4bc978 100644 --- a/lib/Runtime/ByteCode/ByteCodeEmitter.cpp +++ b/lib/Runtime/ByteCode/ByteCodeEmitter.cpp @@ -1,6 +1,6 @@ //------------------------------------------------------------------------------------------------------- // Copyright (C) Microsoft. All rights reserved. -// Copyright (c) 2021 ChakraCore Project Contributors. All rights reserved. +// Copyright (c) ChakraCore Project Contributors. All rights reserved. // Licensed under the MIT license. See LICENSE.txt file in the project root for full license information. //------------------------------------------------------------------------------------------------------- #include "RuntimeByteCodePch.h" From d2377a450586ed9b9ed742e6fcbc7f489820a1b0 Mon Sep 17 00:00:00 2001 From: Lukas Kurz Date: Mon, 15 Apr 2024 13:48:08 +0200 Subject: [PATCH 09/27] Don't break ternary with decimal numbers --- lib/Parser/Scan.cpp | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/lib/Parser/Scan.cpp b/lib/Parser/Scan.cpp index 72603dbe4b1..6dcedc9973c 100644 --- a/lib/Parser/Scan.cpp +++ b/lib/Parser/Scan.cpp @@ -1783,6 +1783,11 @@ tokens Scanner::ScanCore(bool identifyKwds) } else if (m_scriptContext->GetConfig()->IsESOptionalChainingEnabled() && this->PeekFirst(p, last) == '.') { + // `a?.3:0` is actually a ternary operator containing the number `0.3` + bool isTernary = CharTypes::_C_DIG == this->charClassifier->GetCharType(this->PeekFirst(p + 1, last)); + if (isTernary) + break; + p++; token = tkOptChain; break; From 6a2c0ecfcf20827175c73426a03eae15ac52a903 Mon Sep 17 00:00:00 2001 From: Lukas Kurz Date: Mon, 15 Apr 2024 20:18:24 +0200 Subject: [PATCH 10/27] Honor `buildAST` --- lib/Parser/Parse.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/Parser/Parse.cpp b/lib/Parser/Parse.cpp index 39e4cec82b0..d035078c082 100644 --- a/lib/Parser/Parse.cpp +++ b/lib/Parser/Parse.cpp @@ -4300,7 +4300,7 @@ ParseNodePtr Parser::ParsePostfixOperators( break; } default: - if (isOptionalChain) + if (buildAST && isOptionalChain) { // Wrap as optional chain return CreateUniNode(knopOptChain, pnode); From e3820ecf7c22dba56e172ef208150bb6369c497b Mon Sep 17 00:00:00 2001 From: Lukas Kurz Date: Mon, 15 Apr 2024 20:17:00 +0200 Subject: [PATCH 11/27] Tagged template in optional chain is syntax error --- lib/Parser/Parse.cpp | 22 ++++++++++++++++++---- lib/Parser/perrors.h | 3 ++- 2 files changed, 20 insertions(+), 5 deletions(-) diff --git a/lib/Parser/Parse.cpp b/lib/Parser/Parse.cpp index d035078c082..6a41386d789 100644 --- a/lib/Parser/Parse.cpp +++ b/lib/Parser/Parse.cpp @@ -4203,11 +4203,20 @@ ParseNodePtr Parser::ParsePostfixOperators( this->GetScanner()->Scan(); if (!m_token.IsIdentifier()) { - if (isNullPropagating && (tkLParen == m_token.tk || tkLBrack == m_token.tk)) + if (isNullPropagating) { - // Continue to parse function or index (loop) - // Check previous token to check for null-propagation - continue; + switch (m_token.tk) + { + case tkLParen: + case tkLBrack: + // Continue to parse function or index (loop) + // Check previous token to check for null-propagation + continue; + + case tkStrTmplBasic: + case tkStrTmplBegin: + Error(ERRInvalidOptChainWithTaggedTemplate); + } } else if (!(m_token.IsReservedWord())) //allow reserved words in ES5 mode { @@ -4272,6 +4281,11 @@ ParseNodePtr Parser::ParsePostfixOperators( case tkStrTmplBasic: case tkStrTmplBegin: { + if (isOptionalChain) + { + Error(ERRInvalidOptChainWithTaggedTemplate); + } + ParseNode* templateNode = nullptr; if (pnode != nullptr) { diff --git a/lib/Parser/perrors.h b/lib/Parser/perrors.h index 4155fa81695..59c91b08753 100644 --- a/lib/Parser/perrors.h +++ b/lib/Parser/perrors.h @@ -123,7 +123,8 @@ LSC_ERROR_MSG(1103, ERRMissingFrom, "Expected 'from' after import or export clau LSC_ERROR_MSG(1105, ERRInvalidOptChainInNew, "Invalid optional chain in new expression.") LSC_ERROR_MSG(1106, ERRInvalidOptChainInSuper, "Invalid optional chain in call to 'super'.") -// 1107-1199 available for future use +LSC_ERROR_MSG(1107, ERRInvalidOptChainWithTaggedTemplate, "Invalid tagged template in optional chain.") +// 1108-1199 available for future use // Generic errors intended to be re-usable LSC_ERROR_MSG(1200, ERRKeywordAfter, "Unexpected keyword '%s' after '%s'") From cc547645d85b9e4db2070cc000449266ac9a3167 Mon Sep 17 00:00:00 2001 From: Lukas Kurz Date: Mon, 15 Apr 2024 21:02:34 +0200 Subject: [PATCH 12/27] short-circuit indexer expressions --- lib/Runtime/ByteCode/ByteCodeEmitter.cpp | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/lib/Runtime/ByteCode/ByteCodeEmitter.cpp b/lib/Runtime/ByteCode/ByteCodeEmitter.cpp index 747fe4bc978..8be24aa1610 100644 --- a/lib/Runtime/ByteCode/ByteCodeEmitter.cpp +++ b/lib/Runtime/ByteCode/ByteCodeEmitter.cpp @@ -268,7 +268,7 @@ bool IsArguments(ParseNode *pnode) bool ApplyEnclosesArgs(ParseNode* fncDecl, ByteCodeGenerator* byteCodeGenerator); void Emit(ParseNode* pnode, ByteCodeGenerator* byteCodeGenerator, FuncInfo* funcInfo, BOOL fReturnValue, bool isConstructorCall = false, bool isTopLevel = false); -void EmitBinaryOpnds(ParseNode* pnode1, ParseNode* pnode2, ByteCodeGenerator* byteCodeGenerator, FuncInfo* funcInfo, Js::RegSlot computedPropertyLocation = Js::Constants::NoRegister); +void EmitBinaryOpnds(ParseNode* pnode1, ParseNode* pnode2, ByteCodeGenerator* byteCodeGenerator, FuncInfo* funcInfo, Js::RegSlot computedPropertyLocation = Js::Constants::NoRegister, bool isNullPropagating = false); bool IsExpressionStatement(ParseNode* stmt, const Js::ScriptContext *const scriptContext); void EmitInvoke(Js::RegSlot location, Js::RegSlot callObjLocation, Js::PropertyId propertyId, ByteCodeGenerator* byteCodeGenerator, FuncInfo* funcInfo); void EmitInvoke(Js::RegSlot location, Js::RegSlot callObjLocation, Js::PropertyId propertyId, ByteCodeGenerator* byteCodeGenerator, FuncInfo* funcInfo, Js::RegSlot arg1Location); @@ -10097,7 +10097,7 @@ void ByteCodeGenerator::EmitJumpCleanup(ParseNode* target, FuncInfo* funcInfo) } } -void EmitBinaryOpnds(ParseNode* pnode1, ParseNode* pnode2, ByteCodeGenerator* byteCodeGenerator, FuncInfo* funcInfo, Js::RegSlot computedPropertyLocation) +void EmitBinaryOpnds(ParseNode* pnode1, ParseNode* pnode2, ByteCodeGenerator* byteCodeGenerator, FuncInfo* funcInfo, Js::RegSlot computedPropertyLocation, bool isNullPropagating) { // If opnd2 can overwrite opnd1, make sure the value of opnd1 is stashed away. if (MayHaveSideEffectOnNode(pnode1, pnode2, byteCodeGenerator)) @@ -10112,6 +10112,7 @@ void EmitBinaryOpnds(ParseNode* pnode1, ParseNode* pnode2, ByteCodeGenerator* by byteCodeGenerator->Writer()->Reg2(Js::OpCode::Conv_Prop, computedPropertyLocation, pnode1->location); } + EmitNullPropagation(pnode1->location, byteCodeGenerator, funcInfo, isNullPropagating); Emit(pnode2, byteCodeGenerator, funcInfo, false, false, computedPropertyLocation); } @@ -11596,7 +11597,10 @@ void Emit(ParseNode* pnode, ByteCodeGenerator* byteCodeGenerator, FuncInfo* func case knopIndex: { STARTSTATEMENET_IFTOPLEVEL(isTopLevel, pnode); - EmitBinaryOpnds(pnode->AsParseNodeBin()->pnode1, pnode->AsParseNodeBin()->pnode2, byteCodeGenerator, funcInfo); + EmitBinaryOpnds(pnode->AsParseNodeBin()->pnode1, pnode->AsParseNodeBin()->pnode2, byteCodeGenerator, funcInfo, + Js::Constants::NoRegister, + // EmitNullPropagation is called in EmitBinaryOpnds to short-circuit indexer content + pnode->AsParseNodeBin()->isNullPropagating); Js::RegSlot callObjLocation = pnode->AsParseNodeBin()->pnode1->location; Js::RegSlot protoLocation = callObjLocation; @@ -11612,8 +11616,6 @@ void Emit(ParseNode* pnode, ByteCodeGenerator* byteCodeGenerator, FuncInfo* func funcInfo->ReleaseLoc(pnode->AsParseNodeBin()->pnode1); funcInfo->AcquireLoc(pnode); - EmitNullPropagation(callObjLocation, byteCodeGenerator, funcInfo, pnode->AsParseNodeBin()->isNullPropagating); - byteCodeGenerator->Writer()->Element( Js::OpCode::LdElemI_A, pnode->location, protoLocation, pnode->AsParseNodeBin()->pnode2->location); From 9e11b34a01b118e9bc056e5853c38d93f4757b07 Mon Sep 17 00:00:00 2001 From: Lukas Kurz Date: Fri, 19 Apr 2024 00:44:07 +0200 Subject: [PATCH 13/27] Simple method call --- lib/Runtime/ByteCode/ByteCodeEmitter.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lib/Runtime/ByteCode/ByteCodeEmitter.cpp b/lib/Runtime/ByteCode/ByteCodeEmitter.cpp index 8be24aa1610..d0236950cac 100644 --- a/lib/Runtime/ByteCode/ByteCodeEmitter.cpp +++ b/lib/Runtime/ByteCode/ByteCodeEmitter.cpp @@ -8592,6 +8592,8 @@ void EmitCall( } } + EmitNullPropagation(pnodeCall->pnodeTarget->location, byteCodeGenerator, funcInfo, pnodeCall->isNullPropagating); + // If we are strictly overriding the this location, ignore what the call target set this location to. if (overrideThisLocation != Js::Constants::NoRegister) { From 2498b2d3d71d311fb6f655aea1d9cdc70f9e3b9c Mon Sep 17 00:00:00 2001 From: Lukas Kurz Date: Fri, 19 Apr 2024 01:17:42 +0200 Subject: [PATCH 14/27] Comments and review --- lib/Parser/Parse.cpp | 7 +++-- lib/Runtime/ByteCode/ByteCodeEmitter.cpp | 39 +++++++++++++++--------- lib/Runtime/ByteCode/FuncInfo.cpp | 2 +- lib/Runtime/ByteCode/FuncInfo.h | 8 +---- 4 files changed, 30 insertions(+), 26 deletions(-) diff --git a/lib/Parser/Parse.cpp b/lib/Parser/Parse.cpp index 6a41386d789..1e98932ae7e 100644 --- a/lib/Parser/Parse.cpp +++ b/lib/Parser/Parse.cpp @@ -4192,19 +4192,20 @@ ParseNodePtr Parser::ParsePostfixOperators( ParseNodePtr name = nullptr; OpCode opCode = knopDot; + // We don't use separate knops for optional-chains + // Instead mark nodes as null-propagating bool isNullPropagating = tkOptChain == m_token.tk; if (isNullPropagating) { isOptionalChain = true; } - // We don't use a custom token but rather tell that knopDot is null-propagating - // opCode = knopOptChain; this->GetScanner()->Scan(); if (!m_token.IsIdentifier()) { if (isNullPropagating) { + // We don't need an identifier for an Index `?.[` or Call `?.(` switch (m_token.tk) { case tkLParen: @@ -4316,7 +4317,7 @@ ParseNodePtr Parser::ParsePostfixOperators( default: if (buildAST && isOptionalChain) { - // Wrap as optional chain + // Wrap the whole expression as an optional-chain return CreateUniNode(knopOptChain, pnode); } return pnode; diff --git a/lib/Runtime/ByteCode/ByteCodeEmitter.cpp b/lib/Runtime/ByteCode/ByteCodeEmitter.cpp index d0236950cac..c9eec0063d2 100644 --- a/lib/Runtime/ByteCode/ByteCodeEmitter.cpp +++ b/lib/Runtime/ByteCode/ByteCodeEmitter.cpp @@ -20,18 +20,23 @@ void EmitUseBeforeDeclaration(Symbol *sym, ByteCodeGenerator *byteCodeGenerator, void EmitUseBeforeDeclarationRuntimeError(ByteCodeGenerator *byteCodeGenerator, Js::RegSlot location); void VisitClearTmpRegs(ParseNode * pnode, ByteCodeGenerator * byteCodeGenerator, FuncInfo * funcInfo); -/** - * This function generates the common code for null-propagation / optional-chaining. - */ +/// +/// This function generates the common code for null-propagation / optional-chaining. +/// If the targetObject is nullish this will short-circuit(skip) to the end of the chain-expression. +/// +/// It should be called on every ?. location. +/// A call to this function is only valid from a node-emission inside a `knopOptChain` node. +/// static void EmitNullPropagation(Js::RegSlot targetObjectSlot, ByteCodeGenerator *byteCodeGenerator, FuncInfo *funcInfo, bool isNullPropagating) { if (!isNullPropagating) return; - Assert(funcInfo->currentOptionalChain != 0); + // Ensure we've setup the skipLabel in the emission of a `knopOptChain` + Assert(funcInfo->currentOptionalChainSkipLabel >= 0); - // if (targetObject == null) goto chainEnd; + // if (targetObject == null) goto skipLabel; byteCodeGenerator->Writer()->BrReg2( - Js::OpCode::BrEq_A, funcInfo->currentOptionalChain->skipLabel, + Js::OpCode::BrEq_A, funcInfo->currentOptionalChainSkipLabel, targetObjectSlot, funcInfo->nullConstantRegister ); } @@ -11621,31 +11626,35 @@ void Emit(ParseNode* pnode, ByteCodeGenerator* byteCodeGenerator, FuncInfo* func byteCodeGenerator->Writer()->Element( Js::OpCode::LdElemI_A, pnode->location, protoLocation, pnode->AsParseNodeBin()->pnode2->location); - ENDSTATEMENET_IFTOPLEVEL(isTopLevel, pnode); break; } - + // The whole optional-chain expression will be wrapped in a UniNode with `knopOptChain`. case knopOptChain: { - FuncInfo::OptionalChainInfo *previousChain = funcInfo->currentOptionalChain; + Js::ByteCodeLabel previousSkipLabel = funcInfo->currentOptionalChainSkipLabel; + // Create a label that can skip the whole chain and store in `funcInfo` Js::ByteCodeLabel skipLabel = byteCodeGenerator->Writer()->DefineLabel(); - FuncInfo::OptionalChainInfo currentOptionalChain = FuncInfo::OptionalChainInfo(skipLabel); - funcInfo->currentOptionalChain = ¤tOptionalChain; + funcInfo->currentOptionalChainSkipLabel = skipLabel; + // Acquire slot for the result value + // Prefill it with `undefined` (Fallback for short-circuiting) Js::RegSlot resultSlot = funcInfo->AcquireLoc(pnode); - byteCodeGenerator->Writer()->Reg1(Js::OpCode::LdUndef, resultSlot); // result = undefined + byteCodeGenerator->Writer()->Reg1(Js::OpCode::LdUndef, resultSlot); - // emit chain + // emit chain expression + // Every `?.` node will call `EmitNullPropagation` + // `EmitNullPropagation` short-circuits to `skipLabel` in case of a nullish value ParseNodePtr innerNode = pnode->AsParseNodeUni()->pnode1; Emit(innerNode, byteCodeGenerator, funcInfo, false); - // Copy result + // Copy the expression result + // Only reached if we did not short-circuit byteCodeGenerator->Writer()->Reg2(Js::OpCode::Ld_A, resultSlot, innerNode->location); funcInfo->ReleaseLoc(innerNode); byteCodeGenerator->Writer()->MarkLabel(skipLabel); - funcInfo->currentOptionalChain = previousChain; + funcInfo->currentOptionalChainSkipLabel = previousSkipLabel; break; } // this is MemberExpression as rvalue diff --git a/lib/Runtime/ByteCode/FuncInfo.cpp b/lib/Runtime/ByteCode/FuncInfo.cpp index d5d8565cbd6..be75552b79d 100644 --- a/lib/Runtime/ByteCode/FuncInfo.cpp +++ b/lib/Runtime/ByteCode/FuncInfo.cpp @@ -34,7 +34,7 @@ FuncInfo::FuncInfo( outArgsCurrentExpr(0), innerScopeCount(0), currentInnerScopeIndex((uint)-1), - currentOptionalChain(nullptr), + currentOptionalChainSkipLabel(-1), #if DBG outArgsDepth(0), #endif diff --git a/lib/Runtime/ByteCode/FuncInfo.h b/lib/Runtime/ByteCode/FuncInfo.h index 3f983069fae..9fe4754ead1 100644 --- a/lib/Runtime/ByteCode/FuncInfo.h +++ b/lib/Runtime/ByteCode/FuncInfo.h @@ -96,13 +96,7 @@ class FuncInfo Js::RegSlot outArgsCurrentExpr; // max number of out args accumulated in the current nested expression uint innerScopeCount; uint currentInnerScopeIndex; - struct OptionalChainInfo { - Js::ByteCodeLabel skipLabel; - - OptionalChainInfo(Js::ByteCodeLabel skipLabel) { - this->skipLabel = skipLabel; - } - } *currentOptionalChain; + Js::ByteCodeLabel currentOptionalChainSkipLabel; #if DBG uint32 outArgsDepth; // number of calls nested in an expression #endif From 6e7d93508ae687b25c88f81e89a2e4bc721c445e Mon Sep 17 00:00:00 2001 From: Lukas Kurz Date: Sat, 20 Apr 2024 23:54:05 +0200 Subject: [PATCH 15/27] Fix optChain right before function call --- lib/Runtime/ByteCode/ByteCodeEmitter.cpp | 27 +++++++++++++----------- 1 file changed, 15 insertions(+), 12 deletions(-) diff --git a/lib/Runtime/ByteCode/ByteCodeEmitter.cpp b/lib/Runtime/ByteCode/ByteCodeEmitter.cpp index c9eec0063d2..afe43b49eb1 100644 --- a/lib/Runtime/ByteCode/ByteCodeEmitter.cpp +++ b/lib/Runtime/ByteCode/ByteCodeEmitter.cpp @@ -8122,6 +8122,7 @@ void EmitCallTarget( else { *thisLocation = pnodeBinTarget->pnode1->location; + EmitNullPropagation(pnodeBinTarget->pnode1->location, byteCodeGenerator, funcInfo, pnodeBinTarget->isNullPropagating); EmitMethodFld(pnodeBinTarget, protoLocation, propertyId, byteCodeGenerator, funcInfo); } @@ -8130,22 +8131,24 @@ void EmitCallTarget( case knopIndex: { - funcInfo->AcquireLoc(pnodeTarget); + ParseNodeBin *pnodeBinTarget = pnodeTarget->AsParseNodeBin(); + funcInfo->AcquireLoc(pnodeBinTarget); // Assign the call target operand(s), putting them into expression temps if necessary to protect // them from side-effects. - if (fSideEffectArgs || !(ParseNode::Grfnop(pnodeTarget->AsParseNodeBin()->pnode2->nop) & fnopLeaf)) + if (fSideEffectArgs || !(ParseNode::Grfnop(pnodeBinTarget->pnode2->nop) & fnopLeaf)) { // Though we're done with target evaluation after this point, still protect opnd1 from // arg or opnd2 side-effects as it's the "this" pointer. - SaveOpndValue(pnodeTarget->AsParseNodeBin()->pnode1, funcInfo); + SaveOpndValue(pnodeBinTarget->pnode1, funcInfo); } - Emit(pnodeTarget->AsParseNodeBin()->pnode1, byteCodeGenerator, funcInfo, false); - Emit(pnodeTarget->AsParseNodeBin()->pnode2, byteCodeGenerator, funcInfo, false); + Emit(pnodeBinTarget->pnode1, byteCodeGenerator, funcInfo, false); + EmitNullPropagation(pnodeBinTarget->pnode1->location, byteCodeGenerator, funcInfo, pnodeBinTarget->isNullPropagating); + Emit(pnodeBinTarget->pnode2, byteCodeGenerator, funcInfo, false); - Js::RegSlot indexLocation = pnodeTarget->AsParseNodeBin()->pnode2->location; - Js::RegSlot protoLocation = pnodeTarget->AsParseNodeBin()->pnode1->location; + Js::RegSlot indexLocation = pnodeBinTarget->pnode2->location; + Js::RegSlot protoLocation = pnodeBinTarget->pnode1->location; - if (ByteCodeGenerator::IsSuper(pnodeTarget->AsParseNodeBin()->pnode1)) + if (ByteCodeGenerator::IsSuper(pnodeBinTarget->pnode1)) { Emit(pnodeTarget->AsParseNodeSuperReference()->pnodeThis, byteCodeGenerator, funcInfo, false); protoLocation = byteCodeGenerator->EmitLdObjProto(Js::OpCode::LdHomeObjProto, protoLocation, funcInfo); @@ -8157,16 +8160,16 @@ void EmitCallTarget( } else { - *thisLocation = pnodeTarget->AsParseNodeBin()->pnode1->location; + *thisLocation = pnodeBinTarget->pnode1->location; } EmitMethodElem(pnodeTarget, protoLocation, indexLocation, byteCodeGenerator); - funcInfo->ReleaseLoc(pnodeTarget->AsParseNodeBin()->pnode2); // don't release indexLocation until after we use it. + funcInfo->ReleaseLoc(pnodeBinTarget->pnode2); // don't release indexLocation until after we use it. - if (ByteCodeGenerator::IsSuper(pnodeTarget->AsParseNodeBin()->pnode1)) + if (ByteCodeGenerator::IsSuper(pnodeBinTarget->pnode1)) { - funcInfo->ReleaseLoc(pnodeTarget->AsParseNodeBin()->pnode1); + funcInfo->ReleaseLoc(pnodeBinTarget->pnode1); } break; } From 93698457c0045cb347042beacecaf517dd878ca0 Mon Sep 17 00:00:00 2001 From: Lukas Kurz Date: Sun, 21 Apr 2024 16:32:47 +0200 Subject: [PATCH 16/27] Fix `this` propagation No copy of expression result needed anymore --- lib/Runtime/ByteCode/ByteCodeEmitter.cpp | 73 +++++++++++++++--------- 1 file changed, 46 insertions(+), 27 deletions(-) diff --git a/lib/Runtime/ByteCode/ByteCodeEmitter.cpp b/lib/Runtime/ByteCode/ByteCodeEmitter.cpp index afe43b49eb1..8718849aa38 100644 --- a/lib/Runtime/ByteCode/ByteCodeEmitter.cpp +++ b/lib/Runtime/ByteCode/ByteCodeEmitter.cpp @@ -8,6 +8,7 @@ #include "Language/AsmJs.h" #include "ConfigFlagsList.h" +void Emit(ParseNode *pnode, ByteCodeGenerator *byteCodeGenerator, FuncInfo *funcInfo, BOOL fReturnValue, bool isConstructorCall = false, bool isTopLevel = false); void EmitReference(ParseNode *pnode, ByteCodeGenerator *byteCodeGenerator, FuncInfo *funcInfo); void EmitAssignment(ParseNode *asgnNode, ParseNode *lhs, Js::RegSlot rhsLocation, ByteCodeGenerator *byteCodeGenerator, FuncInfo *funcInfo); void EmitLoad(ParseNode *rhs, ByteCodeGenerator *byteCodeGenerator, FuncInfo *funcInfo); @@ -26,6 +27,7 @@ void VisitClearTmpRegs(ParseNode * pnode, ByteCodeGenerator * byteCodeGenerator, /// /// It should be called on every ?. location. /// A call to this function is only valid from a node-emission inside a `knopOptChain` node. +/// See EmitOptionalChain. /// static void EmitNullPropagation(Js::RegSlot targetObjectSlot, ByteCodeGenerator *byteCodeGenerator, FuncInfo *funcInfo, bool isNullPropagating) { if (!isNullPropagating) @@ -41,6 +43,39 @@ static void EmitNullPropagation(Js::RegSlot targetObjectSlot, ByteCodeGenerator ); } +/// +/// The whole optional-chain expression will be wrapped in a UniNode with `knopOptChain`. +/// Use this function to emit the whole expression. +/// +template +static void EmitOptionalChainWrapper(ParseNodeUni *pnodeOptChain, ByteCodeGenerator *byteCodeGenerator, FuncInfo *funcInfo, TEmitProc emitChainContent) { + Assert(knopOptChain == pnodeOptChain->nop); + + Js::ByteCodeLabel previousSkipLabel = funcInfo->currentOptionalChainSkipLabel; + + // Create a label that can skip the whole chain and store it in `funcInfo` + Js::ByteCodeLabel skipLabel = byteCodeGenerator->Writer()->DefineLabel(); + funcInfo->currentOptionalChainSkipLabel = skipLabel; + + // Acquire slot for the result value + // Prefill it with `undefined` (Fallback for short-circuiting) + Js::RegSlot resultSlot = funcInfo->AcquireLoc(pnodeOptChain); + byteCodeGenerator->Writer()->Reg1(Js::OpCode::LdUndef, resultSlot); + + // Copy values from wrapper to inner expression + ParseNodePtr innerNode = pnodeOptChain->pnode1; + innerNode->isUsed = pnodeOptChain->isUsed; + innerNode->location = pnodeOptChain->location; + + // emit chain expression + // Every `?.` node will call `EmitNullPropagation` + // `EmitNullPropagation` short-circuits to `skipLabel` in case of a nullish value + emitChainContent(innerNode); + + byteCodeGenerator->Writer()->MarkLabel(skipLabel); + funcInfo->currentOptionalChainSkipLabel = previousSkipLabel; +} + bool CallTargetIsArray(ParseNode *pnode) { return pnode->nop == knopName && pnode->AsParseNodeName()->PropertyIdFromNameNode() == Js::PropertyIds::Array; @@ -272,7 +307,6 @@ bool IsArguments(ParseNode *pnode) } bool ApplyEnclosesArgs(ParseNode* fncDecl, ByteCodeGenerator* byteCodeGenerator); -void Emit(ParseNode* pnode, ByteCodeGenerator* byteCodeGenerator, FuncInfo* funcInfo, BOOL fReturnValue, bool isConstructorCall = false, bool isTopLevel = false); void EmitBinaryOpnds(ParseNode* pnode1, ParseNode* pnode2, ByteCodeGenerator* byteCodeGenerator, FuncInfo* funcInfo, Js::RegSlot computedPropertyLocation = Js::Constants::NoRegister, bool isNullPropagating = false); bool IsExpressionStatement(ParseNode* stmt, const Js::ScriptContext *const scriptContext); void EmitInvoke(Js::RegSlot location, Js::RegSlot callObjLocation, Js::PropertyId propertyId, ByteCodeGenerator* byteCodeGenerator, FuncInfo* funcInfo); @@ -8078,6 +8112,12 @@ void EmitCallTarget( switch (pnodeTarget->nop) { + case knopOptChain: { + EmitOptionalChainWrapper(pnodeTarget->AsParseNodeUni(), byteCodeGenerator, funcInfo, [&](ParseNodePtr innerNode) { + EmitCallTarget(innerNode, fSideEffectArgs, thisLocation, releaseThisLocation, callObjLocation, byteCodeGenerator, funcInfo, callApplyCallSiteId); + }); + break; + } case knopDot: { ParseNodeBin * pnodeBinTarget = pnodeTarget->AsParseNodeBin(); @@ -11632,34 +11672,13 @@ void Emit(ParseNode* pnode, ByteCodeGenerator* byteCodeGenerator, FuncInfo* func ENDSTATEMENET_IFTOPLEVEL(isTopLevel, pnode); break; } - // The whole optional-chain expression will be wrapped in a UniNode with `knopOptChain`. - case knopOptChain: { - Js::ByteCodeLabel previousSkipLabel = funcInfo->currentOptionalChainSkipLabel; - - // Create a label that can skip the whole chain and store in `funcInfo` - Js::ByteCodeLabel skipLabel = byteCodeGenerator->Writer()->DefineLabel(); - funcInfo->currentOptionalChainSkipLabel = skipLabel; - - // Acquire slot for the result value - // Prefill it with `undefined` (Fallback for short-circuiting) - Js::RegSlot resultSlot = funcInfo->AcquireLoc(pnode); - byteCodeGenerator->Writer()->Reg1(Js::OpCode::LdUndef, resultSlot); - // emit chain expression - // Every `?.` node will call `EmitNullPropagation` - // `EmitNullPropagation` short-circuits to `skipLabel` in case of a nullish value - ParseNodePtr innerNode = pnode->AsParseNodeUni()->pnode1; - Emit(innerNode, byteCodeGenerator, funcInfo, false); - - // Copy the expression result - // Only reached if we did not short-circuit - byteCodeGenerator->Writer()->Reg2(Js::OpCode::Ld_A, resultSlot, innerNode->location); - funcInfo->ReleaseLoc(innerNode); - - byteCodeGenerator->Writer()->MarkLabel(skipLabel); - funcInfo->currentOptionalChainSkipLabel = previousSkipLabel; + case knopOptChain: + EmitOptionalChainWrapper(pnode->AsParseNodeUni(), byteCodeGenerator, funcInfo, [&](ParseNodePtr innerNode) { + Emit(innerNode, byteCodeGenerator, funcInfo, false); + }); break; - } + // this is MemberExpression as rvalue case knopDot: { From ba32362e9a7814b6903ef8af57afcb2f5f3c05c6 Mon Sep 17 00:00:00 2001 From: Lukas Kurz Date: Wed, 24 Apr 2024 17:24:16 +0200 Subject: [PATCH 17/27] Basic tests --- test/es12/optional-chaining.js | 223 +++++++++++++++++++++++++++++++++ test/es12/rlexe.xml | 9 ++ test/rlexedirs.xml | 5 + 3 files changed, 237 insertions(+) create mode 100644 test/es12/optional-chaining.js create mode 100644 test/es12/rlexe.xml diff --git a/test/es12/optional-chaining.js b/test/es12/optional-chaining.js new file mode 100644 index 00000000000..a90fca6dd33 --- /dev/null +++ b/test/es12/optional-chaining.js @@ -0,0 +1,223 @@ +//------------------------------------------------------------------------------------------------------- +// Copyright (C) Microsoft. All rights reserved. +// Copyright (c) ChakraCore Project Contributors. All rights reserved. +// Licensed under the MIT license. See LICENSE.txt file in the project root for full license information. +//------------------------------------------------------------------------------------------------------- + +// @ts-check +/// + +WScript.LoadScriptFile("..\\UnitTestFramework\\UnitTestFramework.js"); + +const simpleObj = { "null": null, "undefined": undefined }; +Object.freeze(simpleObj); + +const tests = [ + { + name: "Simple properties", + body() { + // Verify normal behavior + assert.throws(() => simpleObj.nothing.something, TypeError); + assert.throws(() => simpleObj.null.something, TypeError); + assert.throws(() => simpleObj.undefined.something, TypeError); + + // With optional-chains + assert.isUndefined(simpleObj.nothing?.something, "OptChain should evaluated to 'undefined'"); + assert.isUndefined(simpleObj.null?.something, "OptChain should evaluated to 'undefined'"); + assert.isUndefined(simpleObj.undefined?.something, "OptChain should evaluated to 'undefined'"); + } + }, + { + name: "Simple indexers", + body() { + // Verify normal behavior + assert.throws(() => simpleObj.nothing["something"], TypeError); + assert.throws(() => simpleObj.null["something"], TypeError); + assert.throws(() => simpleObj.undefined["something"], TypeError); + + // With optional-chains + assert.isUndefined(simpleObj.nothing?.["something"], "OptChain should evaluated to 'undefined'"); + assert.isUndefined(simpleObj.null?.["something"], "OptChain should evaluated to 'undefined'"); + assert.isUndefined(simpleObj.undefined?.["something"], "OptChain should evaluated to 'undefined'"); + } + }, + { + name: "Simple method calls", + body() { + // Verify normal behavior + assert.throws(() => simpleObj.nothing(), TypeError); + assert.throws(() => simpleObj.null(), TypeError); + assert.throws(() => simpleObj.undefined(), TypeError); + + // With optional-chains + assert.isUndefined(simpleObj.nothing?.(), "OptChain should evaluated to 'undefined'"); + assert.isUndefined(simpleObj.null?.(), "OptChain should evaluated to 'undefined'"); + assert.isUndefined(simpleObj.undefined?.(), "OptChain should evaluated to 'undefined'"); + } + }, + { + name: "Simple properties before call", + body() { + // Verify normal behavior + assert.throws(() => simpleObj.nothing.something(), TypeError); + assert.throws(() => simpleObj.null.something(), TypeError); + assert.throws(() => simpleObj.undefined.something(), TypeError); + + // With optional-chains + assert.isUndefined(simpleObj.nothing?.something(), "OptChain should evaluated to 'undefined'"); + assert.isUndefined(simpleObj.null?.something(), "OptChain should evaluated to 'undefined'"); + assert.isUndefined(simpleObj.undefined?.something(), "OptChain should evaluated to 'undefined'"); + } + }, + { + name: "Simple indexers before call", + body() { + // Verify normal behavior + assert.throws(() => simpleObj.nothing["something"], TypeError); + assert.throws(() => simpleObj.null["something"], TypeError); + assert.throws(() => simpleObj.undefined["something"], TypeError); + + // With optional-chains + assert.isUndefined(simpleObj.nothing?.["something"](), "OptChain should evaluated to 'undefined'"); + assert.isUndefined(simpleObj.null?.["something"](), "OptChain should evaluated to 'undefined'"); + assert.isUndefined(simpleObj.undefined?.["something"](), "OptChain should evaluated to 'undefined'"); + } + }, + { + name: "Short-circuiting ignores indexer expression and method args", + body() { + let i = 0; + + assert.isUndefined(simpleObj.nothing?.[i++]); + assert.isUndefined(simpleObj.null?.[i++]); + assert.isUndefined(simpleObj.undefined?.[i++]); + + assert.isUndefined(simpleObj.nothing?.[i++]()); + assert.isUndefined(simpleObj.null?.[i++]()); + assert.isUndefined(simpleObj.undefined?.[i++]()); + + assert.isUndefined(simpleObj.nothing?.something(i++)); + assert.isUndefined(simpleObj.null?.something(i++)); + assert.isUndefined(simpleObj.undefined?.something(i++)); + + assert.strictEqual(0, i, "Indexer may never be evaluated"); + + // ToDo: How can I run async tests? + // simpleObj.nothing?.[await Promise.reject()]; + // simpleObj.null?.[await Promise.reject()]; + // simpleObj.undefined?.[await Promise.reject()]; + } + }, + { + name: "Short-circuiting ignores nested properties", + body() { + assert.isUndefined(simpleObj.nothing?.a.b.c.d.e.f.g.h); + assert.isUndefined(simpleObj.null?.a.b.c.d.e.f.g.h); + assert.isUndefined(simpleObj.undefined?.a.b.c.d.e.f.g.h); + } + }, + { + name: "Short-circuiting multiple levels", + body() { + let i = 0; + const specialObj = { + get null() { + i++; + return null; + }, + get undefined() { + i++; + return undefined; + } + }; + + assert.isUndefined(specialObj?.null?.a.b.c.d?.e.f.g.h); + assert.isUndefined(specialObj?.undefined?.a.b.c.d?.e.f.g.h); + + assert.areEqual(2, i, "Properties should be called") + } + }, + { + name: "Propagate 'this' correctly", + body() { + const specialObj = { + b() { return this._b; }, + _b: { c: 42 } + }; + + assert.areEqual(42, specialObj.b().c); + assert.areEqual(42, specialObj?.b().c); + assert.areEqual(42, specialObj.b?.().c); + assert.areEqual(42, specialObj?.b?.().c); + assert.areEqual(42, (specialObj?.b)().c); + assert.areEqual(42, (specialObj.b)?.().c); + assert.areEqual(42, (specialObj?.b)?.().c); + } + }, + // Null check + { + name: "Only check for 'null' and 'undefined'", + body() { + assert.areEqual(0, ""?.length, "Expected empty string length"); + } + }, + // Parsing + { + name: "Parse ternary correctly", + body() { + assert.areEqual(0.42, eval(`"this is not falsy"?.42 : 0`)); + } + }, + { + name: "Tagged Template in OptChain is illegal", + body() { + assert.throws(() => eval("simpleObj.undefined?.`template`"), SyntaxError, "No TaggedTemplate here", "Invalid tagged template in optional chain."); + assert.throws(() => eval(`simpleObj.undefined?. + \`template\``), SyntaxError, "No TaggedTemplate here", "Invalid tagged template in optional chain."); + } + }, + { + name: "No new in OptChain", + body() { + assert.throws(() => eval(` + class Test { } + new Test?.(); + `), SyntaxError, "'new' in OptChain is illegal", "Invalid optional chain in new expression."); + } + }, + { + name: "No super in OptChain", + body() { + assert.throws(() => eval(` + class Base { } + class Test extends Base { + constructor(){ + super?.(); + } + } + `), SyntaxError, "Super in OptChain is illegal", "Invalid use of the 'super' keyword"); + + assert.throws(() => eval(` + class Base { } + class Test extends Base { + constructor(){ + super(); + + super?.abc; + } + } + `), SyntaxError, "Super in OptChain is illegal", "Invalid use of the 'super' keyword"); + } + }, + { + name: "No assignment", + body() { + const a = {}; + assert.throws(() => eval(`a?.b++`), SyntaxError, "Assignment is illegal", "Invalid left-hand side in assignment."); + assert.throws(() => eval(`a?.b += 1`), SyntaxError, "Assignment is illegal", "Invalid left-hand side in assignment."); + assert.throws(() => eval(`a?.b = 5`), SyntaxError, "Assignment is illegal", "Invalid left-hand side in assignment."); + } + } +]; + +testRunner.runTests(tests, { verbose: WScript.Arguments[0] != "summary" }); diff --git a/test/es12/rlexe.xml b/test/es12/rlexe.xml new file mode 100644 index 00000000000..e91ca2eb00d --- /dev/null +++ b/test/es12/rlexe.xml @@ -0,0 +1,9 @@ + + + + + optional-chaining.js + -args summary -endargs + + + diff --git a/test/rlexedirs.xml b/test/rlexedirs.xml index 3c1782b1d19..1e6182ea517 100644 --- a/test/rlexedirs.xml +++ b/test/rlexedirs.xml @@ -272,6 +272,11 @@ es7 + + + es12 + + switchStatement From c8297eca34b40c826015d902dec42a84ed435bca Mon Sep 17 00:00:00 2001 From: Lukas Kurz Date: Sat, 4 May 2024 23:42:34 +0200 Subject: [PATCH 18/27] Fix jit `_ReuseLoc` --- lib/Runtime/ByteCode/ByteCodeEmitter.cpp | 30 +++++++++++++++++----- lib/Runtime/ByteCode/ByteCodeGenerator.cpp | 4 ++- 2 files changed, 26 insertions(+), 8 deletions(-) diff --git a/lib/Runtime/ByteCode/ByteCodeEmitter.cpp b/lib/Runtime/ByteCode/ByteCodeEmitter.cpp index 8718849aa38..cf412231945 100644 --- a/lib/Runtime/ByteCode/ByteCodeEmitter.cpp +++ b/lib/Runtime/ByteCode/ByteCodeEmitter.cpp @@ -39,7 +39,7 @@ static void EmitNullPropagation(Js::RegSlot targetObjectSlot, ByteCodeGenerator // if (targetObject == null) goto skipLabel; byteCodeGenerator->Writer()->BrReg2( Js::OpCode::BrEq_A, funcInfo->currentOptionalChainSkipLabel, - targetObjectSlot, funcInfo->nullConstantRegister + targetObjectSlot, funcInfo->undefinedConstantRegister ); } @@ -57,22 +57,38 @@ static void EmitOptionalChainWrapper(ParseNodeUni *pnodeOptChain, ByteCodeGenera Js::ByteCodeLabel skipLabel = byteCodeGenerator->Writer()->DefineLabel(); funcInfo->currentOptionalChainSkipLabel = skipLabel; - // Acquire slot for the result value - // Prefill it with `undefined` (Fallback for short-circuiting) - Js::RegSlot resultSlot = funcInfo->AcquireLoc(pnodeOptChain); - byteCodeGenerator->Writer()->Reg1(Js::OpCode::LdUndef, resultSlot); - // Copy values from wrapper to inner expression ParseNodePtr innerNode = pnodeOptChain->pnode1; innerNode->isUsed = pnodeOptChain->isUsed; - innerNode->location = pnodeOptChain->location; + innerNode->location = funcInfo->AcquireLoc(pnodeOptChain); // emit chain expression // Every `?.` node will call `EmitNullPropagation` // `EmitNullPropagation` short-circuits to `skipLabel` in case of a nullish value emitChainContent(innerNode); + Js::ByteCodeLabel doneLabel = Js::Constants::NoRegister; + if (pnodeOptChain->isUsed) + { + Assert(innerNode->isUsed); + Assert(Js::Constants::NoRegister != innerNode->location); + + // Skip short-circuiting logic + doneLabel = byteCodeGenerator->Writer()->DefineLabel(); + byteCodeGenerator->Writer()->Br(doneLabel); + } + byteCodeGenerator->Writer()->MarkLabel(skipLabel); + + if (pnodeOptChain->isUsed) + { + Assert(innerNode->isUsed); + Assert(Js::Constants::NoRegister != pnodeOptChain->location); + + // Set `undefined` on short-circuiting + byteCodeGenerator->Writer()->Reg2(Js::OpCode::Ld_A_ReuseLoc, pnodeOptChain->location, funcInfo->undefinedConstantRegister); + byteCodeGenerator->Writer()->MarkLabel(doneLabel); + } funcInfo->currentOptionalChainSkipLabel = previousSkipLabel; } diff --git a/lib/Runtime/ByteCode/ByteCodeGenerator.cpp b/lib/Runtime/ByteCode/ByteCodeGenerator.cpp index 3193503457d..41b1db3b725 100644 --- a/lib/Runtime/ByteCode/ByteCodeGenerator.cpp +++ b/lib/Runtime/ByteCode/ByteCodeGenerator.cpp @@ -4962,10 +4962,12 @@ void AssignRegisters(ParseNode *pnode, ByteCodeGenerator *byteCodeGenerator) CheckMaybeEscapedUse(pnode->AsParseNodeUni()->pnode1, byteCodeGenerator); break; case knopCoalesce: - case knopOptChain: case knopObject: byteCodeGenerator->AssignNullConstRegister(); break; + case knopOptChain: + byteCodeGenerator->AssignUndefinedConstRegister(); + break; case knopClassDecl: { FuncInfo * topFunc = byteCodeGenerator->TopFuncInfo(); From 8baa9e67babc3a07bb1ef4f766974f9be2faa349 Mon Sep 17 00:00:00 2001 From: Lukas Kurz Date: Sat, 4 May 2024 15:03:45 +0200 Subject: [PATCH 19/27] Don't use `LdMethodFld` --- lib/Runtime/ByteCode/ByteCodeEmitter.cpp | 31 ++++++++++++++---------- 1 file changed, 18 insertions(+), 13 deletions(-) diff --git a/lib/Runtime/ByteCode/ByteCodeEmitter.cpp b/lib/Runtime/ByteCode/ByteCodeEmitter.cpp index cf412231945..de92abdeb6a 100644 --- a/lib/Runtime/ByteCode/ByteCodeEmitter.cpp +++ b/lib/Runtime/ByteCode/ByteCodeEmitter.cpp @@ -7925,10 +7925,14 @@ Js::ArgSlot EmitNewObjectOfConstants( return actualArgCount; } -void EmitMethodFld(bool isRoot, bool isScoped, Js::RegSlot location, Js::RegSlot callObjLocation, Js::PropertyId propertyId, ByteCodeGenerator *byteCodeGenerator, FuncInfo *funcInfo, bool registerCacheIdForCall = true) +void EmitMethodFld(bool isRoot, bool isScoped, bool isNullPropagating, Js::RegSlot location, Js::RegSlot callObjLocation, Js::PropertyId propertyId, ByteCodeGenerator *byteCodeGenerator, FuncInfo *funcInfo, bool registerCacheIdForCall = true) { Js::OpCode opcode; - if (!isRoot) + if (isNullPropagating) + { + opcode = (!isScoped && isRoot) ? Js::OpCode::LdRootFld : Js::OpCode::LdFld; + } + else if (!isRoot) { if (callObjLocation == funcInfo->frameObjRegister) { @@ -7968,7 +7972,7 @@ void EmitMethodFld(bool isRoot, bool isScoped, Js::RegSlot location, Js::RegSlot } } -void EmitMethodFld(ParseNode *pnode, Js::RegSlot callObjLocation, Js::PropertyId propertyId, ByteCodeGenerator *byteCodeGenerator, FuncInfo *funcInfo, bool registerCacheIdForCall = true) +void EmitMethodFld(ParseNodeCall *pnodeCall, ParseNode *pnode, Js::RegSlot callObjLocation, Js::PropertyId propertyId, ByteCodeGenerator *byteCodeGenerator, FuncInfo *funcInfo, bool registerCacheIdForCall = true) { // Load a call target of the form x.y(). (Call target may be a plain knopName if we're getting it from // the global object, etc.) @@ -7976,7 +7980,7 @@ void EmitMethodFld(ParseNode *pnode, Js::RegSlot callObjLocation, Js::PropertyId bool isScoped = (byteCodeGenerator->GetFlags() & fscrEval) != 0 || (isRoot && callObjLocation != ByteCodeGenerator::RootObjectRegister); - EmitMethodFld(isRoot, isScoped, pnode->location, callObjLocation, propertyId, byteCodeGenerator, funcInfo, registerCacheIdForCall); + EmitMethodFld(isRoot, isScoped, pnodeCall->isNullPropagating, pnode->location, callObjLocation, propertyId, byteCodeGenerator, funcInfo, registerCacheIdForCall); } // lhs.apply(this, arguments); @@ -8003,7 +8007,7 @@ void EmitApplyCall(ParseNodeCall* pnodeCall, ByteCodeGenerator* byteCodeGenerato // call for apply, we won't remove the entry for "apply" cacheId from // ByteCodeWriter::callRegToLdFldCacheIndexMap, which is contrary to our assumption that we would // have removed an entry from a map upon seeing its corresponding call. - EmitMethodFld(applyNode, funcNode->location, propertyId, byteCodeGenerator, funcInfo, false /*registerCacheIdForCall*/); + EmitMethodFld(pnodeCall, applyNode, funcNode->location, propertyId, byteCodeGenerator, funcInfo, false /*registerCacheIdForCall*/); Symbol *argSym = funcInfo->GetArgumentsSymbol(); Assert(argSym && argSym->IsArguments()); @@ -8106,6 +8110,7 @@ void EmitCallTargetNoEvalComponents( } void EmitCallTarget( + ParseNodeCall *pnodeCall, ParseNode *pnodeTarget, BOOL fSideEffectArgs, Js::RegSlot *thisLocation, @@ -8130,7 +8135,7 @@ void EmitCallTarget( { case knopOptChain: { EmitOptionalChainWrapper(pnodeTarget->AsParseNodeUni(), byteCodeGenerator, funcInfo, [&](ParseNodePtr innerNode) { - EmitCallTarget(innerNode, fSideEffectArgs, thisLocation, releaseThisLocation, callObjLocation, byteCodeGenerator, funcInfo, callApplyCallSiteId); + EmitCallTarget(pnodeCall, innerNode, fSideEffectArgs, thisLocation, releaseThisLocation, callObjLocation, byteCodeGenerator, funcInfo, callApplyCallSiteId); }); break; } @@ -8179,7 +8184,7 @@ void EmitCallTarget( { *thisLocation = pnodeBinTarget->pnode1->location; EmitNullPropagation(pnodeBinTarget->pnode1->location, byteCodeGenerator, funcInfo, pnodeBinTarget->isNullPropagating); - EmitMethodFld(pnodeBinTarget, protoLocation, propertyId, byteCodeGenerator, funcInfo); + EmitMethodFld(pnodeCall, pnodeBinTarget, protoLocation, propertyId, byteCodeGenerator, funcInfo); } break; @@ -8247,7 +8252,7 @@ void EmitCallTarget( { // Load the call target as a property of the instance. Js::PropertyId propertyId = pnodeNameTarget->PropertyIdFromNameNode(); - EmitMethodFld(pnodeNameTarget, *callObjLocation, propertyId, byteCodeGenerator, funcInfo); + EmitMethodFld(pnodeCall, pnodeNameTarget, *callObjLocation, propertyId, byteCodeGenerator, funcInfo); break; } } @@ -8403,7 +8408,7 @@ void EmitCallInstrNoEvalComponents( Assert(pnodeTarget->AsParseNodeBin()->pnode2->nop == knopName); Js::PropertyId propertyId = pnodeTarget->AsParseNodeBin()->pnode2->AsParseNodeName()->PropertyIdFromNameNode(); - EmitMethodFld(pnodeTarget, callObjLocation, propertyId, byteCodeGenerator, funcInfo); + EmitMethodFld(pnodeCall, pnodeTarget, callObjLocation, propertyId, byteCodeGenerator, funcInfo); EmitCallI(pnodeCall, /*fEvaluateComponents*/ FALSE, fIsEval, fHasNewTarget, actualArgCount, byteCodeGenerator, funcInfo, callSiteId, spreadIndices); } break; @@ -8427,7 +8432,7 @@ void EmitCallInstrNoEvalComponents( funcInfo->ReleaseTmpRegister(callObjLocation); Js::PropertyId propertyId = pnodeTarget->AsParseNodeName()->PropertyIdFromNameNode(); - EmitMethodFld(pnodeTarget, callObjLocation, propertyId, byteCodeGenerator, funcInfo); + EmitMethodFld(pnodeCall, pnodeTarget, callObjLocation, propertyId, byteCodeGenerator, funcInfo); EmitCallI(pnodeCall, /*fEvaluateComponents*/ FALSE, fIsEval, fHasNewTarget, actualArgCount, byteCodeGenerator, funcInfo, callSiteId, spreadIndices); break; } @@ -8652,7 +8657,7 @@ void EmitCall( } else { - EmitCallTarget(pnodeTarget, fSideEffectArgs, &thisLocation, &releaseThisLocation, &callObjLocation, byteCodeGenerator, funcInfo, &callApplyCallSiteId); + EmitCallTarget(pnodeCall, pnodeTarget, fSideEffectArgs, &thisLocation, &releaseThisLocation, &callObjLocation, byteCodeGenerator, funcInfo, &callApplyCallSiteId); } } @@ -8704,7 +8709,7 @@ void EmitInvoke( ByteCodeGenerator* byteCodeGenerator, FuncInfo* funcInfo) { - EmitMethodFld(false, false, location, callObjLocation, propertyId, byteCodeGenerator, funcInfo); + EmitMethodFld(false, false, false, location, callObjLocation, propertyId, byteCodeGenerator, funcInfo); funcInfo->StartRecordingOutArgs(1); @@ -8724,7 +8729,7 @@ void EmitInvoke( FuncInfo* funcInfo, Js::RegSlot arg1Location) { - EmitMethodFld(false, false, location, callObjLocation, propertyId, byteCodeGenerator, funcInfo); + EmitMethodFld(false, false, false, location, callObjLocation, propertyId, byteCodeGenerator, funcInfo); funcInfo->StartRecordingOutArgs(2); From 8c4eddf118907d775f10113be9bc7f798cf6fa98 Mon Sep 17 00:00:00 2001 From: Lukas Kurz Date: Tue, 7 May 2024 16:07:54 +0200 Subject: [PATCH 20/27] Add call tests + Split into multiple files --- test/es12/optional-calls.js | 115 ++++++++++++++++++++++++++++++++ test/es12/optional-chaining.js | 118 +-------------------------------- test/es12/optional-parsing.js | 71 ++++++++++++++++++++ test/es12/rlexe.xml | 8 +++ 4 files changed, 195 insertions(+), 117 deletions(-) create mode 100644 test/es12/optional-calls.js create mode 100644 test/es12/optional-parsing.js diff --git a/test/es12/optional-calls.js b/test/es12/optional-calls.js new file mode 100644 index 00000000000..59c455496a0 --- /dev/null +++ b/test/es12/optional-calls.js @@ -0,0 +1,115 @@ +//------------------------------------------------------------------------------------------------------- +// Copyright (C) Microsoft. All rights reserved. +// Copyright (c) ChakraCore Project Contributors. All rights reserved. +// Licensed under the MIT license. See LICENSE.txt file in the project root for full license information. +//------------------------------------------------------------------------------------------------------- + +// @ts-check +/// + +WScript.LoadScriptFile("..\\UnitTestFramework\\UnitTestFramework.js"); + +const simpleObj = { "null": null, "undefined": undefined, something: 42 }; +Object.freeze(simpleObj); + +const tests = [ + { + name: "Simple method call on property", + body() { + // Verify normal behavior + assert.throws(() => simpleObj.nothing(), TypeError); + assert.throws(() => simpleObj.null(), TypeError); + assert.throws(() => simpleObj.undefined(), TypeError); + + // With optional-chains + assert.isUndefined(simpleObj.nothing?.(), "OptChain should evaluated to 'undefined'"); + assert.isUndefined(simpleObj.null?.(), "OptChain should evaluated to 'undefined'"); + assert.isUndefined(simpleObj.undefined?.(), "OptChain should evaluated to 'undefined'"); + } + }, + { + name: "Simple method call on indexer", + body() { + // Verify normal behavior + assert.throws(() => simpleObj["nothing"](), TypeError); + assert.throws(() => simpleObj["null"](), TypeError); + assert.throws(() => simpleObj["undefined"](), TypeError); + + // With optional-chains + assert.isUndefined(simpleObj["nothing"]?.(), "OptChain should evaluated to 'undefined'"); + assert.isUndefined(simpleObj["null"]?.(), "OptChain should evaluated to 'undefined'"); + assert.isUndefined(simpleObj["undefined"]?.(), "OptChain should evaluated to 'undefined'"); + } + }, + { + name: "Simple method call on non-callable property", + body() { + // Verify normal behavior + assert.throws(() => simpleObj.something(), TypeError, "Non-callable prop", "Function expected"); + + // With optional-chains + assert.throws(() => simpleObj.something?.(), TypeError, "Non-callable prop", "Function expected"); + assert.throws(() => simpleObj?.something(), TypeError, "Non-callable prop", "Function expected"); + assert.throws(() => simpleObj?.something?.(), TypeError, "Non-callable prop", "Function expected"); + } + }, + { + name: "Simple method call on non-callable indexer", + body() { + // Verify normal behavior + assert.throws(() => simpleObj["something"](), TypeError, "Non-callable prop", "Function expected"); + + // With optional-chains + assert.throws(() => simpleObj["something"]?.(), TypeError, "Non-callable prop", "Function expected"); + assert.throws(() => simpleObj?.["something"](), TypeError, "Non-callable prop", "Function expected"); + assert.throws(() => simpleObj?.["something"]?.(), TypeError, "Non-callable prop", "Function expected"); + } + }, + { + name: "Optional properties before call", + body() { + // Verify normal behavior + assert.throws(() => simpleObj.nothing.something(), TypeError); + assert.throws(() => simpleObj.null.something(), TypeError); + assert.throws(() => simpleObj.undefined.something(), TypeError); + + // With optional-chains + assert.isUndefined(simpleObj.nothing?.something(), "OptChain should evaluated to 'undefined'"); + assert.isUndefined(simpleObj.null?.something(), "OptChain should evaluated to 'undefined'"); + assert.isUndefined(simpleObj.undefined?.something(), "OptChain should evaluated to 'undefined'"); + } + }, + { + name: "Optional indexers before call", + body() { + // Verify normal behavior + assert.throws(() => simpleObj.nothing["something"](), TypeError); + assert.throws(() => simpleObj.null["something"](), TypeError); + assert.throws(() => simpleObj.undefined["something"](), TypeError); + + // With optional-chains + assert.isUndefined(simpleObj.nothing?.["something"](), "OptChain should evaluated to 'undefined'"); + assert.isUndefined(simpleObj.null?.["something"](), "OptChain should evaluated to 'undefined'"); + assert.isUndefined(simpleObj.undefined?.["something"](), "OptChain should evaluated to 'undefined'"); + } + }, + { + name: "Propagate 'this' correctly", + body() { + const specialObj = { + b() { return this._b; }, + _b: { c: 42 } + }; + + assert.areEqual(42, specialObj.b().c); + assert.areEqual(42, specialObj?.b().c); + assert.areEqual(42, specialObj.b?.().c); + assert.areEqual(42, specialObj?.b?.().c); + assert.areEqual(42, (specialObj?.b)().c); + assert.areEqual(42, (specialObj.b)?.().c); + assert.areEqual(42, (specialObj?.b)?.().c); + } + } +]; + +testRunner.runTests(tests, { verbose: WScript.Arguments[0] != "summary" }); diff --git a/test/es12/optional-chaining.js b/test/es12/optional-chaining.js index a90fca6dd33..944d416baaf 100644 --- a/test/es12/optional-chaining.js +++ b/test/es12/optional-chaining.js @@ -9,7 +9,7 @@ WScript.LoadScriptFile("..\\UnitTestFramework\\UnitTestFramework.js"); -const simpleObj = { "null": null, "undefined": undefined }; +const simpleObj = { "null": null, "undefined": undefined, something: 42 }; Object.freeze(simpleObj); const tests = [ @@ -41,48 +41,6 @@ const tests = [ assert.isUndefined(simpleObj.undefined?.["something"], "OptChain should evaluated to 'undefined'"); } }, - { - name: "Simple method calls", - body() { - // Verify normal behavior - assert.throws(() => simpleObj.nothing(), TypeError); - assert.throws(() => simpleObj.null(), TypeError); - assert.throws(() => simpleObj.undefined(), TypeError); - - // With optional-chains - assert.isUndefined(simpleObj.nothing?.(), "OptChain should evaluated to 'undefined'"); - assert.isUndefined(simpleObj.null?.(), "OptChain should evaluated to 'undefined'"); - assert.isUndefined(simpleObj.undefined?.(), "OptChain should evaluated to 'undefined'"); - } - }, - { - name: "Simple properties before call", - body() { - // Verify normal behavior - assert.throws(() => simpleObj.nothing.something(), TypeError); - assert.throws(() => simpleObj.null.something(), TypeError); - assert.throws(() => simpleObj.undefined.something(), TypeError); - - // With optional-chains - assert.isUndefined(simpleObj.nothing?.something(), "OptChain should evaluated to 'undefined'"); - assert.isUndefined(simpleObj.null?.something(), "OptChain should evaluated to 'undefined'"); - assert.isUndefined(simpleObj.undefined?.something(), "OptChain should evaluated to 'undefined'"); - } - }, - { - name: "Simple indexers before call", - body() { - // Verify normal behavior - assert.throws(() => simpleObj.nothing["something"], TypeError); - assert.throws(() => simpleObj.null["something"], TypeError); - assert.throws(() => simpleObj.undefined["something"], TypeError); - - // With optional-chains - assert.isUndefined(simpleObj.nothing?.["something"](), "OptChain should evaluated to 'undefined'"); - assert.isUndefined(simpleObj.null?.["something"](), "OptChain should evaluated to 'undefined'"); - assert.isUndefined(simpleObj.undefined?.["something"](), "OptChain should evaluated to 'undefined'"); - } - }, { name: "Short-circuiting ignores indexer expression and method args", body() { @@ -137,86 +95,12 @@ const tests = [ assert.areEqual(2, i, "Properties should be called") } }, - { - name: "Propagate 'this' correctly", - body() { - const specialObj = { - b() { return this._b; }, - _b: { c: 42 } - }; - - assert.areEqual(42, specialObj.b().c); - assert.areEqual(42, specialObj?.b().c); - assert.areEqual(42, specialObj.b?.().c); - assert.areEqual(42, specialObj?.b?.().c); - assert.areEqual(42, (specialObj?.b)().c); - assert.areEqual(42, (specialObj.b)?.().c); - assert.areEqual(42, (specialObj?.b)?.().c); - } - }, // Null check { name: "Only check for 'null' and 'undefined'", body() { assert.areEqual(0, ""?.length, "Expected empty string length"); } - }, - // Parsing - { - name: "Parse ternary correctly", - body() { - assert.areEqual(0.42, eval(`"this is not falsy"?.42 : 0`)); - } - }, - { - name: "Tagged Template in OptChain is illegal", - body() { - assert.throws(() => eval("simpleObj.undefined?.`template`"), SyntaxError, "No TaggedTemplate here", "Invalid tagged template in optional chain."); - assert.throws(() => eval(`simpleObj.undefined?. - \`template\``), SyntaxError, "No TaggedTemplate here", "Invalid tagged template in optional chain."); - } - }, - { - name: "No new in OptChain", - body() { - assert.throws(() => eval(` - class Test { } - new Test?.(); - `), SyntaxError, "'new' in OptChain is illegal", "Invalid optional chain in new expression."); - } - }, - { - name: "No super in OptChain", - body() { - assert.throws(() => eval(` - class Base { } - class Test extends Base { - constructor(){ - super?.(); - } - } - `), SyntaxError, "Super in OptChain is illegal", "Invalid use of the 'super' keyword"); - - assert.throws(() => eval(` - class Base { } - class Test extends Base { - constructor(){ - super(); - - super?.abc; - } - } - `), SyntaxError, "Super in OptChain is illegal", "Invalid use of the 'super' keyword"); - } - }, - { - name: "No assignment", - body() { - const a = {}; - assert.throws(() => eval(`a?.b++`), SyntaxError, "Assignment is illegal", "Invalid left-hand side in assignment."); - assert.throws(() => eval(`a?.b += 1`), SyntaxError, "Assignment is illegal", "Invalid left-hand side in assignment."); - assert.throws(() => eval(`a?.b = 5`), SyntaxError, "Assignment is illegal", "Invalid left-hand side in assignment."); - } } ]; diff --git a/test/es12/optional-parsing.js b/test/es12/optional-parsing.js new file mode 100644 index 00000000000..d6803f7867a --- /dev/null +++ b/test/es12/optional-parsing.js @@ -0,0 +1,71 @@ +//------------------------------------------------------------------------------------------------------- +// Copyright (C) Microsoft. All rights reserved. +// Copyright (c) ChakraCore Project Contributors. All rights reserved. +// Licensed under the MIT license. See LICENSE.txt file in the project root for full license information. +//------------------------------------------------------------------------------------------------------- + +// @ts-check +/// + +WScript.LoadScriptFile("..\\UnitTestFramework\\UnitTestFramework.js"); + +const tests = [ + { + name: "Parse ternary correctly", + body() { + assert.areEqual(0.42, eval(`"this is not falsy"?.42 : 0`)); + } + }, + { + name: "Tagged Template in OptChain is illegal", + body() { + assert.throws(() => eval("simpleObj.undefined?.`template`"), SyntaxError, "No TaggedTemplate here", "Invalid tagged template in optional chain."); + assert.throws(() => eval(`simpleObj.undefined?. + \`template\``), SyntaxError, "No TaggedTemplate here", "Invalid tagged template in optional chain."); + } + }, + { + name: "No new in OptChain", + body() { + assert.throws(() => eval(` + class Test { } + new Test?.(); + `), SyntaxError, "'new' in OptChain is illegal", "Invalid optional chain in new expression."); + } + }, + { + name: "No super in OptChain", + body() { + assert.throws(() => eval(` + class Base { } + class Test extends Base { + constructor(){ + super?.(); + } + } + `), SyntaxError, "Super in OptChain is illegal", "Invalid use of the 'super' keyword"); + + assert.throws(() => eval(` + class Base { } + class Test extends Base { + constructor(){ + super(); + + super?.abc; + } + } + `), SyntaxError, "Super in OptChain is illegal", "Invalid use of the 'super' keyword"); + } + }, + { + name: "No assignment", + body() { + const a = {}; + assert.throws(() => eval(`a?.b++`), SyntaxError, "Assignment is illegal", "Invalid left-hand side in assignment."); + assert.throws(() => eval(`a?.b += 1`), SyntaxError, "Assignment is illegal", "Invalid left-hand side in assignment."); + assert.throws(() => eval(`a?.b = 5`), SyntaxError, "Assignment is illegal", "Invalid left-hand side in assignment."); + } + } +]; + +testRunner.runTests(tests, { verbose: WScript.Arguments[0] != "summary" }); diff --git a/test/es12/rlexe.xml b/test/es12/rlexe.xml index e91ca2eb00d..e32791310a6 100644 --- a/test/es12/rlexe.xml +++ b/test/es12/rlexe.xml @@ -5,5 +5,13 @@ optional-chaining.js -args summary -endargs + + optional-parsing.js + -args summary -endargs + + + optional-calls.js + -args summary -endargs + From f48dc1bffb14aa4f2f241dfafdf14b1d1ce9464e Mon Sep 17 00:00:00 2001 From: Lukas Kurz Date: Fri, 10 May 2024 23:09:30 +0200 Subject: [PATCH 21/27] Treat `eval?.()` as indirect `eval` --- lib/Parser/Parse.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/Parser/Parse.cpp b/lib/Parser/Parse.cpp index e777fbe22fb..255f7597e1e 100644 --- a/lib/Parser/Parse.cpp +++ b/lib/Parser/Parse.cpp @@ -4001,7 +4001,7 @@ ParseNodePtr Parser::ParsePostfixOperators( // Note: we used to leave it up to the byte code generator to detect eval calls // at global scope, but now it relies on the flag the parser sets, so set it here. - if (count > 0 && this->NodeIsEvalName(pnode->AsParseNodeCall()->pnodeTarget)) + if (count > 0 && this->NodeIsEvalName(pnode->AsParseNodeCall()->pnodeTarget) && !isNullPropagating) { this->MarkEvalCaller(); fCallIsEval = true; From 05790aebcb3de6422d97fb880a1f46fee3e43a44 Mon Sep 17 00:00:00 2001 From: Lukas Kurz Date: Tue, 28 May 2024 14:32:47 +0200 Subject: [PATCH 22/27] Started optional-deletion --- lib/Runtime/ByteCode/ByteCodeEmitter.cpp | 137 +++++++++++++---------- 1 file changed, 79 insertions(+), 58 deletions(-) diff --git a/lib/Runtime/ByteCode/ByteCodeEmitter.cpp b/lib/Runtime/ByteCode/ByteCodeEmitter.cpp index de92abdeb6a..fe9f1cf2731 100644 --- a/lib/Runtime/ByteCode/ByteCodeEmitter.cpp +++ b/lib/Runtime/ByteCode/ByteCodeEmitter.cpp @@ -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; // 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; Js::ByteCodeLabel doneLabel = Js::Constants::NoRegister; if (pnodeOptChain->isUsed) @@ -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; } @@ -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; + 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; + } + } +} + void Emit(ParseNode* pnode, ByteCodeGenerator* byteCodeGenerator, FuncInfo* funcInfo, BOOL fReturnValue, bool isConstructorCall, bool isTopLevel) { if (pnode == nullptr) @@ -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; } From a18431e23aec86d8dd739a07b463c02ddb56dd67 Mon Sep 17 00:00:00 2001 From: Lukas Kurz Date: Sat, 6 Jul 2024 14:48:54 +0200 Subject: [PATCH 23/27] Test optional-call in eval --- test/es12/optional-calls.js | 45 +++++++++++++++++++++++++++++++++++++ 1 file changed, 45 insertions(+) diff --git a/test/es12/optional-calls.js b/test/es12/optional-calls.js index 59c455496a0..71a8a062925 100644 --- a/test/es12/optional-calls.js +++ b/test/es12/optional-calls.js @@ -109,6 +109,51 @@ const tests = [ assert.areEqual(42, (specialObj.b)?.().c); assert.areEqual(42, (specialObj?.b)?.().c); } + }, + { + name: "Optional call in eval (function)", + body() { + function fn() { + return 42; + } + assert.areEqual(42, eval("fn?.()")); + }, + }, + { + name: "Optional call in eval (lambda)", + body() { + const fn = () => 42; + assert.areEqual(42, eval("fn?.()")); + }, + }, + { + name: "Optional call in eval (object)", + body() { + const obj = { + fn: () => 42, + }; + assert.areEqual(42, eval("obj?.fn?.()")); + }, + }, + { + name: "Optional call in eval (undefined)", + body() { + assert.areEqual(undefined, eval("doesNotExist?.()")); + }, + }, + { + name: "Optional call in eval respects scope", + body() { + function fn() { + return 42; + } + assert.areEqual(24, eval(` + function fn(){ + return 24; + } + fn?.() + `)); + }, } ]; From d96b76b39cb13c27ad9ff7db8aa36ed214a4a3b1 Mon Sep 17 00:00:00 2001 From: Lukas Kurz Date: Sat, 6 Jul 2024 14:49:36 +0200 Subject: [PATCH 24/27] Ensure `isUsed` is set if `MustProduceValue` --- lib/Runtime/ByteCode/ByteCodeGenerator.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/lib/Runtime/ByteCode/ByteCodeGenerator.cpp b/lib/Runtime/ByteCode/ByteCodeGenerator.cpp index 41b1db3b725..dc0d9a2d020 100644 --- a/lib/Runtime/ByteCode/ByteCodeGenerator.cpp +++ b/lib/Runtime/ByteCode/ByteCodeGenerator.cpp @@ -1224,6 +1224,10 @@ ParseNode* VisitBlock(ParseNode *pnode, ByteCodeGenerator* byteCodeGenerator, Pr } } } + if (nullptr != pnodeLastVal) + { + pnodeLastVal->isUsed = true; + } return pnodeLastVal; } From 0abf9c29cdd9919182924852402cc18ce01c1d38 Mon Sep 17 00:00:00 2001 From: Lukas Kurz Date: Sat, 6 Jul 2024 14:55:43 +0200 Subject: [PATCH 25/27] Copy `isUsed` for robustness --- lib/Runtime/ByteCode/ByteCodeEmitter.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/Runtime/ByteCode/ByteCodeEmitter.cpp b/lib/Runtime/ByteCode/ByteCodeEmitter.cpp index fbf58284f85..45de2812a4e 100644 --- a/lib/Runtime/ByteCode/ByteCodeEmitter.cpp +++ b/lib/Runtime/ByteCode/ByteCodeEmitter.cpp @@ -67,6 +67,7 @@ static void EmitOptionalChainWrapper(ParseNodeUni *pnodeOptChain, ByteCodeGenera // `EmitNullPropagation` short-circuits to `skipLabel` in case of a nullish value emitChainContent(innerNode); pnodeOptChain->location = innerNode->location; + pnodeOptChain->isUsed = innerNode->isUsed; Js::ByteCodeLabel doneLabel = Js::Constants::NoRegister; if (pnodeOptChain->isUsed) From a2709e72083d5b94f28610fd98e56681179cbbdb Mon Sep 17 00:00:00 2001 From: Lukas Kurz Date: Sat, 6 Jul 2024 14:56:10 +0200 Subject: [PATCH 26/27] Test for opt-call of root function --- test/es12/optional-calls.js | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/test/es12/optional-calls.js b/test/es12/optional-calls.js index 71a8a062925..4b62e71b8c1 100644 --- a/test/es12/optional-calls.js +++ b/test/es12/optional-calls.js @@ -110,6 +110,16 @@ const tests = [ assert.areEqual(42, (specialObj?.b)?.().c); } }, + { + name: "Optional call of root function", + body(){ + assert.areEqual(42, eval?.("42")); + + globalThis.doNotUseThisBadGlobalFunction = () => 42; + assert.areEqual(42, doNotUseThisBadGlobalFunction?.()); + assert.areEqual(42, eval("doNotUseThisBadGlobalFunction?.()")); + } + }, { name: "Optional call in eval (function)", body() { From 8210f6277f51fdc4af5454c1fed367a756e8a095 Mon Sep 17 00:00:00 2001 From: Lukas Kurz Date: Sat, 6 Jul 2024 15:04:03 +0200 Subject: [PATCH 27/27] Async tests --- test/es12/optional-async.js | 22 ++++++++++++++++++++++ test/es12/optional-chaining.js | 5 ----- test/es12/rlexe.xml | 4 ++++ 3 files changed, 26 insertions(+), 5 deletions(-) create mode 100644 test/es12/optional-async.js diff --git a/test/es12/optional-async.js b/test/es12/optional-async.js new file mode 100644 index 00000000000..f4a11aabadb --- /dev/null +++ b/test/es12/optional-async.js @@ -0,0 +1,22 @@ +//------------------------------------------------------------------------------------------------------- +// Copyright (C) Microsoft. All rights reserved. +// Copyright (c) ChakraCore Project Contributors. All rights reserved. +// Licensed under the MIT license. See LICENSE.txt file in the project root for full license information. +//------------------------------------------------------------------------------------------------------- + +// @ts-check + +const simpleObj = { null: null, undefined: undefined, something: 42 }; +Object.freeze(simpleObj); + +// Short-circuiting ignores indexer expression and method args +(async () => { + simpleObj.nothing?.[await Promise.reject()]; + simpleObj.null?.[await Promise.reject()]; + simpleObj.undefined?.[await Promise.reject()]; +})().then( + () => { + console.log("pass"); + }, + (x) => console.log(x) +); diff --git a/test/es12/optional-chaining.js b/test/es12/optional-chaining.js index 944d416baaf..defdbb7538f 100644 --- a/test/es12/optional-chaining.js +++ b/test/es12/optional-chaining.js @@ -59,11 +59,6 @@ const tests = [ assert.isUndefined(simpleObj.undefined?.something(i++)); assert.strictEqual(0, i, "Indexer may never be evaluated"); - - // ToDo: How can I run async tests? - // simpleObj.nothing?.[await Promise.reject()]; - // simpleObj.null?.[await Promise.reject()]; - // simpleObj.undefined?.[await Promise.reject()]; } }, { diff --git a/test/es12/rlexe.xml b/test/es12/rlexe.xml index e32791310a6..88fbcfa1230 100644 --- a/test/es12/rlexe.xml +++ b/test/es12/rlexe.xml @@ -13,5 +13,9 @@ optional-calls.js -args summary -endargs + + optional-async.js + -args summary -endargs +