Skip to content

Commit

Permalink
[Fix] Fix HPHPi bug with yield and closures
Browse files Browse the repository at this point in the history 
Summary:
We do syntactic transformation of a function containing yield statements at
the AST level. The original function body is turned into a statement that
returns a Continuation object that is associated with a internally created
closure function. In HPHPi, the associated closure function must be declared
before it is used. In the newly added test case, the associated closure
function isn't declared when it is used. I renamed the pendingStatements
to prependingStatements and prepend the closure function before the original
statement from which the closure function is spinned off so the closure
function is declared prior to its usage.
Task ID: # 554932

Blame Rev:

Reviewers:
qigao, mwilliams
CC:
hphp-diffs@lists
Test Plan:
make fast_tests
make slow_tests
Revert Plan:

Tags:

- begin *PUBLIC* platform impact section -
Bugzilla: #
- end platform impact -

Differential Revision: 245798
  • Loading branch information
myang authored and macvicar committed May 9, 2011
1 parent 2ab0c6a commit 0071de0
Show file tree
Hide file tree
Showing 6 changed files with 62 additions and 43 deletions.
33 changes: 17 additions & 16 deletions src/compiler/parser/parser.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,7 @@ Parser::Parser(Scanner &scanner, const char *fileName,
Lock lock(m_ar->getMutex());
m_ar->addFileScope(m_file);

m_pendingStatements.push_back(vector<StatementPtr>());
m_prependingStatements.push_back(vector<StatementPtr>());
}

