From a8caa5c684452c1fc8ba6d97034b82eb15ac656d Mon Sep 17 00:00:00 2001 From: Joseph Griego Date: Mon, 15 Oct 2018 16:10:59 -0700 Subject: [PATCH] Split HAC CheckMisc into CheckArrayPlus and CheckArrayKeyCast Summary: What it says on the tin--split these notices to be controlled by separate flags Reviewed By: viratyosin Differential Revision: D10222024 fbshipit-source-id: b4d4ec8f1cfd18e92bb4dba4d540e6c62aa0809b --- hphp/compiler/option.cpp | 3 ++- hphp/hhbbc/main.cpp | 3 ++- hphp/runtime/base/array-data-inl.h | 8 ++++++-- hphp/runtime/base/array-data.cpp | 4 ++-- hphp/runtime/base/array-data.h | 3 ++- hphp/runtime/base/runtime-option.h | 6 ++++-- hphp/runtime/base/tv-arith.cpp | 4 ++-- hphp/runtime/base/tv-conversions-inl.h | 2 +- hphp/runtime/base/type-array.cpp | 4 ++-- hphp/runtime/ext/array/ext_array.cpp | 4 ++-- hphp/runtime/server/source-root-info.cpp | 2 +- hphp/runtime/vm/jit/irgen-builtin.cpp | 2 +- hphp/runtime/vm/jit/translator-runtime.cpp | 2 +- hphp/runtime/vm/member-operations.h | 2 +- .../slow/array_functions/ake-object.php.opts | 2 +- hphp/test/slow/hack_arr_compat/add.php.opts | 2 +- .../hack_arr_compat/implicit-key.php.opts | 2 +- .../slow/hack_arr_compat/scalars.php.opts | 3 ++- .../hack_arr_compat/split_misc_notices.php | 10 ++++++++++ .../split_misc_notices.php.expectf | 19 +++++++++++++++++++ .../split_misc_notices.php.hphp_opts | 1 + .../split_misc_notices.php.opts | 3 +++ .../hack_arr_compat/split_misc_notices_2.php | 10 ++++++++++ .../split_misc_notices_2.php.expectf | 15 +++++++++++++++ .../split_misc_notices_2.php.hphp_opts | 1 + .../split_misc_notices_2.php.opts | 3 +++ .../slow/inline-interp-stack.php.hphp_opts | 2 +- hphp/test/slow/inline-interp-stack.php.opts | 2 +- 28 files changed, 98 insertions(+), 26 deletions(-) create mode 100644 hphp/test/slow/hack_arr_compat/split_misc_notices.php create mode 100644 hphp/test/slow/hack_arr_compat/split_misc_notices.php.expectf create mode 100644 hphp/test/slow/hack_arr_compat/split_misc_notices.php.hphp_opts create mode 100644 hphp/test/slow/hack_arr_compat/split_misc_notices.php.opts create mode 100644 hphp/test/slow/hack_arr_compat/split_misc_notices_2.php create mode 100644 hphp/test/slow/hack_arr_compat/split_misc_notices_2.php.expectf create mode 100644 hphp/test/slow/hack_arr_compat/split_misc_notices_2.php.hphp_opts create mode 100644 hphp/test/slow/hack_arr_compat/split_misc_notices_2.php.opts diff --git a/hphp/compiler/option.cpp b/hphp/compiler/option.cpp index e93afd7ee1f21..6e07708ee618c 100644 --- a/hphp/compiler/option.cpp +++ b/hphp/compiler/option.cpp @@ -250,7 +250,8 @@ void Option::Load(const IniSetting::Map& ini, Hdf &config) { BIND_HAC_OPTION(CheckRefBind, Notices) BIND_HAC_OPTION(CheckFalseyPromote, Notices) BIND_HAC_OPTION(CheckCompare, Notices) - BIND_HAC_OPTION(CheckMisc, Notices) + BIND_HAC_OPTION(CheckArrayKeyCast, Notices) + BIND_HAC_OPTION(CheckArrayPlus, Notices) BIND_HAC_OPTION_SELF(IsArrayNotices) BIND_HAC_OPTION_SELF(PromoteNotices) BIND_HAC_OPTION_SELF(TypeHintNotices) diff --git a/hphp/hhbbc/main.cpp b/hphp/hhbbc/main.cpp index d337e24abe28c..72b4926054fd7 100644 --- a/hphp/hhbbc/main.cpp +++ b/hphp/hhbbc/main.cpp @@ -273,7 +273,8 @@ std::pair>, RuntimeOption::EvalHackArrCompatCheckRefBind = RuntimeOption::EvalHackArrCompatCheckFalseyPromote = RuntimeOption::EvalHackArrCompatCheckCompare = - RuntimeOption::EvalHackArrCompatCheckMisc = + RuntimeOption::EvalHackArrCompatCheckArrayPlus = + RuntimeOption::EvalHackArrCompatCheckArrayKeyCast = gd.HackArrCompatNotices; RuntimeOption::EvalAllowObjectDestructors = gd.AllowObjectDestructors; RuntimeOption::EvalForbidDynamicCalls = gd.ForbidDynamicCalls; diff --git a/hphp/runtime/base/array-data-inl.h b/hphp/runtime/base/array-data-inl.h index 20a211669dfd6..aa7df3521bcdf 100644 --- a/hphp/runtime/base/array-data-inl.h +++ b/hphp/runtime/base/array-data-inl.h @@ -331,9 +331,13 @@ ALWAYS_INLINE bool checkHACCompare() { return RuntimeOption::EvalHackArrCompatNotices && RuntimeOption::EvalHackArrCompatCheckCompare; } -ALWAYS_INLINE bool checkHACMisc() { +ALWAYS_INLINE bool checkHACArrayPlus() { return RuntimeOption::EvalHackArrCompatNotices && - RuntimeOption::EvalHackArrCompatCheckMisc; + RuntimeOption::EvalHackArrCompatCheckArrayPlus; +} +ALWAYS_INLINE bool checkHACArrayKeyCast() { + return RuntimeOption::EvalHackArrCompatNotices && + RuntimeOption::EvalHackArrCompatCheckArrayKeyCast; } /////////////////////////////////////////////////////////////////////////////// diff --git a/hphp/runtime/base/array-data.cpp b/hphp/runtime/base/array-data.cpp index eb9e4362cdf23..375992d342190 100644 --- a/hphp/runtime/base/array-data.cpp +++ b/hphp/runtime/base/array-data.cpp @@ -1391,7 +1391,7 @@ void raiseHackArrCompatRefIter() { } void raiseHackArrCompatAdd() { - raise_hac_misc_notice("Using + operator on arrays"); + raise_hac_array_plus_notice("Using + operator on arrays"); } void raiseHackArrCompatArrMixedCmp() { @@ -1425,7 +1425,7 @@ std::string makeHackArrCompatImplicitArrayKeyMsg(const TypedValue* key) { } void raiseHackArrCompatImplicitArrayKey(const TypedValue* key) { - raise_hac_misc_notice(makeHackArrCompatImplicitArrayKeyMsg(key)); + raise_hac_array_key_cast_notice(makeHackArrCompatImplicitArrayKeyMsg(key)); } /////////////////////////////////////////////////////////////////////////////// diff --git a/hphp/runtime/base/array-data.h b/hphp/runtime/base/array-data.h index b601d5df9ee3b..f5fba8bf515c9 100644 --- a/hphp/runtime/base/array-data.h +++ b/hphp/runtime/base/array-data.h @@ -1030,7 +1030,8 @@ bool checkHACIntishCast(); bool checkHACRefBind(); bool checkHACFalseyPromote(); bool checkHACCompare(); -bool checkHACMisc(); +bool checkHACArrayPlus(); +bool checkHACArrayKeyCast(); /////////////////////////////////////////////////////////////////////////////// diff --git a/hphp/runtime/base/runtime-option.h b/hphp/runtime/base/runtime-option.h index ef08c2dc77154..45b8217c9f0cb 100644 --- a/hphp/runtime/base/runtime-option.h +++ b/hphp/runtime/base/runtime-option.h @@ -554,7 +554,8 @@ struct RuntimeOption { HC(RefBind, ref_bind) \ HC(FalseyPromote, falsey_promote) \ HC(Compare, compare) \ - HC(Misc, misc) + HC(ArrayKeyCast, array_key_cast) \ + HC(ArrayPlus, array_plus) #define EVALFLAGS() \ /* F(type, name, defaultVal) */ \ @@ -917,7 +918,8 @@ struct RuntimeOption { F(bool, HackArrCompatCheckRefBind, false) \ F(bool, HackArrCompatCheckFalseyPromote, false) \ F(bool, HackArrCompatCheckCompare, false) \ - F(bool, HackArrCompatCheckMisc, false) \ + F(bool, HackArrCompatCheckArrayPlus, false) \ + F(bool, HackArrCompatCheckArrayKeyCast, false) \ /* Raise notices when is_array is called with any hack array */ \ F(bool, HackArrCompatIsArrayNotices, false) \ /* Raise notices when is_vec or is_dict is called with a v/darray */ \ diff --git a/hphp/runtime/base/tv-arith.cpp b/hphp/runtime/base/tv-arith.cpp index 23f5c03ee043d..faa298808c9b1 100644 --- a/hphp/runtime/base/tv-arith.cpp +++ b/hphp/runtime/base/tv-arith.cpp @@ -204,7 +204,7 @@ struct Add { ArrayData* operator()(ArrayData* a1, ArrayData* a2) const { if (UNLIKELY(a1->isHackArray())) throwInvalidAdditionException(a1); if (UNLIKELY(a2->isHackArray())) throwInvalidAdditionException(a2); - if (checkHACMisc()) raiseHackArrCompatAdd(); + if (checkHACArrayPlus()) raiseHackArrCompatAdd(); a1->incRefCount(); // force COW SCOPE_EXIT { a1->decRefCount(); }; return a1->plusEq(a2); @@ -359,7 +359,7 @@ struct AddEq { ArrayData* operator()(ArrayData* ad1, ArrayData* ad2) const { if (UNLIKELY(ad1->isHackArray())) throwInvalidAdditionException(ad1); if (UNLIKELY(ad2->isHackArray())) throwInvalidAdditionException(ad2); - if (checkHACMisc()) raiseHackArrCompatAdd(); + if (checkHACArrayPlus()) raiseHackArrCompatAdd(); if (ad2->empty() || ad1 == ad2) return ad1; if (ad1->empty()) { ad2->incRefCount(); diff --git a/hphp/runtime/base/tv-conversions-inl.h b/hphp/runtime/base/tv-conversions-inl.h index c3b06e0920ce0..949a938b07bda 100644 --- a/hphp/runtime/base/tv-conversions-inl.h +++ b/hphp/runtime/base/tv-conversions-inl.h @@ -133,7 +133,7 @@ inline Cell cellToKey(Cell cell, const ArrayData* ad) { if (!ad->useWeakKeys()) { throwInvalidArrayKeyException(&cell, ad); } - if (checkHACMisc()) { + if (checkHACArrayKeyCast()) { raiseHackArrCompatImplicitArrayKey(&cell); } diff --git a/hphp/runtime/base/type-array.cpp b/hphp/runtime/base/type-array.cpp index e68eb94da95f7..e48714a2ceb05 100644 --- a/hphp/runtime/base/type-array.cpp +++ b/hphp/runtime/base/type-array.cpp @@ -314,7 +314,7 @@ Array& Array::plusImpl(ArrayData *data) { if (m_arr == nullptr || data == nullptr) { throw_bad_array_merge(); } - if (checkHACMisc()) raiseHackArrCompatAdd(); + if (checkHACArrayPlus()) raiseHackArrCompatAdd(); if (!data->empty()) { if (m_arr->empty()) { m_arr = data; @@ -751,7 +751,7 @@ decltype(auto) elem(const Array& arr, Fn fn, bool is_key, if (!ad->useWeakKeys()) { throwInvalidArrayKeyException(&immutable_uninit_base, ad); } - if (checkHACMisc()) { + if (checkHACArrayKeyCast()) { raiseHackArrCompatImplicitArrayKey(&immutable_uninit_base); } return fn(make_tv(staticEmptyString()), diff --git a/hphp/runtime/ext/array/ext_array.cpp b/hphp/runtime/ext/array/ext_array.cpp index 1a2b2f891e447..0b14232bc4960 100644 --- a/hphp/runtime/ext/array/ext_array.cpp +++ b/hphp/runtime/ext/array/ext_array.cpp @@ -335,7 +335,7 @@ bool HHVM_FUNCTION(array_key_exists, switch (cell->m_type) { case KindOfUninit: case KindOfNull: - if (checkHACMisc() && ad->useWeakKeys()) { + if (checkHACArrayKeyCast() && ad->useWeakKeys()) { raiseHackArrCompatImplicitArrayKey(cell); } return ad->useWeakKeys() && ad->exists(staticEmptyString()); @@ -357,7 +357,7 @@ bool HHVM_FUNCTION(array_key_exists, case KindOfFunc: case KindOfClass: if (!ad->useWeakKeys()) throwInvalidArrayKeyException(cell, ad); - if (checkHACMisc()) { + if (checkHACArrayKeyCast()) { raiseHackArrCompatImplicitArrayKey(cell); } raise_warning("Array key should be either a string or an integer"); diff --git a/hphp/runtime/server/source-root-info.cpp b/hphp/runtime/server/source-root-info.cpp index 678fc292bbf79..0116d32ba970d 100644 --- a/hphp/runtime/server/source-root-info.cpp +++ b/hphp/runtime/server/source-root-info.cpp @@ -227,7 +227,7 @@ Array SourceRootInfo::setServerVariables(Array server) const { } { - SuppressHACMiscNotices shacn; + SuppressHACArrayPlusNotices shacn; if (!m_serverVars.empty()) { server += m_serverVars; } diff --git a/hphp/runtime/vm/jit/irgen-builtin.cpp b/hphp/runtime/vm/jit/irgen-builtin.cpp index 055c344f90306..73e8ca16f16f7 100644 --- a/hphp/runtime/vm/jit/irgen-builtin.cpp +++ b/hphp/runtime/vm/jit/irgen-builtin.cpp @@ -2151,7 +2151,7 @@ void emitAKExists(IRGS& env) { if (!arr->isA(TArr) && !arr->isA(TObj)) PUNT(AKExists_badArray); if (key->isA(TInitNull)) { - if (checkHACMisc()) { + if (checkHACArrayKeyCast()) { gen( env, RaiseHackArrCompatNotice, diff --git a/hphp/runtime/vm/jit/translator-runtime.cpp b/hphp/runtime/vm/jit/translator-runtime.cpp index ca733cde77e0c..baec3c5846fa0 100644 --- a/hphp/runtime/vm/jit/translator-runtime.cpp +++ b/hphp/runtime/vm/jit/translator-runtime.cpp @@ -167,7 +167,7 @@ ArrayData* arrayAdd(ArrayData* a1, ArrayData* a2) { assertx(a1->isPHPArray()); assertx(a2->isPHPArray()); - if (checkHACMisc()) raiseHackArrCompatAdd(); + if (checkHACArrayPlus()) raiseHackArrCompatAdd(); if (!a2->empty()) { if (a1->empty()) { diff --git a/hphp/runtime/vm/member-operations.h b/hphp/runtime/vm/member-operations.h index c01f97de8f06d..b18551ee096d9 100644 --- a/hphp/runtime/vm/member-operations.h +++ b/hphp/runtime/vm/member-operations.h @@ -1643,7 +1643,7 @@ inline ArrayData* SetElemArrayPre(ArrayData* a, a, const_cast(funcToStringHelper(key.m_data.pfunc)), value ); } - if (checkHACMisc()) { + if (checkHACArrayKeyCast()) { raiseHackArrCompatImplicitArrayKey(&key); } if (isNullType(key.m_type)) { diff --git a/hphp/test/slow/array_functions/ake-object.php.opts b/hphp/test/slow/array_functions/ake-object.php.opts index 54daa78d0512a..48223ebf93b20 100644 --- a/hphp/test/slow/array_functions/ake-object.php.opts +++ b/hphp/test/slow/array_functions/ake-object.php.opts @@ -1,2 +1,2 @@ -vEval.HackArrCompatNotices=1 --vEval.HackArrCompatCheckMisc=1 +-vEval.HackArrCompatCheckArrayKeyCast=1 diff --git a/hphp/test/slow/hack_arr_compat/add.php.opts b/hphp/test/slow/hack_arr_compat/add.php.opts index 54daa78d0512a..b41bb6127ca07 100644 --- a/hphp/test/slow/hack_arr_compat/add.php.opts +++ b/hphp/test/slow/hack_arr_compat/add.php.opts @@ -1,2 +1,2 @@ -vEval.HackArrCompatNotices=1 --vEval.HackArrCompatCheckMisc=1 +-vEval.HackArrCompatCheckArrayPlus=1 diff --git a/hphp/test/slow/hack_arr_compat/implicit-key.php.opts b/hphp/test/slow/hack_arr_compat/implicit-key.php.opts index 54daa78d0512a..48223ebf93b20 100644 --- a/hphp/test/slow/hack_arr_compat/implicit-key.php.opts +++ b/hphp/test/slow/hack_arr_compat/implicit-key.php.opts @@ -1,2 +1,2 @@ -vEval.HackArrCompatNotices=1 --vEval.HackArrCompatCheckMisc=1 +-vEval.HackArrCompatCheckArrayKeyCast=1 diff --git a/hphp/test/slow/hack_arr_compat/scalars.php.opts b/hphp/test/slow/hack_arr_compat/scalars.php.opts index 908b528f71fa0..fef0f8c6946d8 100644 --- a/hphp/test/slow/hack_arr_compat/scalars.php.opts +++ b/hphp/test/slow/hack_arr_compat/scalars.php.opts @@ -1,4 +1,5 @@ -vEval.HackArrCompatNotices=1 -vEval.HackArrCompatCheckIntishCast=1 -vEval.HackArrCompatCheckCompare=1 --vEval.HackArrCompatCheckMisc=1 +-vEval.HackArrCompatCheckArrayKeyCast=1 +-vEval.HackArrCompatCheckArrayPlus=1 diff --git a/hphp/test/slow/hack_arr_compat/split_misc_notices.php b/hphp/test/slow/hack_arr_compat/split_misc_notices.php new file mode 100644 index 0000000000000..deabe62a4f4c5 --- /dev/null +++ b/hphp/test/slow/hack_arr_compat/split_misc_notices.php @@ -0,0 +1,10 @@ +> +function main () { + $a = darray["" => "empty string", 1 => "one", 0 => "zero"]; + var_dump($a + darray["more stuff" => "yep"]); + var_dump($a[null]); + var_dump($a[true]); + var_dump($a[0.0]); +} diff --git a/hphp/test/slow/hack_arr_compat/split_misc_notices.php.expectf b/hphp/test/slow/hack_arr_compat/split_misc_notices.php.expectf new file mode 100644 index 0000000000000..5e7c93768c76d --- /dev/null +++ b/hphp/test/slow/hack_arr_compat/split_misc_notices.php.expectf @@ -0,0 +1,19 @@ +array(4) { + [""]=> + string(12) "empty string" + [1]=> + string(3) "one" + [0]=> + string(4) "zero" + ["more stuff"]=> + string(3) "yep" +} + +Notice: Hack Array Compat: Implicit conversion of null to array key in %s on line %d +string(12) "empty string" + +Notice: Hack Array Compat: Implicit conversion of bool to array key in %s on line %d +string(3) "one" + +Notice: Hack Array Compat: Implicit conversion of double to array key in %s on line %d +string(4) "zero" diff --git a/hphp/test/slow/hack_arr_compat/split_misc_notices.php.hphp_opts b/hphp/test/slow/hack_arr_compat/split_misc_notices.php.hphp_opts new file mode 100644 index 0000000000000..01fd270e97ab7 --- /dev/null +++ b/hphp/test/slow/hack_arr_compat/split_misc_notices.php.hphp_opts @@ -0,0 +1 @@ +-vRuntime.Eval.HackArrCompatNotices=true -d hhvm.php7.all=0 diff --git a/hphp/test/slow/hack_arr_compat/split_misc_notices.php.opts b/hphp/test/slow/hack_arr_compat/split_misc_notices.php.opts new file mode 100644 index 0000000000000..4af9d31d6c4d6 --- /dev/null +++ b/hphp/test/slow/hack_arr_compat/split_misc_notices.php.opts @@ -0,0 +1,3 @@ +-vEval.HackArrCompatNotices=1 +-vEval.HackArrCompatCheckArrayKeyCast=1 + diff --git a/hphp/test/slow/hack_arr_compat/split_misc_notices_2.php b/hphp/test/slow/hack_arr_compat/split_misc_notices_2.php new file mode 100644 index 0000000000000..deabe62a4f4c5 --- /dev/null +++ b/hphp/test/slow/hack_arr_compat/split_misc_notices_2.php @@ -0,0 +1,10 @@ +> +function main () { + $a = darray["" => "empty string", 1 => "one", 0 => "zero"]; + var_dump($a + darray["more stuff" => "yep"]); + var_dump($a[null]); + var_dump($a[true]); + var_dump($a[0.0]); +} diff --git a/hphp/test/slow/hack_arr_compat/split_misc_notices_2.php.expectf b/hphp/test/slow/hack_arr_compat/split_misc_notices_2.php.expectf new file mode 100644 index 0000000000000..bc558dd207d79 --- /dev/null +++ b/hphp/test/slow/hack_arr_compat/split_misc_notices_2.php.expectf @@ -0,0 +1,15 @@ + +Notice: Hack Array Compat: Using + operator on arrays in %s on line %d +array(4) { + [""]=> + string(12) "empty string" + [1]=> + string(3) "one" + [0]=> + string(4) "zero" + ["more stuff"]=> + string(3) "yep" +} +string(12) "empty string" +string(3) "one" +string(4) "zero" diff --git a/hphp/test/slow/hack_arr_compat/split_misc_notices_2.php.hphp_opts b/hphp/test/slow/hack_arr_compat/split_misc_notices_2.php.hphp_opts new file mode 100644 index 0000000000000..01fd270e97ab7 --- /dev/null +++ b/hphp/test/slow/hack_arr_compat/split_misc_notices_2.php.hphp_opts @@ -0,0 +1 @@ +-vRuntime.Eval.HackArrCompatNotices=true -d hhvm.php7.all=0 diff --git a/hphp/test/slow/hack_arr_compat/split_misc_notices_2.php.opts b/hphp/test/slow/hack_arr_compat/split_misc_notices_2.php.opts new file mode 100644 index 0000000000000..b7e4926fd99e6 --- /dev/null +++ b/hphp/test/slow/hack_arr_compat/split_misc_notices_2.php.opts @@ -0,0 +1,3 @@ +-vEval.HackArrCompatNotices=1 +-vEval.HackArrCompatCheckArrayPlus=1 + diff --git a/hphp/test/slow/inline-interp-stack.php.hphp_opts b/hphp/test/slow/inline-interp-stack.php.hphp_opts index bfdadcce1114b..36bcbd3ae8e13 100644 --- a/hphp/test/slow/inline-interp-stack.php.hphp_opts +++ b/hphp/test/slow/inline-interp-stack.php.hphp_opts @@ -1 +1 @@ --vHackArrCompatNotices=1 -vHackArrCompatCheckMisc=1 -vJitEnableRenameFunction=0 -vRuntime.Hack.Lang.DisallowDynamicVarEnvFuncs=1 -vHackArrCompatCheckIntishCast=1 +-vHackArrCompatNotices=1 -vHackArrCompatCheckArrayKeyCast=1 -vJitEnableRenameFunction=0 -vRuntime.Hack.Lang.DisallowDynamicVarEnvFuncs=1 -vHackArrCompatCheckIntishCast=1 diff --git a/hphp/test/slow/inline-interp-stack.php.opts b/hphp/test/slow/inline-interp-stack.php.opts index da2eda6189bb0..fe7a908281f37 100644 --- a/hphp/test/slow/inline-interp-stack.php.opts +++ b/hphp/test/slow/inline-interp-stack.php.opts @@ -1 +1 @@ --vEval.HackArrCompatNotices=1 -vEval.HackArrCompatCheckMisc=1 -vEval.JitEnableRenameFunction=0 -vHack.Lang.DisallowDynamicVarEnvFuncs=1 -vEval.HackArrCompatCheckIntishCast=1 +-vEval.HackArrCompatNotices=1 -vEval.HackArrCompatCheckArrayKeyCast=1 -vEval.JitEnableRenameFunction=0 -vHack.Lang.DisallowDynamicVarEnvFuncs=1 -vEval.HackArrCompatCheckIntishCast=1