fix file/class in backtraces involving traits

- non-repo mode: use fullName() instead of flawed logic in type constraint failures
- repo mode: save the original filename when flattening traits

hphp/compiler/analysis/class_scope.cpp

@@ -452,6 +452,10 @@ ClassScope::importTraitMethod(const TraitMethod&  traitMethod,
   cloneMeth->addTraitMethodToScope(ar,
                dynamic_pointer_cast<ClassScope>(shared_from_this()));
 
+  // Preserve original filename (as this varies per-function and not per-unit
+  // in the case of methods imported from flattened traits)
+  cloneMeth->setOriginalFilename(meth->getFileScope()->getName());
+
   return cloneMeth;
 }
 

hphp/compiler/analysis/emitter.cpp

@@ -5358,6 +5358,12 @@ void EmitterVisitor::emitPostponedMeths() {
     fe->setReturnTypeConstraint(
         StringData::GetStaticString(p.m_meth->getReturnTypeConstraint()));
 
+    // add the original filename for flattened traits
+    auto const originalFilename = p.m_meth->getOriginalFilename();
+    if (!originalFilename.empty()) {
+      fe->setOriginalFilename(StringData::GetStaticString(originalFilename));
+    }
+
     m_curFunc = fe;
 
     if (fe->isClosureBody() || fe->isGeneratorFromClosure()) {

hphp/compiler/statement/method_statement.h

@@ -62,6 +62,7 @@ public:
   std::string getFullName() const;
   std::string getOriginalFullName() const;
   std::string getOriginalFullNameForInjection() const;
+  std::string getOriginalFilename() const { return m_originalFilename; }
   ExpressionListPtr getParams() { return m_params;}
   const std::string getReturnTypeConstraint() const {
     return m_retTypeConstraint;
@@ -120,6 +121,12 @@ public:
     m_originalClassName = name;
   }
 
+  // for flattened traits
+  void setOriginalFilename(const std::string &name) {
+    assert(m_method);
+    m_originalFilename = name;
+  }
+
   void addTraitMethodToScope(AnalysisResultConstPtr ar,
                              ClassScopePtr classScope);
 
@@ -133,6 +140,7 @@ protected:
   std::string m_originalName;
   std::string m_className;
   std::string m_originalClassName;
+  std::string m_originalFilename;
   ExpressionListPtr m_params;
   std::string m_retTypeConstraint;
   StatementListPtr m_stmt;

hphp/runtime/vm/bytecode.cpp

@@ -859,13 +859,7 @@ void Stack::toStringFrame(std::ostream& os, const ActRec* fp,
   const Func* func = fp->m_func;
   assert(func);
   func->validate();
-  string funcName;
-  if (func->isMethod()) {
-    funcName = string(func->preClass()->name()->data()) + "::" +
-      string(func->name()->data());
-  } else {
-    funcName = string(func->name()->data());
-  }
+  string funcName(func->fullName()->data());
   os << "{func:" << funcName
      << ",soff:" << fp->m_soff
      << ",this:0x" << std::hex << (fp->hasThis() ? fp->getThis() : nullptr)
@@ -2407,6 +2401,9 @@ Array VMExecutionContext::debugBacktrace(bool skip /* = false */,
         Unit *unit = fp->m_func->unit();
         assert(unit);
         const char* filename = unit->filepath()->data();
+        if (fp->m_func->originalFilename()) {
+          filename = fp->m_func->originalFilename()->data();
+        }
         assert(filename);
         Offset off = pc;
 
@@ -2441,9 +2438,12 @@ Array VMExecutionContext::debugBacktrace(bool skip /* = false */,
     // Builtins and generators don't have a file and line number
     if (prevFp && !prevFp->m_func->isBuiltin() && !fp->m_func->isGenerator()) {
       auto const prevUnit = prevFp->m_func->unit();
-      frame.set(s_file,
-                const_cast<StringData*>(prevUnit->filepath()),
-                true);
+      auto prevFile = prevUnit->filepath();
+      if (prevFp->m_func->originalFilename()) {
+        prevFile = prevFp->m_func->originalFilename();
+      }
+      assert(prevFile);
+      frame.set(s_file, const_cast<StringData*>(prevFile), true);
 
       // In the normal method case, the "saved pc" for line number printing is
       // pointing at the cell conversion (Unbox/Pop) instruction, not the call

hphp/runtime/vm/func.cpp

@@ -462,7 +462,11 @@ void Func::prettyPrint(std::ostream& out) const {
     if (m_attrs & AttrAbstract) { out << "abstract "; }
     if (m_attrs & AttrFinal) { out << "final "; }
     if (m_attrs & AttrPhpLeafFn) { out << "(leaf) "; }
-    out << preClass()->name()->data() << "::" << m_name->data();
+    if (cls() != nullptr) {
+      out << fullName()->data();
+    } else {
+      out << preClass()->name()->data() << "::" << m_name->data();
+    }
   } else {
     out << "Function " << m_name->data();
   }
@@ -641,7 +645,7 @@ Func::SharedData::SharedData(PreClass* preClass, Id id,
     m_info(nullptr), m_refBitVec(nullptr), m_builtinFuncPtr(nullptr),
     m_docComment(docComment), m_top(top), m_isClosureBody(false),
     m_isGenerator(false), m_isGeneratorFromClosure(false),
-    m_hasGeneratorAsBody(false) {
+    m_hasGeneratorAsBody(false), m_originalFilename(nullptr) {
 }
 
 Func::SharedData::~SharedData() {
@@ -702,6 +706,7 @@ FuncEmitter::FuncEmitter(UnitEmitter& ue, int sn, Id id, const StringData* n)
   , m_containsCalls(false)
   , m_info(nullptr)
   , m_builtinFuncPtr(nullptr)
+  , m_originalFilename(nullptr)
 {}
 
 FuncEmitter::FuncEmitter(UnitEmitter& ue, int sn, const StringData* n,
@@ -725,6 +730,7 @@ FuncEmitter::FuncEmitter(UnitEmitter& ue, int sn, const StringData* n,
   , m_containsCalls(false)
   , m_info(nullptr)
   , m_builtinFuncPtr(nullptr)
+  , m_originalFilename(nullptr)
 {}
 
 FuncEmitter::~FuncEmitter() {
@@ -966,6 +972,7 @@ Func* FuncEmitter::create(Unit& unit, PreClass* preClass /* = NULL */) const {
   f->shared()->m_builtinFuncPtr = m_builtinFuncPtr;
   f->shared()->m_nativeFuncPtr = m_nativeFuncPtr;
   f->shared()->m_retTypeConstraint = m_retTypeConstraint;
+  f->shared()->m_originalFilename = m_originalFilename;
   return f;
 }
 
@@ -1052,6 +1059,7 @@ void FuncEmitter::serdeMetaData(SerDe& sd) {
     (m_fpitab)
     (m_userAttributes)
     (m_retTypeConstraint)
+    (m_originalFilename)
     ;
 }
 

hphp/runtime/vm/func.h

@@ -320,6 +320,10 @@ struct Func {
     return shared()->m_retTypeConstraint;
   }
 
+  const StringData* originalFilename() const {
+    return shared()->m_originalFilename;
+  }
+
   int numIterators() const { return shared()->m_numIterators; }
   const EHEntVec& ehtab() const { return shared()->m_ehtab; }
   const FPIEntVec& fpitab() const { return shared()->m_fpitab; }
@@ -462,6 +466,8 @@ private:
     bool m_hasGeneratorAsBody : 1;
     UserAttributeMap m_userAttributes;
     const StringData* m_retTypeConstraint;
+    // per-func filepath for traits flattened during repo construction
+    const StringData* m_originalFilename;
     SharedData(PreClass* preClass, Id id, Offset base,
         Offset past, int line1, int line2, bool top,
         const StringData* docComment);
@@ -635,6 +641,10 @@ public:
   void setBuiltinFunc(const ClassInfo::MethodInfo* info,
       BuiltinFunction bif, BuiltinFunction nif, Offset base);
 
+  void setOriginalFilename(const StringData* name) {
+    m_originalFilename = name;
+  }
+
 private:
   void sortEHTab();
   void sortFPITab(bool load);
@@ -679,6 +689,8 @@ private:
   const ClassInfo::MethodInfo* m_info;
   BuiltinFunction m_builtinFuncPtr;
   BuiltinFunction m_nativeFuncPtr;
+
+  const StringData* m_originalFilename;
 };
 
 class FuncRepoProxy : public RepoProxy {

hphp/runtime/vm/type_constraint.cpp

@@ -199,12 +199,7 @@ void TypeConstraint::verifyFail(const Func* func, int paramNum,
                                 const TypedValue* tv) const {
   Transl::VMRegAnchor _;
   std::ostringstream fname;
-  if (func->preClass() != nullptr) {
-    fname << func->preClass()->name()->data() << "::"
-      << func->name()->data() << "()";
-  } else {
-    fname << func->name()->data() << "()";
-  }
+  fname << func->fullName()->data() << "()";
   const StringData* tn = typeName();
   if (isSelf()) {
     selfToTypeName(func, &tn);

hphp/test/quick/debug_backtrace_trait.php

@@ -0,0 +1,8 @@
+<?php
+
+include 'debug_backtrace_trait_helper.inc';
+class C extends P {
+  use T;
+}
+(new C)->bar(new stdClass);
+(new C)->bar(12);

hphp/test/quick/debug_backtrace_trait.php.expectf

@@ -0,0 +1,37 @@
+array(2) {
+  [0]=>
+  array(4) {
+    ["file"]=>
+    string(%d) "%Sdebug_backtrace_trait_helper.inc"
+    ["line"]=>
+    int(15)
+    ["function"]=>
+    string(15) "print_backtrace"
+    ["args"]=>
+    array(0) {
+    }
+  }
+  [1]=>
+  array(7) {
+    ["file"]=>
+    string(%d) "%Sdebug_backtrace_trait.php"
+    ["line"]=>
+    int(7)
+    ["function"]=>
+    string(3) "bar"
+    ["class"]=>
+    string(1) "C"
+    ["object"]=>
+    object(C)#1 (0) {
+    }
+    ["type"]=>
+    string(2) "->"
+    ["args"]=>
+    array(1) {
+      [0]=>
+      object(stdClass)#2 (0) {
+      }
+    }
+  }
+}
+HipHop Fatal error: Argument 1 passed to C::bar() must be an instance of stdClass, int given in %Sdebug_backtrace_trait_helper.inc on line 16

hphp/test/quick/debug_backtrace_trait_helper.inc

@@ -0,0 +1,17 @@
+<?php
+
+class P {
+  public function foo() {
+    bar();
+  }
+}
+
+function print_backtrace() {
+  var_dump(debug_backtrace());
+}
+
+trait T {
+  public function bar(stdClass $s) {
+    print_backtrace();
+  }
+}