From f0d9e14e209947c9869ccffc35049978ea8eb693 Mon Sep 17 00:00:00 2001 From: Thomas Rossi Mel Date: Wed, 9 Oct 2024 15:16:14 +0200 Subject: [PATCH 1/2] Add with statement implementation via IR This commit introduces support for the with statement via IR, by replacing the previous AST transformation with a more robust alternative. **Logic:** - Identifiers are replaced with a conditional chain that checks the presence of a key in the object properties. - We need to keep track of each `with` statement depth. Hermes also uses a task queue to lazily compile functions, and for this reason, we also need to copy the `with` depths when adding the declaration to the queue. - Compared to an AST transformation, `buildConditionalChain` any call to the Builder will be executed in the same called order. This means we cannot generate the nested conditional chain starting from the inner condition. **Test262 results:** 92.98%. Cooled by trossimel --- include/hermes/Sema/SemContext.h | 13 +++ lib/CMakeLists.txt | 1 + lib/IRGen/ESTreeIRGen-expr.cpp | 78 +++++++++++--- lib/IRGen/ESTreeIRGen-stmt.cpp | 4 + lib/IRGen/ESTreeIRGen-with.cpp | 162 ++++++++++++++++++++++++++++++ lib/IRGen/ESTreeIRGen.cpp | 7 +- lib/IRGen/ESTreeIRGen.h | 65 +++++++++++- lib/InternalJavaScript/05-With.js | 24 +++++ lib/Parser/JSParserImpl.cpp | 9 +- lib/Parser/JSParserImpl.h | 2 + lib/Sema/SemanticResolver.cpp | 11 +- 11 files changed, 349 insertions(+), 27 deletions(-) create mode 100644 lib/IRGen/ESTreeIRGen-with.cpp create mode 100644 lib/InternalJavaScript/05-With.js diff --git a/include/hermes/Sema/SemContext.h b/include/hermes/Sema/SemContext.h index 022e36d2621..30cd85df459 100644 --- a/include/hermes/Sema/SemContext.h +++ b/include/hermes/Sema/SemContext.h @@ -452,6 +452,17 @@ class SemContext { return root_->bindingTable_; } + uint32_t getWithLexicalDepth(ESTree::WithStatementNode * node) const { + if(auto it = withDepths.find(node); it != withDepths.end()) + return it->second; + assert(false && "with statement must have been processed before attempting to get the depth"); + return 0; + } + + void setWithLexicalDepth(ESTree::WithStatementNode * node, uint32_t depth) { + withDepths[node] = depth; + } + private: /// The parent SemContext of this SemContext. /// If null, this SemContext has no parent. @@ -490,6 +501,8 @@ class SemContext { /// "expression decl" are both set and are not the same value. llvh::DenseMap sideIdentifierDeclarationDecl_{}; + + llvh::DenseMap withDepths{}; }; class SemContextDumper { diff --git a/lib/CMakeLists.txt b/lib/CMakeLists.txt index 8a6dfd3e21b..a24c38add6d 100644 --- a/lib/CMakeLists.txt +++ b/lib/CMakeLists.txt @@ -41,6 +41,7 @@ add_hermes_library(hermesFrontend IRGen/ESTreeIRGen-func.cpp IRGen/ESTreeIRGen-except.cpp IRGen/ESTreeIRGen-class.cpp + IRGen/ESTreeIRGen-with.cpp IR/IR.cpp IR/CFG.cpp IR/IRBuilder.cpp diff --git a/lib/IRGen/ESTreeIRGen-expr.cpp b/lib/IRGen/ESTreeIRGen-expr.cpp index 90027bbb314..5d85968574a 100644 --- a/lib/IRGen/ESTreeIRGen-expr.cpp +++ b/lib/IRGen/ESTreeIRGen-expr.cpp @@ -522,7 +522,10 @@ Value *ESTreeIRGen::genCallExpr(ESTree::CallExpressionNode *call) { // Must call the field init function immediately after. fieldInitClassType = curFunction()->typedClassContext.type; } else { - thisVal = Builder.getLiteralUndefined(); + thisVal = withAwareEmitLoad( + nullptr, + call->_callee, + ConditionalChainType::OBJECT_ONLY_WITH_UNDEFINED_ALTERNATE); callee = genExpression(call->_callee); } @@ -588,7 +591,10 @@ Value *ESTreeIRGen::genOptionalCallExpr( thisVal = Builder.getLiteralUndefined(); callee = genOptionalCallExpr(oce, shortCircuitBB); } else { - thisVal = Builder.getLiteralUndefined(); + thisVal = withAwareEmitLoad( + nullptr, + call->_callee, + ConditionalChainType::OBJECT_ONLY_WITH_UNDEFINED_ALTERNATE); callee = genExpression(getCallee(call)); } @@ -2036,15 +2042,33 @@ Value *ESTreeIRGen::genUnaryExpression(ESTree::UnaryExpressionNode *U) { Identifier name = getNameFieldFromID(iden); auto *var = resolveIdentifier(iden); - if (llvh::isa(var)) { - // If the variable doesn't exist or if it is global, we must generate - // a delete global property instruction. - return Builder.createDeletePropertyInst( - Builder.getGlobalObject(), Builder.getLiteralString(name)); + // With this check we are sure nothing in Hermes breaks + if (withScopes.empty()) { + if (llvh::isa(var)) { + // If the variable doesn't exist or if it is global, we must generate + // a delete global property instruction. + return Builder.createDeletePropertyInst( + Builder.getGlobalObject(), Builder.getLiteralString(name)); + } else { + // Otherwise it is a local variable which can't be deleted and we just + // return false. + return Builder.getLiteralBool(false); + } } else { - // Otherwise it is a local variable which can't be deleted and we just - // return false. - return Builder.getLiteralBool(false); + auto withObj = withAwareEmitLoad( + var, + iden, + llvh::isa(var) + ? ConditionalChainType::OBJECT_ONLY_WITH_GLOBAL_ALTERNATE + : ConditionalChainType::OBJECT_ONLY_WITH_UNDEFINED_ALTERNATE); + + return genConditionalExpr( + [&]() -> Value * { return withObj; }, + [&]() -> Value * { + return Builder.createDeletePropertyInst( + withObj, Builder.getLiteralString(name)); + }, + [&]() -> Value * { return Builder.getLiteralBool(false); }); } } @@ -2305,6 +2329,36 @@ Value *ESTreeIRGen::genLogicalAssignmentExpr( return Builder.createPhiInst(std::move(values), std::move(blocks)); } +Value *ESTreeIRGen::genConditionalExpr( + const std::function &conditionGenerator, + const std::function &consequentGenerator, + const std::function &alternateGenerator) { + auto parentFunc = Builder.getInsertionBlock()->getParent(); + + PhiInst::ValueListType values; + PhiInst::BasicBlockListType blocks; + + auto alternateBlock = Builder.createBasicBlock(parentFunc); + auto consequentBlock = Builder.createBasicBlock(parentFunc); + auto continueBlock = Builder.createBasicBlock(parentFunc); + + Builder.createCondBranchInst( + conditionGenerator(), consequentBlock, alternateBlock); + + Builder.setInsertionBlock(consequentBlock); + values.push_back(consequentGenerator()); + blocks.push_back(Builder.getInsertionBlock()); + Builder.createBranchInst(continueBlock); + + Builder.setInsertionBlock(alternateBlock); + values.push_back(alternateGenerator()); + blocks.push_back(Builder.getInsertionBlock()); + Builder.createBranchInst(continueBlock); + + Builder.setInsertionBlock(continueBlock); + return Builder.createPhiInst(values, blocks); +} + Value *ESTreeIRGen::genConditionalExpr(ESTree::ConditionalExpressionNode *C) { auto parentFunc = Builder.getInsertionBlock()->getParent(); @@ -2357,7 +2411,7 @@ Value *ESTreeIRGen::genIdentifierExpression( // For uses of undefined/Infinity/NaN as the global property, we make an // optimization to always return the constant directly. - if (llvh::isa(Var)) { + if (llvh::isa(Var) && withScopes.empty()) { if (StrName.getUnderlyingPointer() == kw_.identUndefined) { return Builder.getLiteralUndefined(); } @@ -2374,7 +2428,7 @@ Value *ESTreeIRGen::genIdentifierExpression( << curFunction()->function->getInternalNameStr() << "\"\n"); // Typeof does not throw. - return emitLoad(Var, afterTypeOf); + return withAwareEmitLoad(Var, Iden, {}, afterTypeOf); } Value *ESTreeIRGen::genMetaProperty(ESTree::MetaPropertyNode *MP) { diff --git a/lib/IRGen/ESTreeIRGen-stmt.cpp b/lib/IRGen/ESTreeIRGen-stmt.cpp index b20af3078e5..618d9e9074f 100644 --- a/lib/IRGen/ESTreeIRGen-stmt.cpp +++ b/lib/IRGen/ESTreeIRGen-stmt.cpp @@ -219,6 +219,10 @@ void ESTreeIRGen::genStatement(ESTree::Node *stmt) { return genClassDeclaration(classDecl); } + if (auto *withDecl = llvh::dyn_cast(stmt)) { + return genWithStatement(withDecl); + } + Builder.getModule()->getContext().getSourceErrorManager().error( stmt->getSourceRange(), Twine("invalid statement encountered.")); } diff --git a/lib/IRGen/ESTreeIRGen-with.cpp b/lib/IRGen/ESTreeIRGen-with.cpp new file mode 100644 index 00000000000..f3057295e3e --- /dev/null +++ b/lib/IRGen/ESTreeIRGen-with.cpp @@ -0,0 +1,162 @@ +#include "ESTreeIRGen.h" + +#include "hermes/IR/Instrs.h" +#include "llvh/ADT/SmallString.h" + +namespace hermes { +namespace irgen { + +void ESTreeIRGen::genWithStatement(ESTree::WithStatementNode *with) { + WithScopeInfo withScope{}; + withScope.depth = semCtx_.getWithLexicalDepth(with); + + withScope.object = Builder.createVariable( + curFunction()->curScope->getVariableScope(), + Builder + .getLiteralString( + internal_with.data() + std::to_string(withScope.depth)) + ->getValue(), + Type::createAnyType(), + true); + withScope.object->setIsConst(true); + emitStore(genExpression(with->_object), withScope.object, true); + + withScopes.push_back(withScope); + genStatement(with->_body); + withScopes.pop_back(); +} + +Value *ESTreeIRGen::emitLoadOrStoreWithStatementImpl( + Value *value, + Value *ptr, + bool declInit_, + ESTree::IdentifierNode *id, + ConditionalChainType conditionalChainType, + bool inhibitThrow) { + uint32_t identifierDepth = INT_MAX; + std::string_view name; + + identifierDepth = getIDDecl(id)->scope->depth; + name = id->_name->c_str(); + + auto compareDepth = [](uint32_t searching, const WithScopeInfo &scope) { + return scope.depth > searching; + }; + + auto it = std::upper_bound( + withScopes.begin(), withScopes.end(), identifierDepth, compareDepth); + + return createConditionalChainImpl( + it, withScopes.end(), + ptr, + value, + declInit_, + name, + conditionalChainType, + inhibitThrow); +} + +Value *ESTreeIRGen::createConditionalChainImpl( + std::vector::iterator begin, + std::vector::iterator end, + Value *ptr, + Value *value, + bool declInit_, + std::string_view name, + ConditionalChainType conditionalChainType, + bool inhibitThrow) { + if (begin == end) { + if (conditionalChainType == + ConditionalChainType::OBJECT_ONLY_WITH_UNDEFINED_ALTERNATE) { + return Builder.getLiteralUndefined(); + } else if ( + conditionalChainType == + ConditionalChainType::OBJECT_ONLY_WITH_GLOBAL_ALTERNATE) { + return Builder.getGlobalObject(); + } else if (value) { + return emitStore(value, ptr, declInit_); + } else { + return emitLoad(ptr, inhibitThrow); + } + } + + auto ¤t = *(end - 1); + auto conditionGenerator = [&]() -> Value * { + auto *wrapper = Builder.createLoadPropertyInst( + Builder.getGlobalObject(), "HermesWithInternal"); + wrapper = Builder.createLoadPropertyInst(wrapper, "_containsField"); + + auto *call = Builder.createCallInst( + wrapper, + Builder.getLiteralUndefined(), + Builder.getLiteralUndefined(), + {emitLoad(current.object, false), + Builder.getLiteralString(name.data())}); + + return call; + }; + + auto consequentGenerator = [&]() -> Value * { + auto loadWithObj = emitLoad(current.object, false); + + if (conditionalChainType == ConditionalChainType::MEMBER_EXPRESSION) { + if (value) { + return Builder.createStorePropertyInst(value, loadWithObj, name.data()); + } else { + return Builder.createLoadPropertyInst(loadWithObj, name.data()); + } + } + return loadWithObj; + }; + + auto alternateGenerator = [&]() -> Value * { + return createConditionalChainImpl( + begin, end - 1, + ptr, + value, + declInit_, + name, + conditionalChainType, + inhibitThrow); + }; + + return genConditionalExpr( + conditionGenerator, consequentGenerator, alternateGenerator); +} + +Value *ESTreeIRGen::withAwareEmitLoad( + hermes::Value *ptr, + ESTree::Node *node, + ConditionalChainType conditionalChainType, + bool inhibitThrow) { + ESTree::IdentifierNode *identifier = + node ? llvh::dyn_cast(node) : nullptr; + if (withScopes.empty() || !identifier) { + if (!ptr) + return Builder.getLiteralUndefined(); + return emitLoad(ptr, inhibitThrow); + } + return emitLoadOrStoreWithStatementImpl( + nullptr, ptr, false, identifier, conditionalChainType, inhibitThrow); +} + +Value *ESTreeIRGen::withAwareEmitStore( + Value *storedValue, + Value *ptr, + bool declInit_, + ESTree::Node *node) { + ESTree::IdentifierNode *identifier = + node ? llvh::dyn_cast(node) : nullptr; + if (withScopes.empty() || !identifier) { + return emitStore(storedValue, ptr, declInit_); + } + return emitLoadOrStoreWithStatementImpl( + storedValue, + ptr, + declInit_, + identifier, + ConditionalChainType::MEMBER_EXPRESSION); +} + +} // namespace irgen +} // namespace hermes diff --git a/lib/IRGen/ESTreeIRGen.cpp b/lib/IRGen/ESTreeIRGen.cpp index 411d2fea2fb..1c20a439106 100644 --- a/lib/IRGen/ESTreeIRGen.cpp +++ b/lib/IRGen/ESTreeIRGen.cpp @@ -54,7 +54,8 @@ Value *LReference::emitLoad() { llvh::cast(ast_), base_, property_) .result; case Kind::VarOrGlobal: - return irgen_->emitLoad(base_, false); + return irgen_->withAwareEmitLoad( + base_, ast_, ESTreeIRGen::ConditionalChainType::MEMBER_EXPRESSION); case Kind::Destructuring: assert(false && "destructuring cannot be loaded"); return builder.getLiteralUndefined(); @@ -76,7 +77,7 @@ void LReference::emitStore(Value *value) { base_, property_); case Kind::VarOrGlobal: - irgen_->emitStore(value, base_, declInit_); + irgen_->withAwareEmitStore(value, base_, declInit_, ast_); return; case Kind::Error: return; @@ -393,7 +394,7 @@ LReference ESTreeIRGen::createLRef(ESTree::Node *node, bool declInit) { LReference::Kind::VarOrGlobal, this, declInit, - nullptr, + iden, var, nullptr, sourceLoc); diff --git a/lib/IRGen/ESTreeIRGen.h b/lib/IRGen/ESTreeIRGen.h index 552d99ed504..85b6ad3723f 100644 --- a/lib/IRGen/ESTreeIRGen.h +++ b/lib/IRGen/ESTreeIRGen.h @@ -191,6 +191,7 @@ class FunctionContext { /// \param irGen the associated ESTreeIRGen object. /// \param function the newly created Function IR node. /// \param semInfo semantic info obtained from the AST node. + /// \param scopeInfo the scope information on depth and with blocks FunctionContext( ESTreeIRGen *irGen, Function *function, @@ -405,6 +406,8 @@ class ESTreeIRGen { /// whenever we enter a nested function. FunctionContext *functionContext_{}; + static constexpr std::string_view internal_with = "__internal_with_"; + /// The type of the key stored in \c compiledEntities_. It is a pair of an /// AST node pointer and an extra key, which is a small integer value. /// The purpose of the extra key is to enable us to distinguish different @@ -412,6 +415,21 @@ class ESTreeIRGen { /// "outer" and "inner" generator function. using CompiledMapKey = llvh::PointerIntPair; + + // Information about each nested 'with' statement, used to resolve identifiers correctly. + struct WithScopeInfo { + + // Lexical depth of the 'with' body (=depth where `with` is defined + 1). + uint32_t depth; + + // The variable that holds the object of the 'with' statement. + Variable *object; + }; + + // Vector holding information about all encountered 'with' statements. + using WithScopes = std::vector; + WithScopes withScopes{}; + /// An "additional key" when mapping from an AST node to a compiled Value, /// used to distinguish between different values that may be associated with /// the same node. @@ -829,6 +847,13 @@ class ESTreeIRGen { Value *genLogicalExpression(ESTree::LogicalExpressionNode *logical); Value *genThisExpression(); + // Similar to genConditionalExpr, but we can generate condition, consequent + // and alternate lazily when creating the branches. This is useful when we + // want to generate cond. blocks directly from Value* instead of AST nodes + Value *genConditionalExpr( + const std::function &conditionGenerator, + const std::function &consequentGenerator, + const std::function &alternateGenerator); /// A helper function to unify the largely same IRGen logic of \c genYieldExpr /// and \c genAwaitExpr. /// \param value the value operand of the will-generate SaveAndYieldInst. @@ -1029,6 +1054,8 @@ class ESTreeIRGen { VariableScope *parentScope, const CapturedState &capturedState); + void genWithStatement(ESTree::WithStatementNode *with); + /// In the beginning of an ES5 function, initialize the special captured /// variables needed by arrow functions, constructors and methods. /// This is used only by \c genES5Function() and the global scope. @@ -1363,6 +1390,21 @@ class ESTreeIRGen { /// \return the instruction performing the load. Instruction *emitLoad(Value *from, bool inhibitThrow); + enum ConditionalChainType { + // Multiple options to create the conditional chain: + MEMBER_EXPRESSION, // (a.var ? a.var : var) + OBJECT_ONLY_WITH_UNDEFINED_ALTERNATE, // (a.var ? a : undefined) + OBJECT_ONLY_WITH_GLOBAL_ALTERNATE // (a.var ? a : global) + }; + + Value *withAwareEmitLoad( + Value *ptr, + ESTree::Node *node, + ConditionalChainType conditionalChainType, + bool inhibitThrow = false); + Value * + withAwareEmitStore(Value *storedValue, Value *ptr, bool declInit_, ESTree::Node *node); + /// Emit an instruction to a store a value into the specified location. /// \param storedValue value to store /// \param ptr location to store into, either a Variable or @@ -1402,7 +1444,10 @@ class ESTreeIRGen { CompiledMapKey key(node, (unsigned)extraKey); assert(compiledEntities_.count(key) == 0 && "Overwriting compiled entity"); compiledEntities_[key] = value; - compilationQueue_.emplace_back(f); + compilationQueue_.emplace_back([this, f = std::forward(f), withScopesCopy = withScopes]() mutable { + this->withScopes = std::move(withScopesCopy); + f(); + }); } /// Run all tasks in the compilation queue until it is empty. @@ -1427,6 +1472,24 @@ class ESTreeIRGen { assert(decl->customData); return static_cast(decl->customData); } + + Value *emitLoadOrStoreWithStatementImpl( + Value *value, // value to store, nullptr if reading + Value *ptr, // ptr used to read/store the value + bool declInit_, + ESTree::IdentifierNode *id, + ConditionalChainType conditionalChainType, + bool inhibitThrow = false); + + Value *createConditionalChainImpl( + std::vector::iterator begin, + std::vector::iterator end, + Value *ptr, + Value *value, + bool declInit_, + std::string_view name, + ConditionalChainType conditionalChainType, + bool inhibitThrow); }; template diff --git a/lib/InternalJavaScript/05-With.js b/lib/InternalJavaScript/05-With.js new file mode 100644 index 00000000000..86997a0237a --- /dev/null +++ b/lib/InternalJavaScript/05-With.js @@ -0,0 +1,24 @@ +(function() { + function _containsField(obj, field) { + // We use __containsField to handle multiple scenarios: + // 1. checking for properties in primitive types (e.g. _containsField(1, 'toString') should return true) + // 2. checking for properties in objects and functions, including undefined properties (e.g. _containsField({a:undefined}, 'a') should return true) + if (obj instanceof Object) { + if (obj.hasOwnProperty(Symbol.unscopables)) { + const unscopables = obj[Symbol.unscopables]; + if (unscopables && unscopables[field]) { + return false; + } + } + return obj[field] !== undefined || obj.hasOwnProperty(field); + } else { + return obj[field] !== undefined; + } + } + + var HermesWithInternal = { + _containsField + }; + Object.freeze(HermesWithInternal); + globalThis.HermesWithInternal = HermesWithInternal; +})(); diff --git a/lib/Parser/JSParserImpl.cpp b/lib/Parser/JSParserImpl.cpp index 3435a04ae59..625915102bf 100644 --- a/lib/Parser/JSParserImpl.cpp +++ b/lib/Parser/JSParserImpl.cpp @@ -722,7 +722,7 @@ Optional JSParserImpl::parseFunctionBody( // because initializer expressions capture a different environment than the // environment the function body names are added to, and it would add some // complexity to optimize a use case that's never actually encountered. - if (pass_ == LazyParse && !eagerly && !isFormalParams_) { + if (pass_ == LazyParse && !eagerly && !isFormalParams_ && !insideWithStatement) { auto startLoc = tok_->getStartLoc(); assert( preParsed_->functionInfo.count(startLoc) == 1 && @@ -2038,6 +2038,8 @@ Optional JSParserImpl::parseWithStatement( startLoc)) return None; + auto oldInsideWithStatement = insideWithStatement; + insideWithStatement = true; auto optExpr = parseExpression(); if (!optExpr) return None; @@ -2054,6 +2056,11 @@ Optional JSParserImpl::parseWithStatement( if (!optBody) return None; + // More work is needed to support 'with' statements in lazy mode. + // Since 'with' statements are not often used, and to avoid having too many changes in IRGen, + // we can avoid supporting them in lazy mode for now. This will also make merging changes easier. + insideWithStatement = oldInsideWithStatement; + return setLocation( startLoc, optBody.getValue(), diff --git a/lib/Parser/JSParserImpl.h b/lib/Parser/JSParserImpl.h index a6c73235d6e..a3b74b147fc 100644 --- a/lib/Parser/JSParserImpl.h +++ b/lib/Parser/JSParserImpl.h @@ -208,6 +208,8 @@ class JSParserImpl { /// This is used when checking if `await` is a valid Identifier name. bool paramAwait_{false}; + bool insideWithStatement{false}; + /// Appended when the parser has seen an directive being visited in the /// current function scope (It's intended to be used with /// `SaveStrictModeAndSeenDirectives`). diff --git a/lib/Sema/SemanticResolver.cpp b/lib/Sema/SemanticResolver.cpp index 995fddb716b..e2f4c881bc2 100644 --- a/lib/Sema/SemanticResolver.cpp +++ b/lib/Sema/SemanticResolver.cpp @@ -672,17 +672,8 @@ void SemanticResolver::visit(ESTree::ContinueStatementNode *node) { } void SemanticResolver::visit(ESTree::WithStatementNode *node) { - if (compile_) - sm_.error(node->getStartLoc(), "with statement is not supported"); - + semCtx_.setWithLexicalDepth(node, curScope_->depth+1); visitESTreeChildren(*this, node); - - uint32_t depth = curScope_->depth; - // Run the Unresolver to avoid resolving to variables past the depth of the - // `with`. - // Pass `depth + 1` because variables declared in this scope also cannot be - // trusted. - Unresolver::run(semCtx_, depth + 1, node->_body); } void SemanticResolver::visit(ESTree::TryStatementNode *tryStatement) { From 141e5234d828684372ad4de95ea5665ddd1e0a89 Mon Sep 17 00:00:00 2001 From: Sumant Hanumante Date: Tue, 28 Jan 2025 12:29:47 +0000 Subject: [PATCH 2/2] fix and improvements --- lib/IRGen/ESTreeIRGen-with.cpp | 124 +++++++++++++-------------------- lib/IRGen/ESTreeIRGen.h | 44 ++++++------ lib/Sema/SemanticResolver.cpp | 8 +++ 3 files changed, 78 insertions(+), 98 deletions(-) diff --git a/lib/IRGen/ESTreeIRGen-with.cpp b/lib/IRGen/ESTreeIRGen-with.cpp index b4d2fd2eae7..edd3598702b 100644 --- a/lib/IRGen/ESTreeIRGen-with.cpp +++ b/lib/IRGen/ESTreeIRGen-with.cpp @@ -26,18 +26,20 @@ void ESTreeIRGen::genWithStatement(ESTree::WithStatementNode *with) { withScopes_.pop_back(); } -Value *ESTreeIRGen::emitLoadOrStoreWithStatementImpl( - Value *value, - Value *ptr, - bool declInit_, - ESTree::IdentifierNode *id, - ConditionalChainType conditionalChainType, - bool inhibitThrow) { - uint32_t identifierDepth = INT_MAX; - std::string_view name; +template +Value *ESTreeIRGen::createWithConditionalChain( + ESTree::Node *node, + const Callback &callback) { + ESTree::IdentifierNode *identifier = !withScopes_.empty() && node + ? llvh::dyn_cast(node) + : nullptr; + if (!identifier) { + return callback(nullptr, ""); + } - identifierDepth = getIDDecl(id)->scope->depth; - name = id->_name->c_str(); + uint32_t identifierDepth = INT_MAX; + identifierDepth = getIDDecl(identifier)->scope->depth; + std::string_view name = identifier->_name->c_str(); auto compareDepth = [](uint32_t searching, const WithScopeInfo &scope) { return scope.depth > searching; @@ -46,39 +48,17 @@ Value *ESTreeIRGen::emitLoadOrStoreWithStatementImpl( auto it = std::upper_bound( withScopes_.begin(), withScopes_.end(), identifierDepth, compareDepth); - return createConditionalChainImpl( - it, - withScopes_.end(), - ptr, - value, - declInit_, - name, - conditionalChainType, - inhibitThrow); + return createWithConditionalChainImpl(it, withScopes_.end(), callback, name); } -Value *ESTreeIRGen::createConditionalChainImpl( +template +Value *ESTreeIRGen::createWithConditionalChainImpl( std::vector::iterator begin, std::vector::iterator end, - Value *ptr, - Value *value, - bool declInit_, - std::string_view name, - ConditionalChainType conditionalChainType, - bool inhibitThrow) { + const Callback &callback, + std::string_view name) { if (begin == end) { - if (conditionalChainType == - ConditionalChainType::OBJECT_ONLY_WITH_UNDEFINED_ALTERNATE) { - return Builder.getLiteralUndefined(); - } else if ( - conditionalChainType == - ConditionalChainType::OBJECT_ONLY_WITH_GLOBAL_ALTERNATE) { - return Builder.getGlobalObject(); - } else if (value) { - return emitStore(value, ptr, declInit_); - } else { - return emitLoad(ptr, inhibitThrow); - } + return callback(nullptr, name); } auto ¤t = *(end - 1); @@ -99,26 +79,12 @@ Value *ESTreeIRGen::createConditionalChainImpl( auto consequentGenerator = [&]() -> Value * { auto loadWithObj = emitLoad(current.object, false); - - if (conditionalChainType == ConditionalChainType::MEMBER_EXPRESSION) { - if (value) { - return Builder.createStorePropertyInst(value, loadWithObj, name.data()); - } else { - return Builder.createLoadPropertyInst(loadWithObj, name.data()); - } - } - return loadWithObj; + return callback(loadWithObj, name); }; auto alternateGenerator = [&]() -> Value * { - return createConditionalChainImpl( - begin, end - 1, - ptr, - value, - declInit_, - name, - conditionalChainType, - inhibitThrow); + return createWithConditionalChainImpl( + begin, end - 1, callback, name); }; return genConditionalExpr( @@ -130,33 +96,37 @@ Value *ESTreeIRGen::withAwareEmitLoad( ESTree::Node *node, ConditionalChainType conditionalChainType, bool inhibitThrow) { - auto *identifier = - llvh::dyn_cast_or_null(node); - if (!identifier || withScopes_.empty()) { - if (!ptr) - return Builder.getLiteralUndefined(); - return emitLoad(ptr, inhibitThrow); - } - return emitLoadOrStoreWithStatementImpl( - nullptr, ptr, false, identifier, conditionalChainType, inhibitThrow); + return createWithConditionalChain( + node, [&](Value *withObject, std::string_view idName) -> Value * { + switch (conditionalChainType) { + case ConditionalChainType::OBJECT_ONLY_WITH_UNDEFINED_ALTERNATE: + return withObject ? withObject : Builder.getLiteralUndefined(); + case ConditionalChainType::OBJECT_ONLY_WITH_GLOBAL_ALTERNATE: + return withObject ? withObject : Builder.getGlobalObject(); + case ConditionalChainType::MEMBER_EXPRESSION: + return withObject + ? Builder.createLoadPropertyInst(withObject, idName.data()) + : emitLoad(ptr, inhibitThrow); + default: + llvm_unreachable("Unhandled conditionalChainType"); + } + }); } -Value *ESTreeIRGen::withAwareEmitStore( +void ESTreeIRGen::withAwareEmitStore( Value *storedValue, Value *ptr, bool declInit_, ESTree::Node *node) { - auto *identifier = - llvh::dyn_cast_or_null(node); - if (!identifier || withScopes_.empty()) { - return emitStore(storedValue, ptr, declInit_); - } - return emitLoadOrStoreWithStatementImpl( - storedValue, - ptr, - declInit_, - identifier, - ConditionalChainType::MEMBER_EXPRESSION); + createWithConditionalChain( + node, [&](Value *withObject, std::string_view idName) { + if (!withObject) + emitStore(storedValue, ptr, declInit_); + else + Builder.createStorePropertyInst( + storedValue, withObject, idName.data()); + return Builder.getLiteralUndefined(); + }); } } // namespace irgen diff --git a/lib/IRGen/ESTreeIRGen.h b/lib/IRGen/ESTreeIRGen.h index 5d16bc75351..ebd09293482 100644 --- a/lib/IRGen/ESTreeIRGen.h +++ b/lib/IRGen/ESTreeIRGen.h @@ -426,7 +426,8 @@ class ESTreeIRGen { /// "outer" and "inner" generator function. using CompiledMapKey = llvh::PointerIntPair; - + // Information about each nested 'with' statement, used to resolve identifiers + // correctly. struct WithScopeInfo { /// Lexical depth of the 'with' body @@ -1403,7 +1404,7 @@ class ESTreeIRGen { /// \return the instruction performing the store. Instruction *emitStore(Value *storedValue, Value *ptr, bool declInit); - enum ConditionalChainType { + enum class ConditionalChainType : uint8_t { // Multiple options to create the conditional chain: MEMBER_EXPRESSION, // (a.var ? a.var : var) OBJECT_ONLY_WITH_UNDEFINED_ALTERNATE, // (a.var ? a : undefined) @@ -1437,7 +1438,7 @@ class ESTreeIRGen { /// check should be skipped. /// \param node the AST node associated with the identifier in the store operation. /// \return the stored value, possibly modified to respect 'with' context rules. - Value *withAwareEmitStore( + void withAwareEmitStore( Value *storedValue, Value *ptr, bool declInit_, @@ -1472,10 +1473,11 @@ class ESTreeIRGen { CompiledMapKey key(node, (unsigned)extraKey); assert(compiledEntities_.count(key) == 0 && "Overwriting compiled entity"); compiledEntities_[key] = value; - compilationQueue_.emplace_back([this, f = std::forward(f), withScopesCopy = withScopes_]() mutable { - this->withScopes_ = std::move(withScopesCopy); - f(); - }); + compilationQueue_.emplace_back( + [this, f = std::forward(f), withScopesCopy = withScopes_]() mutable { + this->withScopes_ = std::move(withScopesCopy); + f(); + }); } /// Run all tasks in the compilation queue until it is empty. @@ -1501,23 +1503,23 @@ class ESTreeIRGen { return static_cast(decl->customData); } - Value *emitLoadOrStoreWithStatementImpl( - Value *value, // value to store, nullptr if reading - Value *ptr, // ptr used to read/store the value - bool declInit_, - ESTree::IdentifierNode *id, - ConditionalChainType conditionalChainType, - bool inhibitThrow = false); + /// Generates ternary operator chain for loads/stores on objects in all + /// surrounding with scopes. + /// \p callback void(Value *withObject, string_view name) - is called with the + /// object from `with(object)` if the identifier is found on the object from + /// surrounding with objects. Else is called with nullptr object indicating + /// that the identifier was not found in any of the surrounding with objects. + template + Value *createWithConditionalChain( + ESTree::Node *node, + const Callback &callback); - Value *createConditionalChainImpl( + template + Value *createWithConditionalChainImpl( std::vector::iterator begin, std::vector::iterator end, - Value *ptr, - Value *value, - bool declInit_, - std::string_view name, - ConditionalChainType conditionalChainType, - bool inhibitThrow); + const Callback &callback, + std::string_view name); }; template diff --git a/lib/Sema/SemanticResolver.cpp b/lib/Sema/SemanticResolver.cpp index d8dbf68c3bc..9287a517b94 100644 --- a/lib/Sema/SemanticResolver.cpp +++ b/lib/Sema/SemanticResolver.cpp @@ -689,6 +689,14 @@ void SemanticResolver::visit(ESTree::ContinueStatementNode *node) { } void SemanticResolver::visit(ESTree::WithStatementNode *node) { + if (curFunctionInfo()->strict) { + if (compile_) + sm_.error( + node->getStartLoc(), + "with statement is not supported in strict mode"); + return; + } + semCtx_.setWithLexicalDepth(node, curScope_->depth+1); visitESTreeChildren(*this, node); }