From 89765cd4f2e67ea538e92f032bc25cab0eef8ac0 Mon Sep 17 00:00:00 2001 From: Drew Paroski Date: Fri, 19 Apr 2013 17:00:01 -0700 Subject: [PATCH] Make arrays implement Traversable and KeyedTraversable This diff changes HHVM so that arrays are considered to implement the Traversable and KeyedTraversable interfaces (which have no methods). The idea is that these interfaces will be useful for parameter type constraints for PHP code that wants to be compatible with both arrays and collections (and possibly other objects that implement these interfaces). --- hphp/compiler/analysis/function_scope.cpp | 12 ----- hphp/compiler/analysis/function_scope.h | 2 - hphp/compiler/analysis/type.cpp | 22 +++++++-- .../expression/binary_op_expression.cpp | 11 ++++- hphp/runtime/base/builtin_functions.cpp | 48 +++++++++++++------ hphp/runtime/base/builtin_functions.h | 9 ++-- hphp/runtime/vm/bytecode.cpp | 5 ++ .../vm/translator/hopt/hhbctranslator.cpp | 3 +- hphp/runtime/vm/translator/translator-x64.cpp | 12 +++-- hphp/runtime/vm/type_constraint.cpp | 9 +++- hphp/system/classes/collections.php | 3 ++ .../slow/array_iterator/array-traversable.php | 31 ++++++++++++ .../array-traversable.php.expectf | 14 ++++++ 13 files changed, 136 insertions(+), 45 deletions(-) create mode 100644 hphp/test/slow/array_iterator/array-traversable.php create mode 100644 hphp/test/slow/array_iterator/array-traversable.php.expectf diff --git a/hphp/compiler/analysis/function_scope.cpp b/hphp/compiler/analysis/function_scope.cpp index f1fd6754c..3d635493b 100644 --- a/hphp/compiler/analysis/function_scope.cpp +++ b/hphp/compiler/analysis/function_scope.cpp @@ -574,18 +574,6 @@ void FunctionScope::setPerfectVirtual() { } } -bool FunctionScope::needsTypeCheckWrapper() const { - for (int i = 0; i < m_maxParam; i++) { - if (isRefParam(i)) continue; - if (TypePtr spec = m_paramTypeSpecs[i]) { - if (Type::SameType(spec, m_paramTypes[i])) { - return true; - } - } - } - return false; -} - bool FunctionScope::needsClassParam() { if (!isStatic()) return false; ClassScopeRawPtr cls = getContainingClass(); diff --git a/hphp/compiler/analysis/function_scope.h b/hphp/compiler/analysis/function_scope.h index f401a3a83..161071a82 100644 --- a/hphp/compiler/analysis/function_scope.h +++ b/hphp/compiler/analysis/function_scope.h @@ -267,8 +267,6 @@ public: void clearRetExprs(); void fixRetExprs(); - bool needsTypeCheckWrapper() const; - void setOptFunction(FunctionOptPtr fn) { m_optFunction = fn; } FunctionOptPtr getOptFunction() const { return m_optFunction; } diff --git a/hphp/compiler/analysis/type.cpp b/hphp/compiler/analysis/type.cpp index 34a2217bc..5b0f9d646 100644 --- a/hphp/compiler/analysis/type.cpp +++ b/hphp/compiler/analysis/type.cpp @@ -20,6 +20,7 @@ #include "hphp/compiler/analysis/class_scope.h" #include "hphp/compiler/analysis/file_scope.h" #include "hphp/compiler/expression/expression.h" +#include "hphp/runtime/base/builtin_functions.h" #include using namespace HPHP; @@ -77,14 +78,25 @@ void Type::ResetTypeHintTypes() { s_HHTypeHintTypes.clear(); } -TypePtr Type::CreateObjectType(const std::string &classname) { - return TypePtr(new Type(KindOfObject, classname)); +TypePtr Type::CreateObjectType(const std::string &clsname) { + // For interfaces that support arrays we're pessimistic and + // we treat it as a Variant + if (interface_supports_array(clsname)) { + return Type::Variant; + } + return TypePtr(new Type(KindOfObject, clsname)); } -TypePtr Type::GetType(KindOf kindOf, - const std::string &clsname /* = "" */) { +TypePtr Type::GetType(KindOf kindOf, const std::string &clsname /* = "" */) { assert(kindOf); - if (!clsname.empty()) return TypePtr(new Type(kindOf, clsname)); + if (!clsname.empty()) { + // For interfaces that support arrays we're pessimistic and + // we treat it as a Variant + if (interface_supports_array(clsname)) { + return Type::Variant; + } + return TypePtr(new Type(kindOf, clsname)); + } switch (kindOf) { case KindOfBoolean: return Type::Boolean; diff --git a/hphp/compiler/expression/binary_op_expression.cpp b/hphp/compiler/expression/binary_op_expression.cpp index 68c1d7052..92de108ab 100644 --- a/hphp/compiler/expression/binary_op_expression.cpp +++ b/hphp/compiler/expression/binary_op_expression.cpp @@ -579,8 +579,15 @@ ExpressionPtr BinaryOpExpression::foldConst(AnalysisResultConstPtr ar) { result = v1 || v2; break; case T_LOGICAL_AND: result = v1 && v2; break; - case T_INSTANCEOF: - result = false; break; + case T_INSTANCEOF: { + if (v1.isArray() && v2.isString() && + interface_supports_array(v2.getStringData())) { + result = true; + break; + } + result = false; + break; + } default: return ExpressionPtr(); } diff --git a/hphp/runtime/base/builtin_functions.cpp b/hphp/runtime/base/builtin_functions.cpp index 7ace9bb8d..0eb8e40bc 100644 --- a/hphp/runtime/base/builtin_functions.cpp +++ b/hphp/runtime/base/builtin_functions.cpp @@ -14,8 +14,9 @@ +----------------------------------------------------------------------+ */ -#include "hphp/runtime/base/type_conversions.h" #include "hphp/runtime/base/builtin_functions.h" + +#include "hphp/runtime/base/type_conversions.h" #include "hphp/runtime/base/code_coverage.h" #include "hphp/runtime/base/externals.h" #include "hphp/runtime/base/variable_serializer.h" @@ -48,21 +49,25 @@ namespace HPHP { /////////////////////////////////////////////////////////////////////////////// // static strings -static StaticString s_offsetExists("offsetExists"); -static StaticString s___autoload("__autoload"); -static StaticString s___call("__call"); -static StaticString s___callStatic("__callStatic"); -static StaticString s_exception("exception"); -static StaticString s_previous("previous"); +const StaticString s_offsetExists("offsetExists"); +const StaticString s___autoload("__autoload"); +const StaticString s___call("__call"); +const StaticString s___callStatic("__callStatic"); +const StaticString s_exception("exception"); +const StaticString s_previous("previous"); -StaticString s_self("self"); -StaticString s_parent("parent"); -StaticString s_static("static"); -StaticString s_class("class"); -StaticString s_function("function"); -StaticString s_constant("constant"); -StaticString s_type("type"); -StaticString s_failure("failure"); +const StaticString s_self("self"); +const StaticString s_parent("parent"); +const StaticString s_static("static"); +const StaticString s_class("class"); +const StaticString s_function("function"); +const StaticString s_constant("constant"); +const StaticString s_type("type"); +const StaticString s_failure("failure"); + +const StaticString s_Traversable("Traversable"); +const StaticString s_KeyedTraversable("KeyedTraversable"); +const StaticString s_Indexish("Indexish"); /////////////////////////////////////////////////////////////////////////////// @@ -979,6 +984,19 @@ bool isset(CVarRef v, CStrRef offset, bool isString /* = false */) { return false; } +bool interface_supports_array(const StringData* s) { + return (s->isame(s_Traversable.get()) || + s->isame(s_KeyedTraversable.get()) || + s->isame(s_Indexish.get())); +} + +bool interface_supports_array(const std::string& n) { + const char* s = n.c_str(); + return ((n.size() == 11 && !strcasecmp(s, "Traversable")) || + (n.size() == 16 && !strcasecmp(s, "KeyedTraversable")) || + (n.size() == 8 && !strcasecmp(s, "Indexish"))); +} + String get_source_filename(litstr path, bool dir_component /* = false */) { String ret; if (path[0] == '/') { diff --git a/hphp/runtime/base/builtin_functions.h b/hphp/runtime/base/builtin_functions.h index a0c423f58..05b1b00a0 100644 --- a/hphp/runtime/base/builtin_functions.h +++ b/hphp/runtime/base/builtin_functions.h @@ -60,9 +60,9 @@ namespace HPHP { /////////////////////////////////////////////////////////////////////////////// -extern StaticString s_self; -extern StaticString s_parent; -extern StaticString s_static; +extern const StaticString s_self; +extern const StaticString s_parent; +extern const StaticString s_static; // empty @@ -297,6 +297,9 @@ inline bool is_empty_string(CVarRef v) { return v.isString() && v.getStringData()->empty(); } +bool interface_supports_array(const StringData* s); +bool interface_supports_array(const std::string& n); + /////////////////////////////////////////////////////////////////////////////// // special variable contexts diff --git a/hphp/runtime/vm/bytecode.cpp b/hphp/runtime/vm/bytecode.cpp index 24f2c0e01..e49ce1358 100644 --- a/hphp/runtime/vm/bytecode.cpp +++ b/hphp/runtime/vm/bytecode.cpp @@ -4288,6 +4288,11 @@ inline bool OPTBLD_INLINE VMExecutionContext::cellInstanceOf( if (tv->m_type == KindOfObject) { Class* cls = Unit::lookupClass(ne); if (cls) return tv->m_data.pobj->instanceof(cls); + } else if (tv->m_type == KindOfArray) { + Class* cls = Unit::lookupClass(ne); + if (cls && interface_supports_array(cls->name())) { + return true; + } } return false; } diff --git a/hphp/runtime/vm/translator/hopt/hhbctranslator.cpp b/hphp/runtime/vm/translator/hopt/hhbctranslator.cpp index c78b1d227..f07904674 100644 --- a/hphp/runtime/vm/translator/hopt/hhbctranslator.cpp +++ b/hphp/runtime/vm/translator/hopt/hhbctranslator.cpp @@ -2452,7 +2452,8 @@ void HhbcTranslator::emitInstanceOfD(int classNameStrId) { PUNT(InstanceOfD_MaybeObj); } if (!src->isA(Type::Obj)) { - push(cns(false)); + bool res = (src->isA(Type::Arr) && interface_supports_array(className)); + push(cns(res)); gen(DecRef, src); return; } diff --git a/hphp/runtime/vm/translator/translator-x64.cpp b/hphp/runtime/vm/translator/translator-x64.cpp index 221cfa43b..5194b62e3 100644 --- a/hphp/runtime/vm/translator/translator-x64.cpp +++ b/hphp/runtime/vm/translator/translator-x64.cpp @@ -10257,19 +10257,24 @@ TranslatorX64::translateInstanceOfD(const Tracelet& t, srcReg = getReg(input0->location); } + const StringData* clsName = curUnit()->lookupLitstrId(i.imm[0].u_SA); + if (type != KindOfObject) { + // Handle cases where the left hand side is not an object. If the + // left hand side is an array and the right hand side is an interface + // that supports arrays, we return true, otherwise we return false. + bool res = (type == KindOfArray && interface_supports_array(clsName)); Stats::emitInc(a, Stats::Tx64_InstanceOfDBypass); - // All non-object inputs are not instances if (!input0IsLoc) { assert(!input0->isRef()); emitDecRef(i, srcReg, type); } if (i.changesPC) { - fuseBranchAfterStaticBool(a, t, i, false); + fuseBranchAfterStaticBool(a, t, i, res); assert(!patchAddr); return; } else { - emitImmReg(a, false, r(result)); + emitImmReg(a, res, r(result)); } } else { // Get the input's class from ObjectData->m_cls @@ -10286,7 +10291,6 @@ TranslatorX64::translateInstanceOfD(const Tracelet& t, emitDecRef(i, srcReg, type); } - const StringData* clsName = curUnit()->lookupLitstrId(i.imm[0].u_SA); Class* maybeCls = Unit::lookupUniqueClass(clsName); // maybeInterface is just used as a hint: If it's a trait/interface now but diff --git a/hphp/runtime/vm/type_constraint.cpp b/hphp/runtime/vm/type_constraint.cpp index 6560b665c..7c35eac38 100644 --- a/hphp/runtime/vm/type_constraint.cpp +++ b/hphp/runtime/vm/type_constraint.cpp @@ -14,6 +14,8 @@ +----------------------------------------------------------------------+ */ +#include "hphp/runtime/vm/type_constraint.h" + #include "hphp/util/base.h" #include "hphp/util/trace.h" #include "hphp/runtime/ext/ext_function.h" @@ -23,7 +25,6 @@ #include "hphp/runtime/vm/func.h" #include "hphp/runtime/vm/translator/translator-inline.h" #include "hphp/runtime/base/builtin_functions.h" -#include "hphp/runtime/vm/type_constraint.h" namespace HPHP { @@ -175,6 +176,12 @@ TypeConstraint::check(const TypedValue* tv, const Func* func) const { return !selfOrParentOrCallable && checkTypedefObj(tv); } + if (tv->m_type == KindOfArray && + isObjectOrTypedef() && + interface_supports_array(m_typeName)) { + return true; + } + return isObjectOrTypedef() && !isCallable() ? checkTypedefNonObj(tv) : equivDataTypes(m_type.m_dt, tv->m_type); diff --git a/hphp/system/classes/collections.php b/hphp/system/classes/collections.php index 34c3e4070..6b47a40a7 100644 --- a/hphp/system/classes/collections.php +++ b/hphp/system/classes/collections.php @@ -445,6 +445,9 @@ interface MapAccess extends ConstMapAccess, IndexAccess { } +interface Indexish extends KeyedTraversable { +} + interface ConstVector extends ConstCollection, ConstIndexAccess, KeyedIterable { diff --git a/hphp/test/slow/array_iterator/array-traversable.php b/hphp/test/slow/array_iterator/array-traversable.php new file mode 100644 index 000000000..2d2f8cf62 --- /dev/null +++ b/hphp/test/slow/array_iterator/array-traversable.php @@ -0,0 +1,31 @@ +