From 16a372962c31493e27d4ab04c7b2f7fac5f5e8cd Mon Sep 17 00:00:00 2001
From: Drew Paroski <andrewparoski@fb.com>
Date: Tue, 14 May 2013 22:09:27 -0700
Subject: [PATCH] Tolerate "isset($c->prop)" for collections

Currently collections define magic __get, __set, __isset, and __unset
methods which will throw exceptions if any property access is attempted
on collections (such as "isset($vec->prop)" and "$x = $vec->prop").

However, there is existing code that uses isset() to check if various
objects have a property with a certain name, and it feels like throwing
an exception here is a little harsh. This diff updates collections to
be tolerant of reading properties.
---
 hphp/runtime/base/builtin_functions.cpp         |  2 +-
 hphp/runtime/ext/ext_collections.cpp            |  2 +-
 hphp/test/slow/collection_classes/805.php       | 17 ++++++++++++++---
 .../test/slow/collection_classes/805.php.expect | 11 ++++++++++-
 4 files changed, 26 insertions(+), 6 deletions(-)

diff --git a/hphp/runtime/base/builtin_functions.cpp b/hphp/runtime/base/builtin_functions.cpp
index c72220eb6..7ace9bb8d 100644
--- a/hphp/runtime/base/builtin_functions.cpp
+++ b/hphp/runtime/base/builtin_functions.cpp
@@ -453,7 +453,7 @@ void throw_collection_modified() {
 
 void throw_collection_property_exception() {
   Object e(SystemLib::AllocInvalidOperationExceptionObject(
-    "Cannot access property on a collection"));
+    "Cannot access a property on a collection"));
   throw e;
 }
 
diff --git a/hphp/runtime/ext/ext_collections.cpp b/hphp/runtime/ext/ext_collections.cpp
index 3b76c75f9..ec26ba312 100644
--- a/hphp/runtime/ext/ext_collections.cpp
+++ b/hphp/runtime/ext/ext_collections.cpp
@@ -4567,7 +4567,7 @@ void c_PairIterator::t_rewind() {
     throw_collection_property_exception(); \
   } \
   bool c_##cls::t___isset(Variant name) { \
-    throw_collection_property_exception(); \
+    return false; \
   } \
   Variant c_##cls::t___unset(Variant name) { \
     throw_collection_property_exception(); \
diff --git a/hphp/test/slow/collection_classes/805.php b/hphp/test/slow/collection_classes/805.php
index 1b646d380..69f20f570 100644
--- a/hphp/test/slow/collection_classes/805.php
+++ b/hphp/test/slow/collection_classes/805.php
@@ -8,14 +8,25 @@ try {
     $obj = new $cls;
     try {
       $x = $obj->foo;
+      echo "get: ";
+      var_dump($x);
     } catch (RuntimeException $e) {
-      echo $i;
+      echo "get throws, i = ";
+      var_dump($i);
+    }
+    try {
+      $x = isset($obj->foo);
+      echo "isset: ";
+      var_dump($x);
+    } catch (RuntimeException $e) {
+      echo "isset throws, i = ";
+      var_dump($i);
     }
-    ++$i;
     try {
       $obj->foo = 123;
     } catch (RuntimeException $e) {
-      echo $i;
+      echo "set throws, i = ";
+      var_dump($i);
     }
     ++$i;
   }
diff --git a/hphp/test/slow/collection_classes/805.php.expect b/hphp/test/slow/collection_classes/805.php.expect
index 14bb7d264..56af62f9d 100644
--- a/hphp/test/slow/collection_classes/805.php.expect
+++ b/hphp/test/slow/collection_classes/805.php.expect
@@ -1 +1,10 @@
-012345Done
+get throws, i = int(0)
+isset: bool(false)
+set throws, i = int(0)
+get throws, i = int(1)
+isset: bool(false)
+set throws, i = int(1)
+get throws, i = int(2)
+isset: bool(false)
+set throws, i = int(2)
+Done