remove now-redundant originalFilename field from HHBBC's function representations

Summary:
The duplication of `originalFilename` and `originalUnit` were introduced in D6770083 to support the flattening of traits in HHBBC.

A later diff, D6770160, removed the trait flattening code from hphpc, but we never appear to have cleaned up the originalFilename field.

This change makes it so we only keep `originalUnit` around, and renames `FuncEmitter`'s originalFilename to originalUnit for consistency.

As the asserts demonstrate, this saves 8 bytes from the `php::Func` structs in both low-pointer mode and regular-sized pointer modes in HHBBC. The `originalUnit` field had to be aligned above Attr where we were wasting some padding to realize the gain in lowptr mode.

Reviewed By: ricklavoie

Differential Revision: D73373868

fbshipit-source-id: 78c630124ed53b55c29b58bd49f5fdcbaace9990

Author: Sinan Cepel

hphp/hhbbc/emit.cpp

@@ -972,8 +972,7 @@ void emit_finish_func(EmitUnitState& state, FuncEmitter& fe,
   fe.userAttributes = func.userAttributes;
   fe.retUserType = func.returnUserType;
   fe.retTypeConstraints = func.retTypeConstraints;
-  fe.originalFilename =
-    func.originalFilename ? func.originalFilename :
+  fe.originalUnit =
     func.originalUnit ? func.originalUnit : nullptr;
   fe.originalModuleName = func.originalModuleName;
   fe.requiresFromOriginalModule = func.requiresFromOriginalModule;

hphp/hhbbc/index.cpp

@@ -117,7 +117,7 @@ const StaticString s_Traversable("HH\\Traversable");
 static_assert(CheckSize<php::Block, 24>(), "");
 static_assert(CheckSize<php::Local, use_lowptr ? 12 : 16>(), "");
 static_assert(CheckSize<php::Param, use_lowptr ? 56: 88>(), "");
-static_assert(CheckSize<php::Func, use_lowptr ? 176: 224>(), "");
+static_assert(CheckSize<php::Func, use_lowptr ? 168: 216>(), "");
 
 // Likewise, we also keep the bytecode and immediate types small.
 static_assert(CheckSize<Bytecode, use_lowptr ? 32 : 40>(), "");
@@ -11050,7 +11050,6 @@ struct FlattenJob {
       auto meth = std::move(clone->methods[i]);
       meth->cls = clone.get();
       assertx(meth->clsIdx == i);
-      if (!meth->originalFilename) meth->originalFilename = meth->unit;
       if (!meth->originalUnit)     meth->originalUnit = meth->unit;
       if (!meth->originalClass)    meth->originalClass = closure.name;
       meth->requiresFromOriginalModule = requiresFromOriginalModule;
@@ -11135,7 +11134,6 @@ struct FlattenJob {
     cloned->cls = const_cast<php::Class*>(&dstCls);
     cloned->unit = dstCls.unit;
 
-    if (!cloned->originalFilename) cloned->originalFilename = orig.unit;
     if (!cloned->originalUnit)     cloned->originalUnit = orig.unit;
     cloned->originalClass = orig.originalClass
       ? orig.originalClass

hphp/hhbbc/interp.cpp

@@ -1020,8 +1020,8 @@ void in(ISS& env, const bc::File&) {
     return push(env, TSStr);
   }
 
-  auto filename = env.ctx.func->originalFilename
-    ? env.ctx.func->originalFilename
+  auto filename = env.ctx.func->originalUnit
+    ? env.ctx.func->originalUnit
     : env.ctx.func->unit;
   if (!FileUtil::isAbsolutePath(filename->slice())) {
     filename = makeStaticString(
@@ -1038,8 +1038,8 @@ void in(ISS& env, const bc::Dir&) {
     return push(env, TSStr);
   }
 
-  auto filename = env.ctx.func->originalFilename
-    ? env.ctx.func->originalFilename
+  auto filename = env.ctx.func->originalUnit
+    ? env.ctx.func->originalUnit
     : env.ctx.func->unit;
   if (!FileUtil::isAbsolutePath(filename->slice())) {
     filename = makeStaticString(

hphp/hhbbc/parse.cpp

@@ -686,7 +686,7 @@ std::unique_ptr<php::Func> parse_func(ParseUnitState& puState,
   ret->userAttributes     = fe.userAttributes;
   ret->returnUserType     = fe.retUserType.ptr(ue);
   ret->retTypeConstraints = fe.retTypeConstraints;
-  ret->originalFilename   = fe.originalFilename;
+  ret->originalUnit       = fe.originalUnit;
   ret->originalModuleName = unit->moduleName;
 
   ret->isClosureBody       = fe.isClosureBody;

hphp/hhbbc/representation.cpp

@@ -100,7 +100,6 @@ template <typename SerDe> void Func::serde(SerDe& sd, Class* parentClass) {
     (numIters)
     (mainEntry)
     (returnUserType)
-    (originalFilename)
     (originalUnit)
     (originalClass)
     (originalModuleName)

hphp/hhbbc/representation.h

@@ -303,6 +303,16 @@ struct Func : FuncBase {
    */
   LSString name;
   SrcInfo srcInfo;
+
+
+  /*
+   * If hhbbc flattens a trait, we keep the original filename of the file
+   * that defined the trait in originalUnit to get to the srcLocs. This
+   * ensures that backtraces and whatnot work correctly. Otherwise this is
+   * nullptr. This field lives above `attrs` for alignment reasons.
+   */
+  LSString originalUnit{};
+
   Attr attrs;
 
   /*
@@ -348,17 +358,6 @@ struct Func : FuncBase {
    */
   LSString returnUserType;
 
-  /*
-   * If traits are being flattened by hphpc, we keep the original
-   * filename of a function (the file that defined the trait) so
-   * backtraces and things work correctly.  Otherwise this is nullptr.
-   * Similarly, if hhbbc did the flattening itself, we need the original
-   * unit, to get to the srcLocs. Once we stop flattening in hphpc, we can
-   * drop the originalFilename.
-   */
-  LSString originalFilename;
-  LSString originalUnit{};
-
   /*
    * The reference of the trait where the method was originally
    * defined.  This is used to detected if a method is imported

hphp/runtime/base/backtrace.cpp

@@ -440,8 +440,8 @@ Array createBacktrace(const BacktraceArgs& btArgs) {
     // Builtins and generators don't have a file and line number.
     if (prev && !prev.func()->isBuiltin()) {
       auto const prevFunc = prev.func();
-      auto const prevFile = prevFunc->originalFilename()
-        ? prevFunc->originalFilename()
+      auto const prevFile = prevFunc->originalUnit()
+        ? prevFunc->originalUnit()
         : prevFunc->unit()->filepath();
 
       assertx(prevFile != nullptr);
@@ -684,8 +684,8 @@ Array CompactTraceData::extract() const {
       auto const prevFunc = prev->func;
       auto const prevUnit = prevFunc->unit();
       auto prevFile = prevUnit->filepath();
-      if (prevFunc->originalFilename()) {
-        prevFile = prevFunc->originalFilename();
+      if (prevFunc->originalUnit()) {
+        prevFile = prevFunc->originalUnit();
       }
 
       auto const prevPc = prev->prevPc;

hphp/runtime/base/execution-context.cpp

@@ -1225,8 +1225,8 @@ StringData* ExecutionContext::getContainingFileName() {
   if (ar->skipFrame()) ar = getPrevVMStateSkipFrame(ar);
   if (ar == nullptr) return staticEmptyString();
   Unit* unit = ar->func()->unit();
-  auto const path = ar->func()->originalFilename() ?
-    ar->func()->originalFilename() : unit->filepath();
+  auto const path =  ar->func()->originalUnit() ?
+    ar->func()->originalUnit() : unit->filepath();
   return const_cast<StringData*>(path);
 }
 

hphp/runtime/ext/asio/ext_async-function-wait-handle.cpp

@@ -384,8 +384,8 @@ void c_AsyncFunctionWaitHandle::exitContext(ContextIndex contextIdx) {
 
 // Get the filename in which execution will proceed when execution resumes.
 String c_AsyncFunctionWaitHandle::getFilename() {
-  if (actRec()->func()->originalFilename()) {
-    return actRec()->func()->originalFilename()->data();
+  if (actRec()->func()->originalUnit()) {
+    return actRec()->func()->originalUnit()->data();
   } else {
     return actRec()->func()->unit()->filepath()->data();
   }

hphp/runtime/ext/std/ext_std_errorfunc.cpp

@@ -104,8 +104,8 @@ bool hphp_debug_caller_info_impl(
     RuntimeStruct::registerRuntimeStruct(s_hphp_debug_caller_info, s_fields);
 
   auto const cls = func->cls();
-  auto const path = func->originalFilename() ?
-    func->originalFilename() : func->unit()->filepath();
+  auto const path = func->originalUnit() ?
+    func->originalUnit() : func->unit()->filepath();
 
   auto const has_cls = cls && !func->isClosureBody();
   StructDictInit init(s_struct, has_cls ? 4 : 3);