From f9765d1c58f2bc81b08f218a3492c2a7f1d71afb Mon Sep 17 00:00:00 2001 From: jan Date: Wed, 20 Mar 2013 20:35:42 -0700 Subject: [PATCH] Fix local propagation of generator parameters Alias manager does not know that generator parameters are populated and assumes they are uninit. The current code works because control flow algorithm gives up while trying to deal with the continuation switch statement full of gotos. This diff fixes it by setting isGeneratorParameter flag in symbols representing parameters of enclosing generator wrapper and use variables of enclosing closure. --- hphp/compiler/analysis/alias_manager.cpp | 3 +- hphp/compiler/analysis/live_dict.cpp | 19 ++---------- hphp/compiler/analysis/symbol_table.h | 9 +++++- hphp/compiler/parser/parser.cpp | 9 ++++-- hphp/compiler/statement/method_statement.cpp | 32 +++++++++++++++++--- hphp/compiler/statement/method_statement.h | 9 ++++++ 6 files changed, 54 insertions(+), 27 deletions(-) diff --git a/hphp/compiler/analysis/alias_manager.cpp b/hphp/compiler/analysis/alias_manager.cpp index 1d74f3e46..95ee9b588 100644 --- a/hphp/compiler/analysis/alias_manager.cpp +++ b/hphp/compiler/analysis/alias_manager.cpp @@ -1490,7 +1490,8 @@ ExpressionPtr AliasManager::canonicalizeNode( e->is(Expression::KindOfSimpleVariable) && !e->isThis()) { Symbol *s = spc(SimpleVariable, e)->getSymbol(); - if (s && !s->isParameter() && !s->isClosureVar()) { + if (s && !s->isParameter() && !s->isGeneratorParameter() && + !s->isClosureVar()) { rep = e->makeConstant(m_arp, "null"); Compiler::Error(Compiler::UseUndeclaredVariable, e); if (m_variables->getAttribute(VariableTable::ContainsCompact)) { diff --git a/hphp/compiler/analysis/live_dict.cpp b/hphp/compiler/analysis/live_dict.cpp index 36c8f34f8..a3a2e92a8 100644 --- a/hphp/compiler/analysis/live_dict.cpp +++ b/hphp/compiler/analysis/live_dict.cpp @@ -461,25 +461,11 @@ bool LiveDict::color(TypePtr type) { if (Type::SameType(type, e->getCPPType())) { SimpleVariablePtr sv( static_pointer_cast(e)); - bool isGenParam = false; - if (sv->getFunctionScope()->isGenerator()) { - // do not allow coalescing of symbols which are parameters/use vars - // in the generator (sym->isParameter() will be false b/c we are in - // the scope of the generator function) - FunctionScopeRawPtr origScope(sv->getFunctionScope()->getOrigGenFS()); - assert(origScope); - Symbol *origSym = - origScope->getVariables()->getSymbol(sv->getName()); - if (origSym && - (origSym->isParameter() || origSym->isClosureVar())) { - isGenParam = true; - } - } Symbol *sym = sv->getSymbol(); if (sym && !sym->isGlobal() && !sym->isParameter() && - !isGenParam && + !sym->isGeneratorParameter() && !sym->isClosureVar() && !sym->isStatic() && !e->isThis()) { @@ -623,9 +609,8 @@ public: always_assert(e && e->is(Expression::KindOfSimpleVariable)); SimpleVariablePtr sv(static_pointer_cast(e)); Symbol *sym = sv->getSymbol(); - bool inGen = sv->getFunctionScope()->isGenerator(); if (!sym || sym->isGlobal() || sym->isStatic() || sym->isParameter() || - sym->isClosureVar() || sv->isThis() || inGen) { + sym->isGeneratorParameter() || sym->isClosureVar() || sv->isThis()) { continue; } diff --git a/hphp/compiler/analysis/symbol_table.h b/hphp/compiler/analysis/symbol_table.h index 7ce1252f9..d8b1b3d88 100644 --- a/hphp/compiler/analysis/symbol_table.h +++ b/hphp/compiler/analysis/symbol_table.h @@ -115,6 +115,7 @@ public: bool isIndirectAltered() const { return m_flags.m_indirectAltered; } bool isReferenced() const { return !m_flags.m_notReferenced; } bool isHidden() const { return m_flags.m_hidden; } + bool isGeneratorParameter() const { return m_flags.m_generatorParameter; } bool isClosureVar() const { return m_flags.m_closureVar; } bool isRefClosureVar() const { return m_flags.m_refClosureVar; } bool isPassClosureVar() const { return m_flags.m_passClosureVar; } @@ -140,6 +141,7 @@ public: void setIndirectAltered() { m_flags.m_indirectAltered = true; } void setReferenced() { m_flags.m_notReferenced = false; } void setHidden() { m_flags.m_hidden = true; } + void setGeneratorParameter() { m_flags.m_generatorParameter = true; } void setClosureVar() { m_flags.m_closureVar = true; } void setRefClosureVar() { m_flags.m_refClosureVar = true; } void setPassClosureVar() { m_flags.m_passClosureVar = true; } @@ -185,7 +187,7 @@ private: std::string m_name; unsigned int m_hash; union { - unsigned m_flags_val; + uint64_t m_flags_val; struct { /* internal */ unsigned m_declaration_set : 1; @@ -219,6 +221,7 @@ private: unsigned m_indirectAltered : 1; unsigned m_notReferenced : 1; unsigned m_hidden : 1; + unsigned m_generatorParameter : 1; unsigned m_closureVar : 1; unsigned m_refClosureVar : 1; unsigned m_passClosureVar : 1; @@ -227,6 +230,10 @@ private: unsigned m_stashedVal : 1; unsigned m_reseated : 1; } m_flags; + + static_assert( + sizeof(m_flags_val) == sizeof(m_flags), + "m_flags_val must cover all the flags"); }; ConstructPtr m_declaration; ConstructPtr m_value; diff --git a/hphp/compiler/parser/parser.cpp b/hphp/compiler/parser/parser.cpp index d6d96ccef..dc89467fb 100644 --- a/hphp/compiler/parser/parser.cpp +++ b/hphp/compiler/parser/parser.cpp @@ -1556,10 +1556,13 @@ void Parser::onClosure(Token &out, Token &ret, Token &ref, Token ¶ms, } onFunction(func, &modifiers, ret, ref, name, params, stmts, 0); + ClosureExpressionPtr closure = NEW_EXP( + ClosureExpression, + dynamic_pointer_cast(func->stmt), + dynamic_pointer_cast(cparams->exp)); + closure->getClosureFunction()->setContainingClosure(closure); out.reset(); - out->exp = NEW_EXP(ClosureExpression, - dynamic_pointer_cast(func->stmt), - dynamic_pointer_cast(cparams->exp)); + out->exp = closure; } void Parser::onClosureParam(Token &out, Token *params, Token ¶m, diff --git a/hphp/compiler/statement/method_statement.cpp b/hphp/compiler/statement/method_statement.cpp index ca8d635f2..28ed30f49 100644 --- a/hphp/compiler/statement/method_statement.cpp +++ b/hphp/compiler/statement/method_statement.cpp @@ -33,6 +33,7 @@ #include #include #include +#include #include #include @@ -372,15 +373,36 @@ void MethodStatement::analyzeProgram(AnalysisResultPtr ar) { if (ar->getPhase() == AnalysisResult::AnalyzeAll) { funcScope->setParamSpecs(ar); if (funcScope->isGenerator()) { + MethodStatementRawPtr orig = getOrigGeneratorFunc(); VariableTablePtr variables = funcScope->getVariables(); + Symbol *cont = variables->getSymbol(CONTINUATION_OBJECT_NAME); cont->setHidden(); - getOrigGeneratorFunc()->getFunctionScope()->addUse( - funcScope, BlockScope::UseKindClosure); - getOrigGeneratorFunc()->getFunctionScope()->setContainsBareThis( + + orig->getFunctionScope()->addUse(funcScope, BlockScope::UseKindClosure); + orig->getFunctionScope()->setContainsBareThis( funcScope->containsBareThis(), funcScope->containsRefThis()); - getOrigGeneratorFunc()->getFunctionScope()->setContainsThis( - funcScope->containsThis()); + orig->getFunctionScope()->setContainsThis(funcScope->containsThis()); + + if (ExpressionListPtr params = orig->getParams()) { + for (int i = 0; i < params->getCount(); ++i) { + auto param = dynamic_pointer_cast((*params)[i]); + Symbol *gp = variables->addDeclaredSymbol( + param->getName(), ConstructPtr()); + gp->setGeneratorParameter(); + } + } + + if (ClosureExpressionRawPtr closure = orig->getContainingClosure()) { + if (ExpressionListPtr cvars = closure->getClosureVariables()) { + for (int i = 0; i < cvars->getCount(); ++i) { + auto param = dynamic_pointer_cast((*cvars)[i]); + Symbol *gp = variables->addDeclaredSymbol( + param->getName(), ConstructPtr()); + gp->setGeneratorParameter(); + } + } + } } if (funcScope->isSepExtension() || Option::IsDynamicFunction(m_method, m_name) || Option::AllDynamic) { diff --git a/hphp/compiler/statement/method_statement.h b/hphp/compiler/statement/method_statement.h index ae314cd44..eff88d777 100644 --- a/hphp/compiler/statement/method_statement.h +++ b/hphp/compiler/statement/method_statement.h @@ -22,6 +22,7 @@ namespace HPHP { /////////////////////////////////////////////////////////////////////////////// +DECLARE_BOOST_TYPES(ClosureExpression); DECLARE_BOOST_TYPES(ModifierExpression); DECLARE_BOOST_TYPES(ExpressionList); DECLARE_BOOST_TYPES(StatementList); @@ -104,6 +105,13 @@ public: return m_generatorFunc; } + void setContainingClosure(ClosureExpressionRawPtr exp) { + m_containingClosure = exp; + } + ClosureExpressionRawPtr getContainingClosure() const { + return m_containingClosure; + } + void setClassName(const std::string &name) { m_className = name; } void setOriginalClassName(const std::string &name) { m_originalClassName = name; @@ -127,6 +135,7 @@ protected: std::string m_docComment; MethodStatementRawPtr m_origGeneratorFunc; MethodStatementRawPtr m_generatorFunc; + ClosureExpressionRawPtr m_containingClosure; ExpressionListPtr m_attrList; void setSpecialMethod(ClassScopePtr classScope);