diff --git a/include/hermes/Sema/SemContext.h b/include/hermes/Sema/SemContext.h index cdc57d92630..62affaf1684 100644 --- a/include/hermes/Sema/SemContext.h +++ b/include/hermes/Sema/SemContext.h @@ -59,6 +59,9 @@ class Decl { Const, Class, Import, + /// A catch variable bound with let-like rules (non-ES5). + /// ES14.0 B.3.4 handles ES5-style catch bindings differently. + Catch, /// Function declaration visible only in its lexical scope. ScopedFunction, /// A single catch variable declared like this "catch (e)", see @@ -105,7 +108,7 @@ class Decl { return kind >= Kind::Var; } static bool isKindVarLikeOrScopedFunction(Kind kind) { - return kind >= Kind::ScopedFunction; + return isKindVarLike(kind) || kind == Kind::ScopedFunction; } /// \return true if this kind of declaration is lexically scoped (and cannot /// be re-declared). diff --git a/lib/IRGen/ESTreeIRGen-func.cpp b/lib/IRGen/ESTreeIRGen-func.cpp index 685023679a2..a4ae42171cf 100644 --- a/lib/IRGen/ESTreeIRGen-func.cpp +++ b/lib/IRGen/ESTreeIRGen-func.cpp @@ -762,6 +762,7 @@ void ESTreeIRGen::emitScopeDeclarations(sema::LexicalScope *scope) { case sema::Decl::Kind::Let: case sema::Decl::Kind::Const: case sema::Decl::Kind::Class: + case sema::Decl::Kind::Catch: assert( ((curFunction()->debugAllowRecompileCounter != 0) || (decl->customData == nullptr)) && diff --git a/lib/Sema/DeclCollector.cpp b/lib/Sema/DeclCollector.cpp index fc727ff4c4e..3543f47e69b 100644 --- a/lib/Sema/DeclCollector.cpp +++ b/lib/Sema/DeclCollector.cpp @@ -172,9 +172,9 @@ void DeclCollector::visit(ESTree::CatchClauseNode *node) { addToCur(node); visitESTreeNode(*this, node->_param, node); } - // Declarations in the catch body are associated with CatchClauseNode, - // not BlockStatementNode. - visitESTreeChildren(*this, node->_body); + // CatchClauseNode is supposed to have separate scopes for the body and the + // parameters. + visitESTreeNode(*this, node->_body, node); closeScope(node); } void DeclCollector::visit(ESTree::SwitchStatementNode *node) { diff --git a/lib/Sema/FlowChecker.cpp b/lib/Sema/FlowChecker.cpp index a9b75679f0d..f4c086b7d06 100644 --- a/lib/Sema/FlowChecker.cpp +++ b/lib/Sema/FlowChecker.cpp @@ -879,9 +879,7 @@ void FlowChecker::visit(ESTree::CatchClauseNode *node) { ScopeRAII scope(*this); if (!resolveScopeTypesAndAnnotate(node, node->getScope())) return; - visitESTreeNode(*this, node->_param, node); - // Process body's declarations, skip visiting it, visit its children. - visitESTreeChildren(*this, node->_body); + visitESTreeChildren(*this, node); } void FlowChecker::visitFunctionLike( diff --git a/lib/Sema/ScopedFunctionPromoter.cpp b/lib/Sema/ScopedFunctionPromoter.cpp index 129ef276275..08f35f2022f 100644 --- a/lib/Sema/ScopedFunctionPromoter.cpp +++ b/lib/Sema/ScopedFunctionPromoter.cpp @@ -63,6 +63,9 @@ class ScopedFunctionPromoter { void visit(WithStatementNode *node) { visitScope(node); } + void visit(CatchClauseNode *node) { + visitScope(node); + } /// Needed by RecursiveVisitorDispatch. Optionally can protect against too /// deep nesting. @@ -196,8 +199,10 @@ void ScopedFunctionPromoter::processDeclarations(Node *scope) { idents.clear(); Decl::Kind declKind = extractDeclaredIdents(node, idents); - // We are only interested in let-like declarations. - if (!Decl::isKindLetLike(declKind)) + // We are only interested in let-like declarations, but not ES5Catch. + // ES5Catch doesn't conflict with Var declarations. + // See ES14.0 B.3.4. + if (!Decl::isKindLetLike(declKind) || declKind == Decl::Kind::ES5Catch) continue; // Remember only idents matching the set. @@ -257,6 +262,16 @@ Decl::Kind ScopedFunctionPromoter::extractDeclaredIdents( return Decl::Kind::Class; } + if (auto *catchClause = llvh::dyn_cast(node)) { + resolver_.extractDeclaredIdentsFromID(catchClause->_param, idents); + if (auto *id = + llvh::dyn_cast_or_null(catchClause->_param)) { + return Decl::Kind::ES5Catch; + } else { + return Decl::Kind::Catch; + } + } + { auto *id = llvh::cast(node); for (Node &spec : id->_specifiers) { diff --git a/lib/Sema/SemContext.cpp b/lib/Sema/SemContext.cpp index 05b1a6bf1b8..decf000647a 100644 --- a/lib/Sema/SemContext.cpp +++ b/lib/Sema/SemContext.cpp @@ -482,6 +482,7 @@ void SemContextDumper::printDecl(llvh::raw_ostream &os, const Decl *d) { CASE(Let) CASE(Const) CASE(Class) + CASE(Catch) CASE(Import) CASE(ES5Catch) CASE(FunctionExprName) diff --git a/lib/Sema/SemanticResolver.cpp b/lib/Sema/SemanticResolver.cpp index 9b7ffe4c23c..632c088e4f2 100644 --- a/lib/Sema/SemanticResolver.cpp +++ b/lib/Sema/SemanticResolver.cpp @@ -440,8 +440,7 @@ void SemanticResolver::visit( // Some nodes with attached BlockStatement have already dealt with the scope. if (llvh::isa(parent) || llvh::isa(parent) || - llvh::isa(parent) || - llvh::isa(parent)) { + llvh::isa(parent)) { return visitESTreeChildren(*this, node); } @@ -747,16 +746,10 @@ void SemanticResolver::visit(ESTree::TryStatementNode *tryStatement) { void SemanticResolver::visit(ESTree::CatchClauseNode *node) { ScopeRAII scope{*this, node}; - // Process body's declarations, skip visiting the block, visit its children. + // Process catch clause's declarations (not the ones in the body). processCollectedDeclarations(node); - // Visit the catch param, in case there is destructuring. - visitESTreeNode(*this, node->_param, node); - // The scope declarations are associated with the CatchClauseNode in the - // DeclCollector. - visitESTreeChildren(*this, node->_body); - assert( - !functionContext()->decls->getScopeDeclsForNode(node->_body) && - "CatchClause body block shouldn't have any decls associated"); + // Visit the body, which will make a new scope. + visitESTreeChildren(*this, node); } void SemanticResolver::visit(RegExpLiteralNode *regexp) { @@ -1563,7 +1556,8 @@ void SemanticResolver::processPromotedFuncDecls( for (FunctionDeclarationNode *funcDecl : promotedFuncDecls) { auto *ident = llvh::cast(funcDecl->_id); validateAndDeclareIdentifier(kind, ident); - functionContext()->promotedFuncDecls.insert(ident->_name); + functionContext()->promotedFuncDecls.try_emplace( + ident->_name, semCtx_.getDeclarationDecl(ident)); } } @@ -1635,7 +1629,7 @@ Decl::Kind SemanticResolver::extractIdentsFromDecl( // https://www.ecma-international.org/ecma-262/10.0/index.html#sec-variablestatements-in-catch-blocks return Decl::Kind::ES5Catch; } else { - return Decl::Kind::Let; + return Decl::Kind::Catch; } } @@ -1753,6 +1747,7 @@ void SemanticResolver::validateAndDeclareIdentifier( const bool sameScope = prevName.decl->scope == curScope_; const bool topLevel = curScope_->parentFunction->getFunctionBodyScope() == curScope_; + const bool prevInPrevScope = prevName.decl->scope == curScope_->parentScope; // Check whether the redeclaration is invalid. // Note that since "var" declarations have been hoisted to the function @@ -1771,6 +1766,19 @@ void SemanticResolver::validateAndDeclareIdentifier( // FormalParameters also occurs in the LexicallyDeclaredNames of // FunctionBody. // + // Catch (non-ES5) clause variables must not conflict with the lexically + // scoped names or var-declared names in their block: + // * It is a Syntax Error if BoundNames of CatchParameter contains any + // duplicate elements. + // * It is a Syntax Error if any element of the BoundNames of CatchParameter + // also occurs in the LexicallyDeclaredNames of Block. + // NOTE: It's possible that a function in the body of the catch has been + // promoted to a Var at function scope, so it has to be accounted for. + // * It is a Syntax Error if any element of the BoundNames of CatchParameter + // also occurs in the VarDeclaredNames of Block unless CatchParameter is + // CatchParameter : BindingIdentifier. + // visit(VariableDeclarationNode *) will handle this final case. + // // Case by case explanations for our representation: // // ES5Catch, var @@ -1813,7 +1821,13 @@ void SemanticResolver::validateAndDeclareIdentifier( prevKind == Decl::Kind::ScopedFunction && kind == Decl::Kind::ScopedFunction)) || (prevKind == Decl::Kind::Parameter && Decl::isKindLetLike(kind) && - topLevel)) { + topLevel) || + // LexicallyDeclaredNames of CatchBlock are only in the block scope + // itself, so check prevInPrevScope (it's like checking topLevel for + // parameters). + // This is an error regardless of if it's an ES5 or ES6 catch. + ((prevKind == Decl::Kind::Catch || prevKind == Decl::Kind::ES5Catch) && + Decl::isKindLetLike(kind) && prevInPrevScope)) { sm_.error( ident->getSourceRange(), llvh::Twine("Identifier '") + ident->_name->str() + @@ -1832,15 +1846,32 @@ void SemanticResolver::validateAndDeclareIdentifier( // Var, ScopedFunc -> if non-param non-strict or same scope, then use prev, // else declare new else if ( - Decl::isKindVarLike(prevKind) && - Decl::isKindVarLikeOrScopedFunction(kind)) { + Decl::isKindVarLike(prevKind) && kind == Decl::Kind::ScopedFunction) { + decl = nullptr; if (sameScope) { decl = prevName.decl; - } else if (functionContext()->promotedFuncDecls.count(ident->_name)) { + } else { + auto it = functionContext()->promotedFuncDecls.find(ident->_name); + if (it != functionContext()->promotedFuncDecls.end()) { + // We've already promoted this function, so add a new binding + // and point it to the original Decl. + reuseDeclForNewBinding = true; + decl = it->second; + } + } + } + // ES5Catch, ScopedFunc -> + // if promoted, use promoted function, else declare new + // ES5Catch doesn't prevent promotion, so we have to check it specially. + else if ( + prevKind == Decl::Kind::ES5Catch && + kind == Decl::Kind::ScopedFunction) { + auto it = functionContext()->promotedFuncDecls.find(ident->_name); + if (it != functionContext()->promotedFuncDecls.end()) { // We've already promoted this function, so add a new binding // and point it to the original Decl. reuseDeclForNewBinding = true; - decl = prevName.decl; + decl = it->second; } else { decl = nullptr; } diff --git a/lib/Sema/SemanticResolver.h b/lib/Sema/SemanticResolver.h index ff11a369c48..fe69e037364 100644 --- a/lib/Sema/SemanticResolver.h +++ b/lib/Sema/SemanticResolver.h @@ -492,9 +492,10 @@ class FunctionContext { /// All declarations in the function. std::unique_ptr decls; - /// The set of names that have been promoted to function scope by - /// promoteScopedFunctionDecls in this function. - llvh::DenseSet promotedFuncDecls{}; + /// The map of names that have been promoted to function scope by + /// promoteScopedFunctionDecls in this function, mapped to their Var + /// declaration in function scope. + llvh::DenseMap promotedFuncDecls{}; /// The depth of the function's scope in the binding table. /// Populated when ScopeRAII is created within the function. diff --git a/test/Sema/catch-block-destr.js b/test/Sema/catch-block-destr.js index 00a156831fb..dbbee281fff 100644 --- a/test/Sema/catch-block-destr.js +++ b/test/Sema/catch-block-destr.js @@ -16,9 +16,10 @@ try {} catch ([a, b]) { let x; } // CHECK-NEXT: Scope %s.1 // CHECK-NEXT: Scope %s.2 // CHECK-NEXT: Scope %s.3 -// CHECK-NEXT: Decl %d.1 'a' Let -// CHECK-NEXT: Decl %d.2 'b' Let -// CHECK-NEXT: Decl %d.3 'x' Let +// CHECK-NEXT: Decl %d.1 'a' Catch +// CHECK-NEXT: Decl %d.2 'b' Catch +// CHECK-NEXT: Scope %s.4 +// CHECK-NEXT: Decl %d.3 'x' Let // CHECK:Program Scope %s.1 // CHECK-NEXT: TryStatement @@ -27,7 +28,7 @@ try {} catch ([a, b]) { let x; } // CHECK-NEXT: ArrayPattern // CHECK-NEXT: Id 'a' [D:E:%d.1 'a'] // CHECK-NEXT: Id 'b' [D:E:%d.2 'b'] -// CHECK-NEXT: BlockStatement +// CHECK-NEXT: BlockStatement Scope %s.4 // CHECK-NEXT: VariableDeclaration // CHECK-NEXT: VariableDeclarator // CHECK-NEXT: Id 'x' [D:E:%d.3 'x'] diff --git a/test/Sema/catch-block.js b/test/Sema/catch-block.js index 74afa681c9f..4c5d089b202 100644 --- a/test/Sema/catch-block.js +++ b/test/Sema/catch-block.js @@ -17,14 +17,15 @@ try {} catch (e) { let x; } // CHECK-NEXT: Scope %s.2 // CHECK-NEXT: Scope %s.3 // CHECK-NEXT: Decl %d.1 'e' ES5Catch -// CHECK-NEXT: Decl %d.2 'x' Let +// CHECK-NEXT: Scope %s.4 +// CHECK-NEXT: Decl %d.2 'x' Let // CHECK:Program Scope %s.1 // CHECK-NEXT: TryStatement // CHECK-NEXT: BlockStatement Scope %s.2 // CHECK-NEXT: CatchClause Scope %s.3 // CHECK-NEXT: Id 'e' [D:E:%d.1 'e'] -// CHECK-NEXT: BlockStatement +// CHECK-NEXT: BlockStatement Scope %s.4 // CHECK-NEXT: VariableDeclaration // CHECK-NEXT: VariableDeclarator // CHECK-NEXT: Id 'x' [D:E:%d.2 'x'] diff --git a/test/Sema/flow/catch-block.js b/test/Sema/flow/catch-block.js index 1be1aef7bab..85e49c25623 100644 --- a/test/Sema/flow/catch-block.js +++ b/test/Sema/flow/catch-block.js @@ -25,7 +25,8 @@ try {} catch (e) { let x: number = 1; } // CHECK-NEXT: Scope %s.3 // CHECK-NEXT: Scope %s.4 // CHECK-NEXT: Decl %d.3 'e' ES5Catch : any -// CHECK-NEXT: Decl %d.4 'x' Let : number +// CHECK-NEXT: Scope %s.5 +// CHECK-NEXT: Decl %d.4 'x' Let : number // CHECK:Program Scope %s.1 // CHECK-NEXT: ExpressionStatement @@ -37,7 +38,7 @@ try {} catch (e) { let x: number = 1; } // CHECK-NEXT: BlockStatement Scope %s.3 // CHECK-NEXT: CatchClause Scope %s.4 // CHECK-NEXT: Id 'e' [D:E:%d.3 'e'] -// CHECK-NEXT: BlockStatement +// CHECK-NEXT: BlockStatement Scope %s.5 // CHECK-NEXT: VariableDeclaration // CHECK-NEXT: VariableDeclarator // CHECK-NEXT: NumericLiteral : number diff --git a/test/Sema/flow/implicit-return.js b/test/Sema/flow/implicit-return.js index 570b9f84600..09fcc2f3897 100644 --- a/test/Sema/flow/implicit-return.js +++ b/test/Sema/flow/implicit-return.js @@ -128,24 +128,26 @@ function main(x) { // CHECK-NEXT: Decl %d.18 'arguments' Var Arguments // CHECK-NEXT: Scope %s.13 // CHECK-NEXT: Scope %s.14 +// CHECK-NEXT: Scope %s.15 // CHECK-NEXT: Func strict -// CHECK-NEXT: Scope %s.15 +// CHECK-NEXT: Scope %s.16 // CHECK-NEXT: Decl %d.19 'arguments' Var Arguments -// CHECK-NEXT: Scope %s.16 // CHECK-NEXT: Scope %s.17 +// CHECK-NEXT: Scope %s.18 // CHECK-NEXT: Func strict -// CHECK-NEXT: Scope %s.18 +// CHECK-NEXT: Scope %s.19 // CHECK-NEXT: Decl %d.20 'arguments' Var Arguments -// CHECK-NEXT: Scope %s.19 -// CHECK-NEXT: Scope %s.20 +// CHECK-NEXT: Scope %s.20 // CHECK-NEXT: Scope %s.21 -// CHECK-NEXT: Scope %s.22 +// CHECK-NEXT: Scope %s.22 +// CHECK-NEXT: Scope %s.23 +// CHECK-NEXT: Scope %s.24 // CHECK-NEXT: Func strict -// CHECK-NEXT: Scope %s.23 +// CHECK-NEXT: Scope %s.25 // CHECK-NEXT: Decl %d.21 'arguments' Var Arguments -// CHECK-NEXT: Scope %s.24 -// CHECK-NEXT: Scope %s.25 -// CHECK-NEXT: Scope %s.26 +// CHECK-NEXT: Scope %s.26 +// CHECK-NEXT: Scope %s.27 +// CHECK-NEXT: Scope %s.28 // CHECK:Program Scope %s.1 // CHECK-NEXT: ExpressionStatement @@ -201,29 +203,29 @@ function main(x) { // CHECK-NEXT: ReturnStatement // CHECK-NEXT: NumericLiteral : number // CHECK-NEXT: CatchClause Scope %s.14 -// CHECK-NEXT: BlockStatement +// CHECK-NEXT: BlockStatement Scope %s.15 // CHECK-NEXT: ReturnStatement // CHECK-NEXT: NumericLiteral : number // CHECK-NEXT: FunctionDeclaration : %function.4 // CHECK-NEXT: Id 'f6' [D:E:%d.10 'f6'] // CHECK-NEXT: BlockStatement // CHECK-NEXT: TryStatement -// CHECK-NEXT: BlockStatement Scope %s.16 +// CHECK-NEXT: BlockStatement Scope %s.17 // CHECK-NEXT: ReturnStatement // CHECK-NEXT: NumericLiteral : number -// CHECK-NEXT: BlockStatement Scope %s.17 +// CHECK-NEXT: BlockStatement Scope %s.18 // CHECK-NEXT: FunctionDeclaration : %function.4 // CHECK-NEXT: Id 'f7' [D:E:%d.11 'f7'] // CHECK-NEXT: BlockStatement // CHECK-NEXT: TryStatement -// CHECK-NEXT: BlockStatement Scope %s.19 +// CHECK-NEXT: BlockStatement Scope %s.20 // CHECK-NEXT: TryStatement -// CHECK-NEXT: BlockStatement Scope %s.20 +// CHECK-NEXT: BlockStatement Scope %s.21 // CHECK-NEXT: ReturnStatement // CHECK-NEXT: NumericLiteral : number -// CHECK-NEXT: BlockStatement Scope %s.21 -// CHECK-NEXT: CatchClause Scope %s.22 -// CHECK-NEXT: BlockStatement +// CHECK-NEXT: BlockStatement Scope %s.22 +// CHECK-NEXT: CatchClause Scope %s.23 +// CHECK-NEXT: BlockStatement Scope %s.24 // CHECK-NEXT: ReturnStatement // CHECK-NEXT: NumericLiteral : number // CHECK-NEXT: FunctionDeclaration : %function.4 @@ -231,12 +233,12 @@ function main(x) { // CHECK-NEXT: BlockStatement // CHECK-NEXT: LabeledStatement // CHECK-NEXT: Id 'label' -// CHECK-NEXT: BlockStatement Scope %s.24 +// CHECK-NEXT: BlockStatement Scope %s.26 // CHECK-NEXT: TryStatement -// CHECK-NEXT: BlockStatement Scope %s.25 +// CHECK-NEXT: BlockStatement Scope %s.27 // CHECK-NEXT: BreakStatement // CHECK-NEXT: Id 'label' -// CHECK-NEXT: BlockStatement Scope %s.26 +// CHECK-NEXT: BlockStatement Scope %s.28 // CHECK-NEXT: ReturnStatement // CHECK-NEXT: NumericLiteral : number // CHECK-NEXT: ObjectExpression : %object.5 diff --git a/test/hermes/capture-in-catch-params.js b/test/hermes/capture-in-catch-params.js new file mode 100644 index 00000000000..c4171516b0e --- /dev/null +++ b/test/hermes/capture-in-catch-params.js @@ -0,0 +1,24 @@ +/** + * Copyright (c) Meta Platforms, Inc. and affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +// RUN: %hermes %s | %FileCheck --match-full-lines %s +// RUN: %hermes -lazy %s | %FileCheck --match-full-lines %s + +var probeParam, probeBlock; +let x = 'outside'; + +try { + throw []; +} catch ([_ = probeParam = function() { return x; }]) { + probeBlock = function() { return x; }; + let x = 'inside'; +} + +print(probeParam()); +// CHECK: outside +print(probeBlock()); +// CHECK: inside