Fix order dependencies in BuiltinSymbols::Load

We need to import the c++ classes and constants before parsing
systemlib.php so that static analysis is aware of them.

hphp/compiler/analysis/analysis_result.cpp

@@ -557,11 +557,6 @@ bool AnalysisResult::isSystemConstant(const std::string &constName) const {
 ///////////////////////////////////////////////////////////////////////////////
 // Program
 
-void AnalysisResult::loadBuiltinFunctions() {
-  AnalysisResultPtr ar = shared_from_this();
-  BuiltinSymbols::LoadFunctions(ar, m_functions);
-}
-
 void AnalysisResult::loadBuiltins() {
   AnalysisResultPtr ar = shared_from_this();
   BuiltinSymbols::LoadFunctions(ar, m_functions);

hphp/compiler/analysis/analysis_result.h

@@ -150,7 +150,6 @@ public:
 
   void addNSFallbackFunc(ConstructPtr c, FileScopePtr fs);
 
-  void loadBuiltinFunctions();
   void loadBuiltins();
   void analyzeProgram(bool system = false);
   void analyzeIncludes();

hphp/compiler/analysis/emitter.cpp

@@ -7509,7 +7509,6 @@ Unit* hphp_compiler_parse(const char* code, int codeLen, const MD5& md5,
     Option::WholeProgram = false;
     Type::InitTypeHintMap();
     BuiltinSymbols::LoadSuperGlobals();
-    BuiltinSymbols::NoSuperGlobals = false;
     TypeConstraint tc;
     return nullptr;
   }

hphp/compiler/builtin_symbols.cpp

@@ -49,7 +49,6 @@ using namespace HPHP;
 ///////////////////////////////////////////////////////////////////////////////
 
 bool BuiltinSymbols::Loaded = false;
-bool BuiltinSymbols::NoSuperGlobals = false;
 StringBag BuiltinSymbols::s_strings;
 
 StringToFunctionScopePtrMap BuiltinSymbols::s_functions;
@@ -315,7 +314,7 @@ void BuiltinSymbols::ImportExtClasses(AnalysisResultPtr ar) {
   }
 }
 
-bool BuiltinSymbols::Load(AnalysisResultPtr ar, bool extOnly /* = false */) {
+bool BuiltinSymbols::Load(AnalysisResultPtr ar) {
   if (Loaded) return true;
   Loaded = true;
 
@@ -328,82 +327,77 @@ bool BuiltinSymbols::Load(AnalysisResultPtr ar, bool extOnly /* = false */) {
   s_variables = VariableTablePtr(new VariableTable(*ar2.get()));
   s_constants = ConstantTablePtr(new ConstantTable(*ar2.get()));
 
-  // parse all PHP files under system/classes
-  if (!extOnly) {
-    s_systemAr = ar = AnalysisResultPtr(new AnalysisResult());
-    ar->loadBuiltinFunctions();
-    string slib = get_systemlib();
-
-    Scanner scanner(slib.c_str(), slib.size(),
-                    Option::GetScannerType(), "systemlib.php");
-    Compiler::Parser parser(scanner, "systemlib.php", ar);
-    if (!parser.parse()) {
-      Logger::Error("Unable to parse systemlib.php: %s",
-                    parser.getMessage().c_str());
-      assert(false);
-    }
-
-    ar->analyzeProgram(true);
-    ar->inferTypes();
-    const StringToFileScopePtrMap &files = ar->getAllFiles();
-    for (StringToFileScopePtrMap::const_iterator iterFile = files.begin();
-         iterFile != files.end(); iterFile++) {
-      const StringToClassScopePtrVecMap &classes =
-        iterFile->second->getClasses();
-      for (StringToClassScopePtrVecMap::const_iterator iter = classes.begin();
-           iter != classes.end(); ++iter) {
-        assert(iter->second.size() == 1);
-        iter->second[0]->setSystem();
-        assert(!s_classes[iter->first]);
-        s_classes[iter->first] = iter->second[0];
-      }
-
-      const StringToFunctionScopePtrMap &functions =
-        iterFile->second->getFunctions();
-      for (StringToFunctionScopePtrMap::const_iterator iter = functions.begin();
-           iter != functions.end(); ++iter) {
-        iter->second->setSystem();
-        s_functions[iter->first] = iter->second;
-      }
-    }
-  } else {
-    NoSuperGlobals = true;
-  }
-
   // load extension constants, classes and dynamics
   ImportExtConstants(ar, s_constants, ClassInfo::GetSystem());
   ImportExtClasses(ar);
 
-  if (!extOnly) {
-    Array constants = ClassInfo::GetSystemConstants();
-    LocationPtr loc(new Location);
-    for (ArrayIter it = constants.begin(); it; ++it) {
-      CVarRef key = it.first();
-      if (!key.isString()) continue;
-      std::string name = key.toCStrRef().data();
-      if (s_constants->getSymbol(name)) continue;
-      if (name == "true" || name == "false" || name == "null") continue;
-      CVarRef value = it.secondRef();
-      if (!value.isInitialized() || value.isObject()) continue;
-      ExpressionPtr e = Expression::MakeScalarExpression(ar2, ar2, loc, value);
-      TypePtr t =
-        value.isNull()    ? Type::Null    :
-        value.isBoolean() ? Type::Boolean :
-        value.isInteger() ? Type::Int64   :
-        value.isDouble()  ? Type::Double  :
-        value.isArray()   ? Type::Array   : Type::Variant;
+  Array constants = ClassInfo::GetSystemConstants();
+  LocationPtr loc(new Location);
+  for (ArrayIter it = constants.begin(); it; ++it) {
+    CVarRef key = it.first();
+    if (!key.isString()) continue;
+    std::string name = key.toCStrRef().data();
+    if (s_constants->getSymbol(name)) continue;
+    if (name == "true" || name == "false" || name == "null") continue;
+    CVarRef value = it.secondRef();
+    if (!value.isInitialized() || value.isObject()) continue;
+    ExpressionPtr e = Expression::MakeScalarExpression(ar2, ar2, loc, value);
+    TypePtr t =
+      value.isNull()    ? Type::Null    :
+      value.isBoolean() ? Type::Boolean :
+      value.isInteger() ? Type::Int64   :
+      value.isDouble()  ? Type::Double  :
+      value.isArray()   ? Type::Array   : Type::Variant;
 
-      s_constants->add(key.toCStrRef().data(), t, e, ar2, e);
-    }
-    s_variables = ar2->getVariables();
-    for (int i = 0, n = NumGlobalNames(); i < n; ++i) {
-      s_variables->add(GlobalNames[i], Type::Variant, false, ar,
-                       ConstructPtr(), ModifierExpressionPtr());
-    }
+    s_constants->add(key.toCStrRef().data(), t, e, ar2, e);
   }
+  s_variables = ar2->getVariables();
+  for (int i = 0, n = NumGlobalNames(); i < n; ++i) {
+    s_variables->add(GlobalNames[i], Type::Variant, false, ar,
+                     ConstructPtr(), ModifierExpressionPtr());
+  }
+
   s_constants->setDynamic(ar, "SID", true);
   s_constants->setDynamic(ar, "PHP_SAPI", true);
 
+  // parse all PHP files under system/classes
+  s_systemAr = ar = AnalysisResultPtr(new AnalysisResult());
+  ar->loadBuiltins();
+  string slib = get_systemlib();
+
+  Scanner scanner(slib.c_str(), slib.size(),
+                  Option::GetScannerType(), "systemlib.php");
+  Compiler::Parser parser(scanner, "systemlib.php", ar);
+  if (!parser.parse()) {
+    Logger::Error("Unable to parse systemlib.php: %s",
+                  parser.getMessage().c_str());
+    assert(false);
+  }
+
+  ar->analyzeProgram(true);
+  ar->inferTypes();
+  const StringToFileScopePtrMap &files = ar->getAllFiles();
+  for (StringToFileScopePtrMap::const_iterator iterFile = files.begin();
+       iterFile != files.end(); iterFile++) {
+    const StringToClassScopePtrVecMap &classes =
+      iterFile->second->getClasses();
+    for (StringToClassScopePtrVecMap::const_iterator iter = classes.begin();
+         iter != classes.end(); ++iter) {
+      assert(iter->second.size() == 1);
+      iter->second[0]->setSystem();
+      assert(!s_classes[iter->first]);
+      s_classes[iter->first] = iter->second[0];
+    }
+
+    const StringToFunctionScopePtrMap &functions =
+      iterFile->second->getFunctions();
+    for (StringToFunctionScopePtrMap::const_iterator iter = functions.begin();
+         iter != functions.end(); ++iter) {
+      iter->second->setSystem();
+      s_functions[iter->first] = iter->second;
+    }
+  }
+
   return true;
 }
 
@@ -483,7 +477,6 @@ void BuiltinSymbols::LoadSuperGlobals() {
 }
 
 bool BuiltinSymbols::IsSuperGlobal(const std::string &name) {
-  if (NoSuperGlobals) return false;
   return s_superGlobals.find(name) != s_superGlobals.end();
 }
 

hphp/compiler/builtin_symbols.h

@@ -34,9 +34,8 @@ DECLARE_BOOST_TYPES(ConstantTable);
 class BuiltinSymbols {
 public:
   static bool Loaded;
-  static bool NoSuperGlobals; // for SystemCPP bootstraping only
 
-  static bool Load(AnalysisResultPtr ar, bool extOnly = false);
+  static bool Load(AnalysisResultPtr ar);
 
   static void LoadFunctions(AnalysisResultPtr ar,
                             StringToFunctionScopePtrMap &functions);

hphp/compiler/compiler.cpp

@@ -539,13 +539,13 @@ int process(const CompilerOptions &po) {
 
   bool isPickledPHP = (po.target == "php" && po.format == "pickled");
   if (!isPickledPHP) {
-    if (!BuiltinSymbols::Load(ar,
-                              po.target == "hhbc" && !Option::WholeProgram)) {
-      return false;
-    }
     if (po.target == "hhbc" && !Option::WholeProgram) {
-      BuiltinSymbols::NoSuperGlobals = false;
+      // We're trying to produce the same bytecode as runtime parsing.
+      // There's nothing to do.
     } else {
+      if (!BuiltinSymbols::Load(ar)) {
+        return false;
+      }
       ar->loadBuiltins();
     }
     hphp_process_init();