Allow index chaining

Summary:
This fixes the "bug" in PHP's grammar where you aren't allowed to use the [...] operator on the return value of a function. It rewrites all instances which would be syntax errors into ##__xhp_idx($arr, $index)##. __xhp_idx is implemented in ext.cpp in C and is probably faster than the idx we made in PHP.

It behaves exactly as the [...] operator would.

If you want to ignore undefined index warnings you can do ##@foo()['etc']## and the @ will only apply to the __xhp_idx call.

One thing to watch out for is that each chained [...] on the end of a function is a function call. So if you're doing something like ##foo()['bar']['etc']['muk']## you get 3 calls to __xhp_idx.

TODO: Add ini entry to disable this feature (only need to set a flag).

Reviewed By: dweatherford

Test Plan:
marcel@dev050 ~/xhp $ cat honk.php
  <?php
  error_reporting(E_ALL);
  function foo() {
    return array(
      'bar' => 'etc',
    );
  }

  echo foo()['bar'] . "\n";
  echo foo()['etc'];
  echo @foo()['etc'];
  marcel@dev050 ~/xhp $ php honk.php
  etc
  PHP Notice:  Undefined index: etc in /home/marcel/xhp/honk.php on line 10

I also ran the whole codebase through xhpize to make sure no lines were getting rewritten that didn't need it.

Revert: OK

DiffCamp Revision: 66576

ext.cpp

@@ -3,6 +3,7 @@
 #include "zend.h"
 #include "zend_API.h"
 #include "zend_compile.h"
+#include "zend_operators.h"
 #include "zend_hash.h"
 #include "zend_extensions.h"
 #include "ext/standard/info.h"
@@ -233,17 +234,70 @@ static PHP_MINFO_FUNCTION(xhp) {
   php_info_print_table_end();
 }
 
+ZEND_FUNCTION(__xhp_idx) {
+  zval *dict, *offset;
+  if (zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC, "az", &dict, &offset) == FAILURE) {
+    RETURN_NULL();
+  }
+  zval **value;
+  switch (Z_TYPE_P(offset)) {
+    case IS_RESOURCE:
+      zend_error(E_STRICT, "Resource ID#%ld used as offset, casting to integer (%ld)", Z_LVAL_P(offset), Z_LVAL_P(offset));
+      /* Fall Through */
+    case IS_DOUBLE:
+    case IS_BOOL:
+    case IS_LONG:
+      long loffset;
+      if (Z_TYPE_P(offset) == IS_DOUBLE) {
+        loffset = (long)Z_DVAL_P(offset);
+      } else {
+        loffset = Z_LVAL_P(offset);
+      }
+      if (zend_hash_index_find(Z_ARRVAL_P(dict), loffset, (void **) &value) == SUCCESS) {
+        break;
+      }
+      zend_error(E_NOTICE, "Undefined offset: %ld", loffset);
+      RETURN_NULL();
+
+    case IS_STRING:
+      if (zend_symtable_find(Z_ARRVAL_P(dict), offset->value.str.val, offset->value.str.len+1, (void **) &value) == SUCCESS) {
+        break;
+      }
+      zend_error(E_NOTICE, "Undefined index: %s", offset->value.str.val);
+      RETURN_NULL();
+
+    case IS_NULL:
+      if (zend_hash_find(Z_ARRVAL_P(dict), "", sizeof(""), (void **) &value) == SUCCESS) {
+        break;
+      }
+      zend_error(E_NOTICE, "Undefined index: ");
+      RETURN_NULL();
+
+    default:
+      zend_error(E_WARNING, "Illegal offset type");
+      RETURN_NULL();
+      break;
+  }
+  *return_value = **value;
+  zval_copy_ctor(return_value);
+}
+
+zend_function_entry xhp_functions[] = {
+  ZEND_FE(__xhp_idx, NULL)
+  {NULL, NULL, NULL}
+};
+
 zend_module_entry xhp_module_entry = {
-	STANDARD_MODULE_HEADER,
-	PHP_XHP_EXTNAME,
-	NULL,
-	PHP_MINIT(xhp),
-	NULL,
-	NULL,
-	NULL,
-	PHP_MINFO(xhp),
-	PHP_XHP_VERSION,
-	STANDARD_MODULE_PROPERTIES
+  STANDARD_MODULE_HEADER,
+  PHP_XHP_EXTNAME,
+  xhp_functions,
+  PHP_MINIT(xhp),
+  NULL,
+  NULL,
+  NULL,
+  PHP_MINFO(xhp),
+  PHP_XHP_VERSION,
+  STANDARD_MODULE_PROPERTIES
 };
 
 #ifdef COMPILE_DL_XHP

