diff --git a/hphp/compiler/analysis/class_scope.cpp b/hphp/compiler/analysis/class_scope.cpp index 67fd97245..7c52161ad 100644 --- a/hphp/compiler/analysis/class_scope.cpp +++ b/hphp/compiler/analysis/class_scope.cpp @@ -14,10 +14,8 @@ +----------------------------------------------------------------------+ */ -#include -#include -#include "hphp/compiler/analysis/analysis_result.h" #include "hphp/compiler/analysis/class_scope.h" +#include "hphp/compiler/analysis/analysis_result.h" #include "hphp/compiler/analysis/code_error.h" #include "hphp/compiler/analysis/constant_table.h" #include "hphp/compiler/analysis/file_scope.h" @@ -46,6 +44,9 @@ #include "hphp/runtime/base/zend/zend_string.h" #include "hphp/util/util.h" +#include +#include + using namespace HPHP; using std::map; @@ -757,8 +758,9 @@ const string& ClassScope::getNewGeneratorName( if (mapIt != genRenameMap.end()) { return mapIt->second; } - string newName = oldName + "_" + - lexical_cast(genFuncScope->getNewID()); + string newName = ParserBase::newContinuationName( + oldName + "_" + lexical_cast(genFuncScope->getNewID()) + ); genRenameMap[oldName] = newName; return genRenameMap[oldName]; } @@ -781,7 +783,7 @@ ClassScope::renameCreateContinuationCalls(AnalysisResultPtr ar, const string &oldGenName = dynamic_pointer_cast((*params)[1])->getString(); - MethodStatementPtr origGenStmt = importedMethods[oldGenName]; + MethodStatementPtr origGenStmt = importedMethods[Util::toLower(oldGenName)]; assert(origGenStmt); const string &newGenName = origGenStmt->getOriginalName(); diff --git a/hphp/compiler/analysis/emitter.cpp b/hphp/compiler/analysis/emitter.cpp index 145d9135d..6803c6823 100644 --- a/hphp/compiler/analysis/emitter.cpp +++ b/hphp/compiler/analysis/emitter.cpp @@ -1155,9 +1155,11 @@ void MetaInfoBuilder::setForUnit(UnitEmitter& target) const { free(meta); } +EmitterVisitor::EmittedClosures EmitterVisitor::s_emittedClosures; + EmitterVisitor::EmitterVisitor(UnitEmitter& ue) : m_ue(ue), m_curFunc(ue.getMain()), m_evalStackIsUnknown(false), - m_actualStackHighWater(0), m_fdescHighWater(0), m_closureCounter(0) { + m_actualStackHighWater(0), m_fdescHighWater(0) { m_prevOpcode = OpLowInvalid; m_evalStack.m_actualStackHighWaterPtr = &m_actualStackHighWater; m_evalStack.m_fdescHighWaterPtr = &m_fdescHighWater; @@ -3896,14 +3898,10 @@ bool EmitterVisitor::visitImpl(ConstructPtr node) { } } - StringData* className = newClosureName(); - const static StringData* parentName = - StringData::GetStaticString("Closure"); - const Location* sLoc = ce->getLocation().get(); - PreClassEmitter* pce = m_ue.newPreClassEmitter( - className, PreClass::AlwaysHoistable); - pce->init(sLoc->line0, sLoc->line1, m_ue.bcPos(), - AttrUnique | AttrPersistent, parentName, nullptr); + // The parser generated a unique name for the function, + // use that for the class + std::string clsName = ce->getClosureFunction()->getOriginalName(); + StringData* className = StringData::GetStaticString(clsName); // We're still at the closure definition site. Emit code to instantiate // the new anonymous class, with the use variables as arguments. @@ -3911,9 +3909,35 @@ bool EmitterVisitor::visitImpl(ConstructPtr node) { for (int i = 0; i < useCount; ++i) { emitBuiltinCallArg(e, (*valuesList)[i], i, useVars[i].second); } + + if (Option::WholeProgram) { + int my_id; + { + EmittedClosures::accessor acc; + s_emittedClosures.insert(acc, className); + my_id = ++acc->second; + } + if (my_id > 1) { + // The closure was from a trait, so we need a unique name in the + // implementing class + className = StringData::GetStaticString( + // _ is different from the #, which is used for many closures in + // the same func in ParserBase::newClosureName + className->toCPPString() + '_' + std::to_string(my_id) + ); + } + } + e.CreateCl(useCount, className); - // From here on out, we're just building metadata for the closure. + // From here on out, we're creating a new class to hold the closure. + const static StringData* parentName = + StringData::GetStaticString("Closure"); + const Location* sLoc = ce->getLocation().get(); + PreClassEmitter* pce = m_ue.newPreClassEmitter( + className, PreClass::AlwaysHoistable); + pce->init(sLoc->line0, sLoc->line1, m_ue.bcPos(), + AttrUnique | AttrPersistent, parentName, nullptr); // Instance variables. TypedValue uninit; @@ -6807,30 +6831,6 @@ void EmitterVisitor::finishFunc(Emitter& e, FuncEmitter* fe) { e.getUnitEmitter().recordFunction(fe); } -StringData* EmitterVisitor::newClosureName() { - std::ostringstream str; - str << "Closure" << '$'; - if (m_curFunc->pce() != nullptr) { - str << m_curFunc->pce()->name()->data(); - } - str << '$'; - if (m_curFunc->isPseudoMain()) { - str << "__pseudoMain"; - } else { - str << m_curFunc->name()->data(); - } - /* - * Uniquify the name - */ - str << '$' - << std::hex - << m_curFunc->ue().md5().q[1] << m_curFunc->ue().md5().q[0] - << std::dec - << '$' << m_closureCounter++; - - return StringData::GetStaticString(str.str()); -} - void EmitterVisitor::initScalar(TypedValue& tvVal, ExpressionPtr val) { assert(val->isScalar()); tvVal.m_type = KindOfUninit; diff --git a/hphp/compiler/analysis/emitter.h b/hphp/compiler/analysis/emitter.h index fc53e68d4..eff16dca7 100644 --- a/hphp/compiler/analysis/emitter.h +++ b/hphp/compiler/analysis/emitter.h @@ -538,7 +538,9 @@ private: hphp_hash_map m_jumpTargetEvalStacks; int m_actualStackHighWater; int m_fdescHighWater; - int m_closureCounter; // used to uniquify closures' mangled names + typedef tbb::concurrent_hash_map EmittedClosures; + static EmittedClosures s_emittedClosures; std::deque m_controlTargets; std::deque m_funclets; std::deque m_exnHandlers; @@ -678,7 +680,6 @@ public: void copyOverFPIRegions(FuncEmitter* fe); void saveMaxStackCells(FuncEmitter* fe); void finishFunc(Emitter& e, FuncEmitter* fe); - StringData* newClosureName(); void initScalar(TypedValue& tvVal, ExpressionPtr val); bool requiresDeepInit(ExpressionPtr initExpr) const; diff --git a/hphp/compiler/expression/scalar_expression.cpp b/hphp/compiler/expression/scalar_expression.cpp index 8d012edfd..c728a6f3f 100644 --- a/hphp/compiler/expression/scalar_expression.cpp +++ b/hphp/compiler/expression/scalar_expression.cpp @@ -14,9 +14,6 @@ +----------------------------------------------------------------------+ */ -#include -#include -#include #include "hphp/compiler/expression/scalar_expression.h" #include "hphp/util/parser/hphp.tab.hpp" #include "hphp/util/util.h" @@ -35,6 +32,10 @@ #include "hphp/runtime/ext/ext_variable.h" #include "hphp/compiler/analysis/file_scope.h" +#include +#include +#include + using namespace HPHP; /////////////////////////////////////////////////////////////////////////////// @@ -157,16 +158,21 @@ void ScalarExpression::analyzeProgram(AnalysisResultPtr ar) { if (b && b->is(BlockScope::ClassScope)) { m_translated += "::"; } - m_translated += func->getOriginalName(); + if (func->isClosure()) { + m_translated += "{closure}"; + } else { + m_translated += func->getOriginalName(); + } } } break; } case T_FUNC_C: - if (getFunctionScope()) { - m_translated = getFunctionScope()->getOriginalName(); - if (m_translated[0] == '0') { + if (FunctionScopePtr func = getFunctionScope()) { + if (func->isClosure()) { m_translated = "{closure}"; + } else { + m_translated = func->getOriginalName(); } } break; diff --git a/hphp/compiler/parser/parser.cpp b/hphp/compiler/parser/parser.cpp index a8ab64236..a202e4f07 100644 --- a/hphp/compiler/parser/parser.cpp +++ b/hphp/compiler/parser/parser.cpp @@ -113,7 +113,7 @@ using namespace HPHP::Compiler; extern void prepare_generator(Parser *_p, Token &stmt, Token ¶ms); extern void create_generator(Parser *_p, Token &out, Token ¶ms, - Token &name, const std::string &closureName, + Token &name, const std::string &genName, const char *clsname, Token *modifiers, Token &origGenFunc, bool isHhvm, Token *attr); @@ -825,14 +825,20 @@ void Parser::onFunction(Token &out, Token *modifiers, Token &ret, Token &ref, FunctionStatementPtr func; + string funcName = name->text(); + if (funcName.empty()) { + funcName = newClosureName(m_clsName, m_containingFuncName); + } else if (m_lambdaMode) { + funcName += "{lambda}"; + } + if (funcContext.isGenerator) { - AnonFuncKind fKind = name->text().empty() ? - ContinuationFromClosure : Continuation; - const string &closureName = getAnonFuncName(fKind); + string genName = newContinuationName(funcName); + Token new_params; prepare_generator(this, stmt, new_params); - func = NEW_STMT(FunctionStatement, exp, ref->num(), closureName, + func = NEW_STMT(FunctionStatement, exp, ref->num(), genName, dynamic_pointer_cast(new_params->exp), ret.typeAnnotationName(), dynamic_pointer_cast(stmt->stmt), @@ -857,7 +863,7 @@ void Parser::onFunction(Token &out, Token *modifiers, Token &ret, Token &ref, // the MethodStatement it's building will get the docComment pushComment(comment); Token origGenFunc; - create_generator(this, out, params, name, closureName, nullptr, nullptr, + create_generator(this, out, params, name, genName, nullptr, nullptr, origGenFunc, (!Option::WholeProgram || !Option::ParseTimeOpts), attr); @@ -872,15 +878,6 @@ void Parser::onFunction(Token &out, Token *modifiers, Token &ret, Token &ref, } } else { - string funcName = name->text(); - if (funcName.empty()) { - funcName = getAnonFuncName(Closure); - } else if (m_lambdaMode) { - string f; - f += GetAnonPrefix(CreateFunction); - funcName = f + "_" + funcName; - } - ExpressionListPtr attrList; if (attr && attr->exp) { attrList = dynamic_pointer_cast(attr->exp); @@ -1133,12 +1130,23 @@ void Parser::onMethod(Token &out, Token &modifiers, Token &ret, Token &ref, fixStaticVars(); MethodStatementPtr mth; + + string funcName = name->text(); + if (funcName.empty()) { + funcName = newClosureName(m_clsName, m_containingFuncName); + } + if (funcContext.isGenerator) { - const string &closureName = getAnonFuncName(ParserBase::Continuation); + string genName = newContinuationName(funcName); + if (m_inTrait) { + // see traits/2067.php + genName = newContinuationName(funcName + "@" + m_clsName); + } + Token new_params; prepare_generator(this, stmt, new_params); ModifierExpressionPtr exp2 = Construct::Clone(exp); - mth = NEW_STMT(MethodStatement, exp2, ref->num(), closureName, + mth = NEW_STMT(MethodStatement, exp2, ref->num(), genName, dynamic_pointer_cast(new_params->exp), ret.typeAnnotationName(), dynamic_pointer_cast(stmt->stmt), @@ -1156,7 +1164,7 @@ void Parser::onMethod(Token &out, Token &modifiers, Token &ret, Token &ref, // the MethodStatement it's building will get the docComment pushComment(comment); Token origGenFunc; - create_generator(this, out, params, name, closureName, m_clsName.c_str(), + create_generator(this, out, params, name, genName, m_clsName.c_str(), &modifiers, origGenFunc, (!Option::WholeProgram || !Option::ParseTimeOpts), attr); @@ -1172,7 +1180,7 @@ void Parser::onMethod(Token &out, Token &modifiers, Token &ret, Token &ref, if (attr && attr->exp) { attrList = dynamic_pointer_cast(attr->exp); } - mth = NEW_STMT(MethodStatement, exp, ref->num(), name->text(), + mth = NEW_STMT(MethodStatement, exp, ref->num(), funcName, old_params, ret.typeAnnotationName(), stmts, attribute, comment, @@ -1559,6 +1567,11 @@ void Parser::onThrow(Token &out, Token &expr) { } void Parser::onClosureStart(Token &name) { + if (!m_funcName.empty()) { + m_containingFuncName = m_funcName; + } else { + // pseudoMain + } onFunctionStart(name, true); } diff --git a/hphp/compiler/parser/parser.h b/hphp/compiler/parser/parser.h index bd503f0b6..582354507 100644 --- a/hphp/compiler/parser/parser.h +++ b/hphp/compiler/parser/parser.h @@ -304,6 +304,7 @@ private: std::vector m_compilerHaltOffsetVec; std::string m_clsName; // for T_CLASS_C inside a closure std::string m_funcName; + std::string m_containingFuncName; bool m_inTrait; // parser output diff --git a/hphp/runtime/ext/ext_fb.cpp b/hphp/runtime/ext/ext_fb.cpp index d32a01b0c..41061acf8 100644 --- a/hphp/runtime/ext/ext_fb.cpp +++ b/hphp/runtime/ext/ext_fb.cpp @@ -1455,8 +1455,7 @@ bool f_fb_rename_function(CStrRef orig_func_name, CStrRef new_func_name) { } if (function_exists(new_func_name)) { - if (new_func_name.data()[0] != - ParserBase::CharCreateFunction) { // create_function + if (new_func_name.data()[0] != '1') { raise_warning("fb_rename_function(%s, %s) failed: %s already exists!", orig_func_name.data(), new_func_name.data(), new_func_name.data()); diff --git a/hphp/runtime/ext/ext_reflection.cpp b/hphp/runtime/ext/ext_reflection.cpp index 2ab06aa4d..f83f02eb1 100644 --- a/hphp/runtime/ext/ext_reflection.cpp +++ b/hphp/runtime/ext/ext_reflection.cpp @@ -22,6 +22,7 @@ #include "hphp/runtime/base/runtime_option.h" #include "hphp/runtime/base/string_util.h" #include "hphp/runtime/vm/jit/translator-inline.h" +#include "hphp/util/parser/parser.h" #include "hphp/system/lib/systemlib.h" @@ -746,7 +747,10 @@ Array f_hphp_get_class_info(CVarRef name) { size_t const numMethods = cls->preClass()->numMethods(); for (Slot i = 0; i < numMethods; ++i) { const Func* m = methods[i]; - if (isdigit(m->name()->data()[0])) continue; + if (isdigit(m->name()->data()[0]) || + ParserBase::IsClosureOrContinuationName(m->name()->toCPPString())) { + continue; + } Array info = Array::Create(); set_method_info(info, m); arr.set(StringUtil::ToLower(m->nameRef()), VarNR(info)); @@ -757,7 +761,10 @@ Array f_hphp_get_class_info(CVarRef name) { i < cls->traitsEndIdx(); ++i) { const Func* m = clsMethods[i]; - if (isdigit(m->name()->data()[0])) continue; + if (isdigit(m->name()->data()[0]) || + ParserBase::IsClosureOrContinuationName(m->name()->toCPPString())) { + continue; + } Array info = Array::Create(); set_method_info(info, m); arr.set(StringUtil::ToLower(m->nameRef()), VarNR(info)); diff --git a/hphp/runtime/vm/class.cpp b/hphp/runtime/vm/class.cpp index 84bcb2be5..96547edf8 100644 --- a/hphp/runtime/vm/class.cpp +++ b/hphp/runtime/vm/class.cpp @@ -32,6 +32,7 @@ #include "hphp/runtime/vm/request_arena.h" #include "hphp/system/lib/systemlib.h" #include "hphp/util/logger.h" +#include "hphp/util/parser/parser.h" #include #include @@ -2367,7 +2368,10 @@ void Class::getMethodNames(const Class* ctx, HphpArray* methods) const { Func* const* pcMethods = m_preClass->methods(); for (size_t i = 0, sz = m_preClass->numMethods(); i < sz; i++) { Func* func = pcMethods[i]; - if (isdigit(func->name()->data()[0])) continue; + if (isdigit(func->name()->data()[0]) || + ParserBase::IsClosureOrContinuationName(func->name()->toCPPString())) { + continue; + } if (!(func->attrs() & AttrPublic)) { if (!ctx) continue; if (ctx != this) { diff --git a/hphp/runtime/vm/unit.cpp b/hphp/runtime/vm/unit.cpp index 5f9a591d1..385a89a9f 100644 --- a/hphp/runtime/vm/unit.cpp +++ b/hphp/runtime/vm/unit.cpp @@ -27,6 +27,7 @@ #include "hphp/util/util.h" #include "hphp/util/atomic.h" #include "hphp/util/read_only_arena.h" +#include "hphp/util/parser/parser.h" #include "hphp/runtime/ext/ext_variable.h" #include "hphp/runtime/vm/bytecode.h" @@ -185,7 +186,9 @@ Array Unit::getUserFunctions() { it != s_namedDataMap->end(); ++it) { Func* func_ = it->second.getCachedFunc(); if (!func_ || func_->isBuiltin() || - isdigit(func_->name()->data()[0])) { + isdigit(func_->name()->data()[0]) || + ParserBase::IsClosureOrContinuationName( + func_->name()->toCPPString())) { continue; } a.append(func_->nameRef()); diff --git a/hphp/test/ext/test_parser.cpp b/hphp/test/ext/test_parser.cpp index b8d88cd39..b6268b0a8 100644 --- a/hphp/test/ext/test_parser.cpp +++ b/hphp/test/ext/test_parser.cpp @@ -56,7 +56,6 @@ bool TestParser::VerifyParser(const char *input, const char *output, bool ret = true; { AnalysisResultPtr ar(new AnalysisResult()); - Compiler::Parser::Reset(); StatementListPtr tree = Compiler::Parser::ParseString(input, ar); std::ostringstream code; CodeGenerator cg(&code); diff --git a/hphp/test/ext/test_parser_stmt.cpp b/hphp/test/ext/test_parser_stmt.cpp index 491a1d295..42ce37936 100644 --- a/hphp/test/ext/test_parser_stmt.cpp +++ b/hphp/test/ext/test_parser_stmt.cpp @@ -666,7 +666,7 @@ bool TestParserStmt::TestYieldStatement() { "}\n" "function foo() {\n" "return hphp_create_continuation" - "('', '3990978909_1', __FUNCTION__);\n" + "('', 'foo$continuation', __FUNCTION__);\n" "}\n"); V(" string(5) "enter" [1]=> - string(%d) "%s" + string(16) "gen$continuation" [2]=> array(1) { ["args"]=> @@ -585,7 +585,7 @@ array(3) { [0]=> string(4) "exit" [1]=> - string(%d) "%s" + string(16) "gen$continuation" [2]=> NULL } @@ -670,7 +670,7 @@ array(3) { [0]=> string(5) "enter" [1]=> - string(%d) "%s" + string(16) "gen$continuation" [2]=> array(1) { ["args"]=> @@ -685,7 +685,7 @@ array(3) { [0]=> string(4) "exit" [1]=> - string(%d) "%s" + string(16) "gen$continuation" [2]=> NULL } @@ -762,7 +762,7 @@ array(3) { [0]=> string(5) "enter" [1]=> - string(%d) "%s" + string(16) "gen$continuation" [2]=> array(1) { ["args"]=> @@ -777,7 +777,7 @@ array(3) { [0]=> string(4) "exit" [1]=> - string(%d) "%s" + string(16) "gen$continuation" [2]=> NULL } @@ -819,7 +819,7 @@ array(3) { [0]=> string(4) "exit" [1]=> - string(%d) "%s" + string(16) "gen$continuation" [2]=> NULL } diff --git a/hphp/test/quick/debugger/flow2.php.expectf b/hphp/test/quick/debugger/flow2.php.expectf index 9d1366990..6ad396ea4 100644 --- a/hphp/test/quick/debugger/flow2.php.expectf +++ b/hphp/test/quick/debugger/flow2.php.expectf @@ -111,7 +111,7 @@ break flow2.php:12 Breakpoint 2 set on line 12 of flow2.php @test(2) Constructor -Breakpoint 1 reached at %d_%d() on line 10 of %s/flow2.php +Breakpoint 1 reached at genFoo$continuation() on line 10 of %s/flow2.php 9 function genFoo($a) { 10 $a = bar($a); 11 $z = yield $a+5; @@ -123,7 +123,7 @@ Break at foo() on line 40 of %s/flow2.php 41 var_dump($x); continue -Breakpoint 2 reached at %d_%d() on line 12 of %s/flow2.php +Breakpoint 2 reached at genFoo$continuation() on line 12 of %s/flow2.php 11 $z = yield $a+5; 12 yield $z+1; 13 error_log('Finished in genFoo'); diff --git a/hphp/test/quick/debugger/flow_gen.php.expectf b/hphp/test/quick/debugger/flow_gen.php.expectf index e8885c3a3..ee50056bb 100644 --- a/hphp/test/quick/debugger/flow_gen.php.expectf +++ b/hphp/test/quick/debugger/flow_gen.php.expectf @@ -5,13 +5,13 @@ Program %s/flow_gen.php exited normally. break flow_gen.php:12 Breakpoint 1 set on line 12 of flow_gen.php @test(1) -Breakpoint 1 reached at %d_%d() on line 12 of %s/flow_gen.php +Breakpoint 1 reached at genFoo$continuation() on line 12 of %s/flow_gen.php 11 function genFoo($a) { 12 $a = bar($a); 13 $z = yield $a+5; next -Break at %d_%d() on line 13 of %s/flow_gen.php +Break at genFoo$continuation() on line 13 of %s/flow_gen.php 12 $a = bar($a); 13 $z = yield $a+5; 14 yield $z+1; @@ -51,7 +51,7 @@ Break at Continuation::send() step Break at Continuation::send() step -Break at %d_%d() on line 11 of %s/flow_gen.php +Break at genFoo$continuation() on line 11 of %s/flow_gen.php 10 11 function genFoo($a) { 12 $a = bar($a); @@ -62,13 +62,13 @@ Break at %d_%d() on line 11 of %s/flow_gen.php 17 step -Break at %d_%d() on line 13 of %s/flow_gen.php +Break at genFoo$continuation() on line 13 of %s/flow_gen.php 12 $a = bar($a); 13 $z = yield $a+5; 14 yield $z+1; next -Break at %d_%d() on line 14 of %s/flow_gen.php +Break at genFoo$continuation() on line 14 of %s/flow_gen.php 13 $z = yield $a+5; 14 yield $z+1; 15 error_log('Finished in genFoo'); @@ -108,7 +108,7 @@ Break at Continuation::send() step Break at Continuation::send() step -Break at %d_%d() on line 11 of %s/flow_gen.php +Break at genFoo$continuation() on line 11 of %s/flow_gen.php 10 11 function genFoo($a) { 12 $a = bar($a); @@ -119,20 +119,20 @@ Break at %d_%d() on line 11 of %s/flow_gen.php 17 step -Break at %d_%d() on line 14 of %s/flow_gen.php +Break at genFoo$continuation() on line 14 of %s/flow_gen.php 13 $z = yield $a+5; 14 yield $z+1; 15 error_log('Finished in genFoo'); next -Break at %d_%d() on line 15 of %s/flow_gen.php +Break at genFoo$continuation() on line 15 of %s/flow_gen.php 14 yield $z+1; 15 error_log('Finished in genFoo'); 16 } next Finished in genFoo -Break at %d_%d() on line 11 of %s/flow_gen.php +Break at genFoo$continuation() on line 11 of %s/flow_gen.php 10 11 function genFoo($a) { 12 $a = bar($a); diff --git a/hphp/test/quick/generator_method.php b/hphp/test/quick/generator_method.php index 8b52f260a..7707504ef 100644 --- a/hphp/test/quick/generator_method.php +++ b/hphp/test/quick/generator_method.php @@ -1,12 +1,12 @@ gen() as $num) { var_dump($num); } -foreach (A::sgen() as $num) { var_dump($num); } +foreach ($a->Gen() as $num) { var_dump($num); } +foreach (A::SGen() as $num) { var_dump($num); } diff --git a/hphp/test/quick/trait_two_use_closure.php b/hphp/test/quick/trait_two_use_closure.php new file mode 100644 index 000000000..1b74b0b50 --- /dev/null +++ b/hphp/test/quick/trait_two_use_closure.php @@ -0,0 +1,21 @@ +b()); + var_dump((new F)->b()); +} + +main(); diff --git a/hphp/test/quick/trait_two_use_closure.php.expectf b/hphp/test/quick/trait_two_use_closure.php.expectf new file mode 100644 index 000000000..99681ba1c --- /dev/null +++ b/hphp/test/quick/trait_two_use_closure.php.expectf @@ -0,0 +1,6 @@ +object(Closure$A::b%S)#2 (0) { +} +string(1) "d" +object(Closure$A::b%S)#3 (0) { +} +string(1) "d" diff --git a/hphp/test/quick/yield_final.php.expectf b/hphp/test/quick/yield_final.php.expectf index 1267210b8..21401efe9 100644 --- a/hphp/test/quick/yield_final.php.expectf +++ b/hphp/test/quick/yield_final.php.expectf @@ -1 +1 @@ -HipHop Fatal error: Cannot override final method A::method1() in %s on line 4 +HipHop Fatal error: Cannot override final method A::%Smethod1%S() in %s on line 4 diff --git a/hphp/test/quick/zend_closure_020.php.expectf b/hphp/test/quick/zend_closure_020.php.expectf index e9a0bdf90..d677c2598 100644 --- a/hphp/test/quick/zend_closure_020.php.expectf +++ b/hphp/test/quick/zend_closure_020.php.expectf @@ -2,8 +2,8 @@ object(foo)#1 (2) { ["test":"foo":private]=> int(3) ["a"]=> - object(Closure$foo$x$%s$0)#2 (1) { - ["a":"Closure$foo$x$%s$0":private]=> + object(Closure$foo::x)#2 (1) { + ["a":"Closure$foo::x":private]=> *RECURSION* } } diff --git a/hphp/util/parser/parser.cpp b/hphp/util/parser/parser.cpp index c15c1f6ef..abbbe3e82 100644 --- a/hphp/util/parser/parser.cpp +++ b/hphp/util/parser/parser.cpp @@ -16,58 +16,49 @@ #include "parser.h" #include "hphp/util/hash.h" +#include namespace HPHP { /////////////////////////////////////////////////////////////////////////////// -Mutex ParserBase::s_mutex; -std::map ParserBase::s_closureIds; - -char ParserBase::GetAnonPrefix(AnonFuncKind kind) { - static_assert(Closure == 0 && Continuation <= 9, - "AnonFuncKind enum has unexpected values"); - static_assert(CharClosure == '0' && CharContinuation <= '9', - "AnonFuncKindChar enum has unexpected values"); - return '0' + kind; -} - -template -static bool NameImpl(const std::string &name) { - return !name.empty() && isdigit(name[0]) && i == (name[0] - '0'); -} - bool ParserBase::IsClosureName(const std::string &name) { - return NameImpl(name); -} - -bool ParserBase::IsCreateFunctionName(const std::string &name) { - return NameImpl(name); + return boost::istarts_with(name, "closure$"); } bool ParserBase::IsContinuationName(const std::string &name) { - return NameImpl(name) || - NameImpl(name); -} - -bool ParserBase::IsContinuationFromClosureName(const std::string &name) { - return NameImpl(name); + return boost::iends_with(name, "$continuation"); } bool ParserBase::IsClosureOrContinuationName(const std::string &name) { return IsClosureName(name) || IsContinuationName(name); } -bool ParserBase::IsAnonFunctionName(const char *name) { - if (!*name) return true; - char begin = CharClosure; - char end = CharContinuation; - char test = name[0]; - return begin <= test && test <= end; +std::string ParserBase::newContinuationName(const std::string &name) { + assert(!name.empty()); + size_t pos = 0; + std::string shorterName = name; + std::string suffix("$continuation"); + while((pos = shorterName.find(suffix, pos)) != std::string::npos) { + shorterName.replace(pos, suffix.length(), ""); + } + return shorterName + suffix; } -void ParserBase::Reset() { - Lock lock(s_mutex); - s_closureIds.clear(); +std::string ParserBase::newClosureName( + const std::string &className, + const std::string &funcName) { + std::string name = "Closure$"; + if (!className.empty()) { + name += className + "::"; + } + name += funcName; + + int id = ++m_seenClosures[name]; + if (id > 1) { + // we've seen the same name before, uniquify + name = name + '#' + std::to_string(id); + } + return name; } /////////////////////////////////////////////////////////////////////////////// @@ -145,23 +136,6 @@ void ParserBase::popClass() { m_classes.pop_back(); } -std::string ParserBase::getAnonFuncName(AnonFuncKind kind) { - int64_t h = hash_string_cs(m_fileName, strlen(m_fileName)); - int closureId; - { - Lock lock(s_mutex); - int &id = s_closureIds[h]; - closureId = ++id; - } - - string ret; - ret += GetAnonPrefix(kind); - ret += boost::lexical_cast(h); - ret += "_"; - ret += boost::lexical_cast(closureId); - return ret; -} - /////////////////////////////////////////////////////////////////////////////// // typevar scopes diff --git a/hphp/util/parser/parser.h b/hphp/util/parser/parser.h index 5dcc20d41..bd4456260 100644 --- a/hphp/util/parser/parser.h +++ b/hphp/util/parser/parser.h @@ -57,39 +57,13 @@ public: StaticName }; - /** - * These numbers are scattered throughout the code base (often hardcoded). - * Do not change unless you change all the occurances. - */ - enum AnonFuncKind { - Closure, - CreateFunction, - ContinuationFromClosure, - Continuation - }; - - enum AnonFuncKindChar { - CharClosure = '0', - CharCreateFunction, - CharContinuationFromClosure, - CharContinuation - }; - - static char GetAnonPrefix(AnonFuncKind kind); - static bool IsClosureName (const std::string &name); - static bool IsCreateFunctionName (const std::string &name); static bool IsContinuationName (const std::string &name); - static bool IsContinuationFromClosureName(const std::string &name); static bool IsClosureOrContinuationName (const std::string &name); - static bool IsAnonFunctionName (const std::string &name) { - return IsAnonFunctionName(name.c_str()); - } - static bool IsAnonFunctionName (const char *name); - /** - * Reset parser static variables. Good for unit tests. - */ - static void Reset(); + static std::string newContinuationName (const std::string &name); + std::string newClosureName( + const std::string &className, + const std::string &funcName); public: ParserBase(Scanner &scanner, const char *fileName); @@ -145,7 +119,6 @@ public: void pushClass(bool isXhpClass); bool peekClass(); void popClass(); - std::string getAnonFuncName(AnonFuncKind kind); // for typevar checking void pushTypeScope(); @@ -227,10 +200,6 @@ protected: std::string m_namespace; // current namespace hphp_string_imap m_aliases; hphp_string_imap m_seenClosures; - - // for closure hidden name - static Mutex s_mutex; - static std::map s_closureIds; }; ///////////////////////////////////////////////////////////////////////////////