void Parser::error(const char* fmt, ...) {
Expand Down Expand Up @@ -619,7 +619,7 @@ void Parser::onFunctionStart(Token &name) {
newScope();
m_generators.push_back(0);
m_foreaches.push_back(0);
m_pendingStatements.push_back(vector<StatementPtr>());
m_prependingStatements.push_back(vector<StatementPtr>());
m_funcName = name.text();
m_hasCallToGetArgs.push_back(false);
m_staticVars.push_back(StringToExpressionPtrVecMap());
Expand Down Expand Up @@ -678,7 +678,7 @@ void Parser::onFunction(Token &out, Token &ret, Token &ref, Token &name,
int yieldCount = m_generators.back();
m_generators.pop_back();
m_foreaches.pop_back();
m_pendingStatements.pop_back();
m_prependingStatements.pop_back();

bool hasCallToGetArgs = m_hasCallToGetArgs.back();
m_hasCallToGetArgs.pop_back();
Expand All @@ -703,9 +703,9 @@ void Parser::onFunction(Token &out, Token &ret, Token &ref, Token &name,
if (func->ignored()) {
out->stmt = NEW_STMT0(StatementList);
} else {
ASSERT(!m_pendingStatements.empty());
vector<StatementPtr> &pending = m_pendingStatements.back();
pending.push_back(func);
ASSERT(!m_prependingStatements.empty());
vector<StatementPtr> &prepending = m_prependingStatements.back();
prepending.push_back(func);

create_generator(this, out, params, name, closureName, NULL, NULL,
hasCallToGetArgs);
Expand Down Expand Up @@ -856,7 +856,7 @@ void Parser::onMethod(Token &out, Token &modifiers, Token &ret, Token &ref,
int yieldCount = m_generators.back();
m_generators.pop_back();
m_foreaches.pop_back();
m_pendingStatements.pop_back();
m_prependingStatements.pop_back();

bool hasCallToGetArgs = m_hasCallToGetArgs.back();
m_hasCallToGetArgs.pop_back();
Expand Down Expand Up @@ -940,17 +940,18 @@ void Parser::addStatement(Token &out, Token &stmts, Token &new_stmt) {
} else {
out->stmt = stmts->stmt;
}
if (new_stmt->stmt) {
out->stmt->addElement(new_stmt->stmt);
}

ASSERT(!m_pendingStatements.empty());
vector<StatementPtr> &pending = m_pendingStatements.back();
if (!pending.empty()) {
for (unsigned i = 0; i < pending.size(); i++) {
out->stmt->addElement(pending[i]);
ASSERT(!m_prependingStatements.empty());
vector<StatementPtr> &prepending = m_prependingStatements.back();
if (!prepending.empty()) {
ASSERT(prepending.size() == 1);
for (unsigned i = 0; i < prepending.size(); i++) {
out->stmt->addElement(prepending[i]);
}
pending.clear();
prepending.clear();
}
if (new_stmt->stmt) {
out->stmt->addElement(new_stmt->stmt);
}
}

Expand Down
2 changes: 1 addition & 1 deletion src/compiler/parser/parser.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -215,7 +215,7 @@ class Parser : public ParserBase {
std::vector<BlockScopePtrVec> m_scopes;
std::vector<int> m_generators;
std::vector<int> m_foreaches;
std::vector<std::vector<StatementPtr> > m_pendingStatements;
std::vector<std::vector<StatementPtr> > m_prependingStatements;
std::string m_clsName; // for T_CLASS_C inside a closure
std::string m_funcName;

Expand Down
35 changes: 18 additions & 17 deletions src/runtime/eval/parser/parser.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -228,7 +228,7 @@ StatementPtr Parser::ParseFile(const char *fileName,
Parser::Parser(Scanner &scanner, const char *fileName,
vector<StaticStatementPtr> &statics)
: ParserBase(scanner, fileName), m_staticStatements(statics) {
m_pendingStatements.push_back(vector<StatementPtr>());
m_prependingStatements.push_back(vector<StatementPtr>());
}

void Parser::error(const char* fmt, ...) {
Expand Down Expand Up @@ -980,7 +980,7 @@ void Parser::onFunctionStart(Token &name) {
m_scanner.detachDocComment());
m_hasCallToGetArgs.push_back(false);
m_foreaches.push_back(0);
m_pendingStatements.push_back(vector<StatementPtr>());
m_prependingStatements.push_back(vector<StatementPtr>());
pushFunc(func);
}

Expand All @@ -998,7 +998,7 @@ void Parser::onFunction(Token &out, Token &ret, Token &ref, Token &name,
bool hasCallToGetArgs = m_hasCallToGetArgs.back();
m_hasCallToGetArgs.pop_back();
m_foreaches.pop_back();
m_pendingStatements.pop_back();
m_prependingStatements.pop_back();

if (func->hasYield()) {
string closureName = getClosureName();
Expand All @@ -1009,9 +1009,9 @@ void Parser::onFunction(Token &out, Token &ret, Token &ref, Token &name,
StatementListStatementPtr body = stmt->getStmtList();
func->init(this, ref.num(), new_params->params(), body, hasCallToGetArgs);

ASSERT(!m_pendingStatements.empty());
vector<StatementPtr> &pending = m_pendingStatements.back();
pending.push_back(func);
ASSERT(!m_prependingStatements.empty());
vector<StatementPtr> &prepending = m_prependingStatements.back();
prepending.push_back(func);

create_generator(this, out, params, name, closureName, NULL, NULL,
hasCallToGetArgs);
Expand Down Expand Up @@ -1120,7 +1120,7 @@ void Parser::onMethodStart(Token &name, Token &modifiers) {
m_scanner.detachDocComment());
m_hasCallToGetArgs.push_back(false);
m_foreaches.push_back(0);
m_pendingStatements.push_back(vector<StatementPtr>());
m_prependingStatements.push_back(vector<StatementPtr>());
pushFunc(func);
}

Expand All @@ -1138,7 +1138,7 @@ void Parser::onMethod(Token &out, Token &modifiers, Token &ret, Token &ref,
bool hasCallToGetArgs = m_hasCallToGetArgs.back();
m_hasCallToGetArgs.pop_back();
m_foreaches.pop_back();
m_pendingStatements.pop_back();
m_prependingStatements.pop_back();

if (ms->hasYield()) {
string closureName = getClosureName();
Expand Down Expand Up @@ -1261,17 +1261,18 @@ void Parser::addStatement(Token &out, Token &stmts, Token &new_stmt) {
ASSERT(stmts->stmt());
out->stmt() = stmts->stmt();
ASSERT(out->getStmtList());
if (new_stmt->stmt()) {
out->getStmtList()->add(new_stmt->stmt());
}

ASSERT(!m_pendingStatements.empty());
vector<StatementPtr> &pending = m_pendingStatements.back();
if (!pending.empty()) {
for (unsigned i = 0; i < pending.size(); i++) {
out->getStmtList()->add(pending[i]);
ASSERT(!m_prependingStatements.empty());
vector<StatementPtr> &prepending = m_prependingStatements.back();
if (!prepending.empty()) {
ASSERT(prepending.size() == 1);
for (unsigned i = 0; i < prepending.size(); i++) {
out->getStmtList()->add(prepending[i]);
}
pending.clear();
prepending.clear();
}
if (new_stmt->stmt()) {
out->getStmtList()->add(new_stmt->stmt());
}
}

Expand Down
2 changes: 1 addition & 1 deletion src/runtime/eval/parser/parser.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -308,7 +308,7 @@ class Parser : public ParserBase {
void pushFunc(FunctionStatementPtr fs);
void popFunc();
std::vector<bool> m_hasCallToGetArgs;
std::vector<std::vector<StatementPtr> > m_pendingStatements;
std::vector<std::vector<StatementPtr> > m_prependingStatements;

ExpressionPtr getDynamicVariable(ExpressionPtr exp, bool encap);
ExpressionPtr createDynamicVariable(ExpressionPtr exp);
Expand Down
17 changes: 17 additions & 0 deletions src/test/test_code_run.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17086,6 +17086,23 @@ bool TestCodeRun::TestYield() {
"int(2)\n"
"int(4)\n");

MVCRO("<?php\n"
"class A {"
" public function foo() {"
" $this->bar(function() {yield 1; yield 2; yield 3;});"
" }"
" public function bar(Closure $c) {"
" $a = $c();"
" foreach ($a as $b) {"
" echo $b.\"\\n\";"
" }"
" }"
"}"
"$a = new A();"
"$a->foo();",
"1\n"
"2\n"
"3\n");
return true;
}

Expand Down
16 changes: 8 additions & 8 deletions src/test/test_parser_stmt.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -662,10 +662,6 @@ bool TestParserStmt::TestYieldStatement() {
Option::EnableHipHopSyntax = true;

V("<?php function foo() { yield break;}",
"function foo() {\n"
"return new Continuation('02316968161694270338_1', "
"get_defined_vars());\n"
"}\n"
"function (Continuation $" CONTINUATION_OBJECT_NAME ") {\n"
"extract($" CONTINUATION_OBJECT_NAME "->getVars(), EXTR_REFS);\n"
"switch ($" CONTINUATION_OBJECT_NAME "->getLabel()) {\n"
Expand All @@ -678,14 +674,14 @@ bool TestParserStmt::TestYieldStatement() {
YIELD_LABEL_PREFIX "1:\n"
"$" CONTINUATION_OBJECT_NAME "->done();\n"
"}\n"
);

V("<?php function foo() { yield 123;}",

"function foo() {\n"
"return new Continuation('02316968161694270338_1', "
"get_defined_vars());\n"
"}\n"
);

V("<?php function foo() { yield 123;}",

"function (Continuation $" CONTINUATION_OBJECT_NAME ") {\n"
"extract($" CONTINUATION_OBJECT_NAME "->getVars(), EXTR_REFS);\n"
"switch ($" CONTINUATION_OBJECT_NAME "->getLabel()) {\n"
Expand All @@ -697,6 +693,10 @@ bool TestParserStmt::TestYieldStatement() {
YIELD_LABEL_PREFIX "1:\n"
"$" CONTINUATION_OBJECT_NAME "->done();\n"
"}\n"
"function foo() {\n"
"return new Continuation('02316968161694270338_1', "
"get_defined_vars());\n"
"}\n"
);

V("<?php class bar { function foo() { yield 123; yield 456;} }",
Expand Down

0 comments on commit 0071de0

Please sign in to comment.