ext.hpp

@@ -4,7 +4,7 @@
 #endif
 #include "php.h"
 
-#define PHP_XHP_VERSION "1.2.2"
+#define PHP_XHP_VERSION "1.3.0"
 #define PHP_XHP_EXTNAME "xhp"
 
 extern zend_module_entry xhp_module_entry;

xhp/parser.y

@@ -40,7 +40,9 @@ static void replacestr(string &source, const string &find, const string &rep) {
 
 %}
 
-%expect 2
+%expect 9
+// 2: PHP's if/else grammar
+// 7: expr '[' dim_offset ']' -- shift will default to first grammar
 %name-prefix = "xhp"
 %pure-parser
 %parse-param { void* yyscanner }
@@ -1851,6 +1853,23 @@ fully_qualified_class_name:
   }
 ;
 
+// Fix the "bug" in PHP's grammar where you can't chain the [] operator on a
+// function call.
+// This introduces some shift/reduce conflicts. We want the shift here to fall
+// back to regular PHP grammar. In the case where it's an extension of the PHP
+// grammar our code gets picked up.
+expr:
+  expr '[' dim_offset ']' {
+    if (yyextra->idx_chain) {
+      yyextra->used = true;
+      $$ = "__xhp_idx(" + $1 + ", " + $3 + ")";
+    } else {
+      $$ = $1 + $2 + $3 + $4;
+    }
+  }
+;
+
+
 %%
 
 const char* yytokname(int tok) {

xhp/xhp.hpp

@@ -14,13 +14,15 @@ class yy_extra_type {
       used = false;
       short_tags = true;
       asp_tags = false;
+      idx_chain = false;
       has_doc_block = false;
       expecting_xhp_class_statements = false;
       pushStack();
     }
 
     bool short_tags; // `short_open_tag` in php.ini
     bool asp_tags; // `asp_tags` in php.ini
+    bool idx_chain; // allow code like `foo()['bar']`
     size_t first_lineno; // line number before scanning the current token
     size_t lineno; // current line number being scanned.
     std::string error; // description of error (if terminated true)

xhp/xhp_preprocess.cpp

@@ -36,10 +36,19 @@ XHPResult xhp_preprocess(string &in, string &out, bool isEval, string &errDescri
     } else if (*jj == ':') { // :fb:thing
       if ((jj[1] >= 'a' && jj[1] <= 'z') || (jj[1] >= 'A' && jj[1] <= 'Z')) {
         maybe_xhp = true;
+        break;
       }
     } else if (!memcmp(jj, "element", 7)) {
       maybe_xhp = true;
       break;
+    } else if (*jj == ')') { // foo()['etc']
+      do {
+        ++jj;
+      } while (*jj == ' ' || *jj == '\r' || *jj == '\n' || *jj == '\t');
+      if (*jj == '[') {
+        maybe_xhp = true;
+        break;
+      }
     }
   }
 
@@ -52,6 +61,7 @@ XHPResult xhp_preprocess(string &in, string &out, bool isEval, string &errDescri
   void* scanner;
   code_rope new_code;
   yy_extra_type extra;
+  extra.idx_chain = true;
   extra.insert_token = isEval ? T_OPEN_TAG_FAKE : 0;
   xhplex_init(&scanner);
   xhpset_extra(&extra, scanner);