name anonymous continuations as Class::Method$continuation

As a user, this often annoyed me that I couldn't get any debugging info
out of the name and then @ahupp asked for it.

I didn't really know what to name closure ones as the name is done at
emission time. So I went with ##{closure}##. I'm not sure how I feel
about that since it will be uncorrelated with the emitted name.

While I was in there, I deleted a lot of unused code.

I'm totally open to other names and ideas.

hphp/compiler/analysis/class_scope.cpp

@@ -754,8 +754,8 @@ const string& ClassScope::getNewGeneratorName(
   if (mapIt != genRenameMap.end()) {
     return mapIt->second;
   }
-  string newName = oldName + "_" +
-    lexical_cast<string>(genFuncScope->getNewID());
+  string newName = lexical_cast<string>(genFuncScope->getNewID()) +
+    "_" + oldName;
   genRenameMap[oldName] = newName;
   return genRenameMap[oldName];
 }
@@ -778,7 +778,7 @@ ClassScope::renameCreateContinuationCalls(AnalysisResultPtr ar,
     const string &oldGenName =
       dynamic_pointer_cast<ScalarExpression>((*params)[1])->getString();
 
-    MethodStatementPtr origGenStmt = importedMethods[oldGenName];
+    MethodStatementPtr origGenStmt = importedMethods[Util::toLower(oldGenName)];
     assert(origGenStmt);
 
     const string &newGenName = origGenStmt->getOriginalName();

hphp/compiler/parser/parser.cpp

@@ -819,9 +819,13 @@ void Parser::onFunction(Token &out, Token *modifiers, Token &ret, Token &ref,
   FunctionStatementPtr func;
 
   if (funcContext.isGenerator) {
-    AnonFuncKind fKind = name->text().empty() ?
-      ContinuationFromClosure : Continuation;
-    const string &closureName = getAnonFuncName(fKind);
+    string closureName = "0_";
+    if (m_inTrait) {
+      // need to de-dupe
+      closureName += m_clsName + "_";
+    }
+    closureName += getContinuationName(name->text());
+
     Token new_params;
     prepare_generator(this, stmt, new_params);
 
@@ -865,11 +869,9 @@ void Parser::onFunction(Token &out, Token *modifiers, Token &ret, Token &ref,
   } else {
     string funcName = name->text();
     if (funcName.empty()) {
-      funcName = getAnonFuncName(Closure);
+      funcName = "{closure}";
     } else if (m_lambdaMode) {
-      string f;
-      f += GetAnonPrefix(CreateFunction);
-      funcName = f + "_" + funcName;
+      funcName += "{lambda}";
     }
 
     ExpressionListPtr attrList;
@@ -1118,8 +1120,15 @@ void Parser::onMethod(Token &out, Token &modifiers, Token &ret, Token &ref,
   fixStaticVars();
 
   MethodStatementPtr mth;
+
   if (funcContext.isGenerator) {
-    const string &closureName = getAnonFuncName(ParserBase::Continuation);
+    string closureName = "0_";
+    if (m_inTrait) {
+      // need to de-dupe
+      closureName += m_clsName + "_";
+    }
+    closureName += getContinuationName(name->text());
+
     Token new_params;
     prepare_generator(this, stmt, new_params);
     ModifierExpressionPtr exp2 = Construct::Clone(exp);

hphp/runtime/ext/ext_fb.cpp

@@ -1446,8 +1446,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());

hphp/test/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);

hphp/test/test_parser_stmt.cpp

@@ -663,7 +663,7 @@ bool TestParserStmt::TestYieldStatement() {
     "}\n"
     "function foo() {\n"
     "return hphp_create_continuation"
-    "('', '3990978909_1', __FUNCTION__);\n"
+    "('', '0_foo$continuation', __FUNCTION__);\n"
     "}\n");
 
   V("<?php function foo() { yield 123;}",
@@ -673,14 +673,14 @@ bool TestParserStmt::TestYieldStatement() {
     "}\n"
     "function foo() {\n"
     "return hphp_create_continuation"
-    "('', '3990978909_1', __FUNCTION__);\n"
+    "('', '0_foo$continuation', __FUNCTION__);\n"
     "}\n");
 
   V("<?php class bar { function foo() { yield 123; yield 456;} }",
     "class bar {\n"
     "public function foo() {\n"
     "return hphp_create_continuation"
-    "(__CLASS__, '3990978909_1', __METHOD__);\n"
+    "(__CLASS__, '0_foo$continuation', __METHOD__);\n"
     "}\n"
     "public function ($" CONTINUATION_OBJECT_NAME ") {\n"
     "hphp_unpack_continuation();\n"

hphp/test/vm/Setprofile.php.expectf

@@ -574,7 +574,7 @@ array(3) {
   [0]=>
   string(5) "enter"
   [1]=>
-  string(%d) "%s"
+  string(18) "0_gen$continuation"
   [2]=>
   array(1) {
     ["args"]=>
@@ -589,7 +589,7 @@ array(3) {
   [0]=>
   string(4) "exit"
   [1]=>
-  string(%d) "%s"
+  string(18) "0_gen$continuation"
   [2]=>
   NULL
 }
@@ -674,7 +674,7 @@ array(3) {
   [0]=>
   string(5) "enter"
   [1]=>
-  string(%d) "%s"
+  string(18) "0_gen$continuation"
   [2]=>
   array(1) {
     ["args"]=>
@@ -689,7 +689,7 @@ array(3) {
   [0]=>
   string(4) "exit"
   [1]=>
-  string(%d) "%s"
+  string(18) "0_gen$continuation"
   [2]=>
   NULL
 }
@@ -766,7 +766,7 @@ array(3) {
   [0]=>
   string(5) "enter"
   [1]=>
-  string(%d) "%s"
+  string(18) "0_gen$continuation"
   [2]=>
   array(1) {
     ["args"]=>
@@ -781,7 +781,7 @@ array(3) {
   [0]=>
   string(4) "exit"
   [1]=>
-  string(%d) "%s"
+  string(18) "0_gen$continuation"
   [2]=>
   NULL
 }
@@ -823,7 +823,7 @@ array(3) {
   [0]=>
   string(4) "exit"
   [1]=>
-  string(%d) "%s"
+  string(18) "0_gen$continuation"
   [2]=>
   NULL
 }

hphp/test/vm/generator_method.php

@@ -1,12 +1,12 @@
 <?php
 
 class A {
-  public function gen() {
+  public function Gen() {
     var_dump($this);
     yield 1; yield 2; yield 3;
   }
 
-  public static function sgen() {
+  public static function SGen() {
     var_dump(get_called_class());
     yield 4; yield 5; yield 6;
   }
@@ -14,5 +14,5 @@ class A {
 
 
 $a = new A();
-foreach ($a->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); }

hphp/util/parser/parser.cpp

@@ -15,59 +15,36 @@
 */
 
 #include "parser.h"
+#include <atomic>
 #include <util/hash.h>
 
 namespace HPHP {
 ///////////////////////////////////////////////////////////////////////////////
 
-Mutex ParserBase::s_mutex;
-std::map<int64_t, int> 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 <int i>
-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<Closure>(name);
-}
-
-bool ParserBase::IsCreateFunctionName(const std::string &name) {
-  return NameImpl<CreateFunction>(name);
+  return name.compare("{closure}") == 0;
 }
 
 bool ParserBase::IsContinuationName(const std::string &name) {
-  return NameImpl<ContinuationFromClosure>(name) ||
-         NameImpl<Continuation>(name);
-}
-
-bool ParserBase::IsContinuationFromClosureName(const std::string &name) {
-  return NameImpl<ContinuationFromClosure>(name);
+  return name.rfind("$continuation") != std::string::npos;
 }
 
 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::getContinuationName(const std::string &name) {
+  static std::atomic<int32_t> id(0);
+  std::string prefix;
 
-void ParserBase::Reset() {
-  Lock lock(s_mutex);
-  s_closureIds.clear();
+  if (name.empty()) {
+    // The real name will be done at emission time so we can't know it here :(
+    prefix = "Closure_" + std::to_string(id++);
+  } else {
+    prefix = name;
+  }
+
+  return prefix + "$continuation";
 }
 
 ///////////////////////////////////////////////////////////////////////////////
@@ -144,23 +121,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<string>(h);
-  ret += "_";
-  ret += boost::lexical_cast<string>(closureId);
-  return ret;
-}
-
 ///////////////////////////////////////////////////////////////////////////////
 // typevar scopes
 

hphp/util/parser/parser.h

@@ -57,39 +57,10 @@ 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();
+  std::string getContinuationName(const std::string &name);
 
 public:
   ParserBase(Scanner &scanner, const char *fileName);
@@ -149,7 +120,6 @@ public:
   void pushClass(bool isXhpClass);
   bool peekClass();
   void popClass();
-  std::string getAnonFuncName(AnonFuncKind kind);
 
   // for typevar checking
   void pushTypeScope();
@@ -237,10 +207,6 @@ protected:
   NamespaceState m_nsState;
   std::string m_namespace; // current namespace
   hphp_string_imap<std::string> m_aliases;
-
-  // for closure hidden name
-  static Mutex s_mutex;
-  static std::map<int64_t, int> s_closureIds;
 };
 
 ///////////////////////////////////////////////////////////////////////////////