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
facebook · Oct 15, 2018 · a8caa5c · a8caa5c
1 parent 2fb2a32
commit a8caa5c
Show file tree

Hide file tree

Showing 28 changed files with 98 additions and 26 deletions.
diff --git 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
@@ -273,7 +273,8 @@ std::pair<std::vector<std::unique_ptr<UnitEmitter>>,
     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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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<KindOfPersistentString>(staticEmptyString()),

diff --git 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
@@ -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
@@ -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
@@ -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
@@ -1643,7 +1643,7 @@ inline ArrayData* SetElemArrayPre(ArrayData* a,
       a, const_cast<StringData*>(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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -0,0 +1,10 @@
+<?hh
+
+<<__EntryPoint>>
+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
@@ -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
@@ -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
@@ -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
@@ -0,0 +1,10 @@
+<?hh
+
+<<__EntryPoint>>
+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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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