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

@@ -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)) {

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<SimpleVariable>(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<SimpleVariable>(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;
       }
 

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;

hphp/compiler/parser/parser.cpp

@@ -1556,10 +1556,13 @@ void Parser::onClosure(Token &out, Token &ret, Token &ref, Token &params,
   }
   onFunction(func, &modifiers, ret, ref, name, params, stmts, 0);
 
+  ClosureExpressionPtr closure = NEW_EXP(
+    ClosureExpression,
+    dynamic_pointer_cast<FunctionStatement>(func->stmt),
+    dynamic_pointer_cast<ExpressionList>(cparams->exp));
+  closure->getClosureFunction()->setContainingClosure(closure);
   out.reset();
-  out->exp = NEW_EXP(ClosureExpression,
-                     dynamic_pointer_cast<FunctionStatement>(func->stmt),
-                     dynamic_pointer_cast<ExpressionList>(cparams->exp));
+  out->exp = closure;
 }
 
 void Parser::onClosureParam(Token &out, Token *params, Token &param,

hphp/compiler/statement/method_statement.cpp

@@ -33,6 +33,7 @@
 #include <compiler/expression/parameter_expression.h>
 #include <compiler/expression/assignment_expression.h>
 #include <compiler/expression/simple_variable.h>
+#include <compiler/expression/closure_expression.h>
 
 #include <compiler/analysis/ast_walker.h>
 #include <compiler/analysis/analysis_result.h>
@@ -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<ParameterExpression>((*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<ParameterExpression>((*cvars)[i]);
+            Symbol *gp = variables->addDeclaredSymbol(
+              param->getName(), ConstructPtr());
+            gp->setGeneratorParameter();
+          }
+        }
+      }
     }
     if (funcScope->isSepExtension() ||
         Option::IsDynamicFunction(m_method, m_name) || Option::AllDynamic) {

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);