Skip to content

Commit

Permalink
NullSafe props, part 1: Runtime
Browse files Browse the repository at this point in the history
Summary: This diff adds support of NullSafe properties to the runtime (see also {D1875131}).

===Overview===

- Object property AST node support
- Bytecode emitter support
- Bytecode interpreter support
- Bytecode verifier support
- Runtime errors for mutation cases of NullSafe props
- Parse time errors
- JIT support

===Details==

The new `QT` member code is introduced in the `CGetM` instruction:

- QT:<id> - a null-safe version of PT:<id> (a litstr by an immediate id as a prop)

Example:

  $x = null;
  $x?->y;

Generated bytecode:

  Pseudo-main at 0
  maxStackCells: 2
  numLocals: 1
  numIterators: 0
    // line 3
      0: Null
      1: SetL 0
      3: PopC
    // line 4
      4: CGetM <L:0 QT:"y"> # note QT
     20: PopC
     21: Int 1
     30: RetC
  Pseudo-main at 0
  maxStackCells: 2
  numLocals: 1
  numIterators: 0

Runtime/parse checks that forbids several use-cases:

  $x?->y;        // allow
  isset($x?->y); // allow
  empty($x?->y); // allow

  $x?->y = 1;    // disallow
  $x?->y += 1;   // disallow
  $x?->y++;      // disallow
  unset($x?->y); // disallow

  function byRef(&x) {}
  byRef($x?->y); // disallow

Reviewed By: @jdelong

Differential Revision: D1818128
  • Loading branch information
DmitrySoshnikov authored and hhvm-bot committed Mar 11, 2015
1 parent 349fd25 commit 35819cd
Show file tree
Hide file tree
Showing 63 changed files with 1,090 additions and 287 deletions.
42 changes: 35 additions & 7 deletions hphp/compiler/analysis/emitter.cpp
Expand Up @@ -49,7 +49,6 @@
#include "hphp/compiler/expression/modifier_expression.h"
#include "hphp/compiler/expression/new_object_expression.h"
#include "hphp/compiler/expression/object_method_expression.h"
#include "hphp/compiler/expression/object_property_expression.h"
#include "hphp/compiler/expression/parameter_expression.h"
#include "hphp/compiler/expression/qop_expression.h"
#include "hphp/compiler/expression/scalar_expression.h"
Expand Down Expand Up @@ -156,6 +155,7 @@ namespace StackSym {
static const char S = 0x60; // Static property marker
static const char M = 0x70; // Non elem/prop/W part of M-vector
static const char K = 0x80; // Marker for information about a class base
static const char Q = 0x90; // NullSafe Property marker

static const char CN = C | N;
static const char CG = C | G;
Expand Down Expand Up @@ -189,6 +189,7 @@ namespace StackSym {
case StackSym::E: res += "E"; break;
case StackSym::W: res += "W"; break;
case StackSym::P: res += "P"; break;
case StackSym::Q: res += "Q"; break;
case StackSym::S: res += "S"; break;
case StackSym::K: res += "K"; break;
default: break;
Expand Down Expand Up @@ -1983,7 +1984,8 @@ void EmitterVisitor::popEvalStackMMany() {
char sym = m_evalStack.top();
char symFlavor = StackSym::GetSymFlavor(sym);
char marker = StackSym::GetMarker(sym);
if (marker == StackSym::E || marker == StackSym::P) {
if (marker == StackSym::E || marker == StackSym::P ||
marker == StackSym::Q) {
if (symFlavor != StackSym::C && symFlavor != StackSym::L &&
symFlavor != StackSym::T && symFlavor != StackSym::I) {
InvariantViolation(
Expand Down Expand Up @@ -4370,6 +4372,16 @@ bool EmitterVisitor::visit(ConstructPtr node) {
case Expression::KindOfObjectPropertyExpression: {
ObjectPropertyExpressionPtr op(
static_pointer_cast<ObjectPropertyExpression>(node));
if (op->isNullSafe() &&
op->hasAnyContext(
Expression::RefValue
| Expression::LValue
| Expression::DeepReference
) && !op->hasContext(Expression::InvokeArgument)
) {
throw IncludeTimeFatalException(op,
Strings::NULLSAFE_PROP_WRITE_ERROR);
}
ExpressionPtr obj = op->getObject();
SimpleVariablePtr sv = dynamic_pointer_cast<SimpleVariable>(obj);
if (sv && sv->isThis() && sv->hasContext(Expression::ObjectContext)) {
Expand All @@ -4387,7 +4399,12 @@ bool EmitterVisitor::visit(ConstructPtr node) {
Expression::ObjectContext)) {
m_tempLoc = op->getLocation();
}
markProp(e);
markProp(
e,
op->isNullSafe()
? PropAccessType::NullSafe
: PropAccessType::Normal
);
return true;
}

Expand Down Expand Up @@ -4872,7 +4889,8 @@ int EmitterVisitor::scanStackForLocation(int iLast) {
for (int i = iLast; i >= 0; --i) {
char marker = StackSym::GetMarker(m_evalStack.get(i));
if (marker != StackSym::E && marker != StackSym::W &&
marker != StackSym::P && marker != StackSym::M) {
marker != StackSym::P && marker != StackSym::M &&
marker != StackSym::Q) {
return i;
}
}
Expand Down Expand Up @@ -4998,6 +5016,10 @@ void EmitterVisitor::buildVectorImm(std::vector<uchar>& vectorImm,
vectorImm.push_back(MPC);
}
} break;
case StackSym::Q: {
assert(symFlavor == StackSym::T);
vectorImm.push_back(MQT);
} break;
case StackSym::S: {
assert(false);
}
Expand Down Expand Up @@ -5981,7 +6003,7 @@ void EmitterVisitor::markNewElem(Emitter& e) {
m_evalStack.push(StackSym::W);
}

void EmitterVisitor::markProp(Emitter& e) {
void EmitterVisitor::markProp(Emitter& e, PropAccessType propAccessType) {
if (m_evalStack.empty()) {
InvariantViolation(
"Emitter encountered an empty evaluation stack inside "
Expand All @@ -5991,7 +6013,13 @@ void EmitterVisitor::markProp(Emitter& e) {
}
char sym = m_evalStack.top();
if (sym == StackSym::C || sym == StackSym::L || sym == StackSym::T) {
m_evalStack.set(m_evalStack.size()-1, (sym | StackSym::P));
m_evalStack.set(
m_evalStack.size()-1,
(sym | (propAccessType == PropAccessType::NullSafe
? StackSym::Q
: StackSym::P
))
);
} else {
InvariantViolation(
"Emitter encountered an unsupported StackSym \"%s\" on "
Expand Down Expand Up @@ -6928,7 +6956,7 @@ void EmitterVisitor::emitMemoizeProp(Emitter& e,
m_evalStack.setKnownCls(m_curFunc->pce()->name(), false);
m_evalStack.push(StackSym::T);
m_evalStack.setString(m_curFunc->memoizePropName);
markProp(e);
markProp(e, PropAccessType::Normal);
}

assert(numParams <= paramIDs.size());
Expand Down
4 changes: 3 additions & 1 deletion hphp/compiler/analysis/emitter.h
Expand Up @@ -24,6 +24,8 @@
#include "hphp/compiler/statement/trait_prec_statement.h"
#include "hphp/compiler/statement/trait_alias_statement.h"
#include "hphp/compiler/statement/typedef_statement.h"
#include "hphp/compiler/expression/object_property_expression.h"
#include "hphp/parser/parser.h"

#include "hphp/runtime/vm/func.h"
#include "hphp/runtime/vm/func-emitter.h"
Expand Down Expand Up @@ -717,7 +719,7 @@ class EmitterVisitor {

void markElem(Emitter& e);
void markNewElem(Emitter& e);
void markProp(Emitter& e);
void markProp(Emitter& e, PropAccessType propAccessType);
void markSProp(Emitter& e);
void markName(Emitter& e);
void markNameSecond(Emitter& e);
Expand Down
2 changes: 1 addition & 1 deletion hphp/compiler/analysis/select_rewriters.cpp
Expand Up @@ -166,7 +166,7 @@ ObjectPropertyExpressionPtr getResultColumn(
);
ObjectPropertyExpressionPtr result(
new ObjectPropertyExpression(expr->getScope(),
expr->getLocation(), obj, propName)
expr->getLocation(), obj, propName, PropAccessType::Normal)
);
return result;
}
Expand Down
4 changes: 2 additions & 2 deletions hphp/compiler/expression/object_property_expression.cpp
Expand Up @@ -23,7 +23,6 @@
#include "hphp/compiler/analysis/file_scope.h"
#include "hphp/compiler/analysis/variable_table.h"
#include "hphp/compiler/option.h"
#include "hphp/compiler/expression/scalar_expression.h"
#include "hphp/compiler/expression/simple_variable.h"
#include "hphp/util/hash.h"
#include "hphp/parser/hphp.tab.hpp"
Expand All @@ -35,7 +34,7 @@ using namespace HPHP;

ObjectPropertyExpression::ObjectPropertyExpression
(EXPRESSION_CONSTRUCTOR_PARAMETERS,
ExpressionPtr object, ExpressionPtr property)
ExpressionPtr object, ExpressionPtr property, PropAccessType propAccessType)
: Expression(
EXPRESSION_CONSTRUCTOR_PARAMETER_VALUES(ObjectPropertyExpression)),
LocalEffectsContainer(AccessorEffect),
Expand All @@ -44,6 +43,7 @@ ObjectPropertyExpression::ObjectPropertyExpression
m_propSymValid = false;
m_object->setContext(Expression::ObjectContext);
m_object->setContext(Expression::AccessContext);
m_nullsafe = (propAccessType == PropAccessType::NullSafe);
}

ExpressionPtr ObjectPropertyExpression::clone() {
Expand Down
6 changes: 5 additions & 1 deletion hphp/compiler/expression/object_property_expression.h
Expand Up @@ -18,6 +18,7 @@
#define incl_HPHP_OBJECT_PROPERTY_EXPRESSION_H_

#include "hphp/compiler/expression/expression.h"
#include "hphp/parser/parser.h"

namespace HPHP {
///////////////////////////////////////////////////////////////////////////////
Expand All @@ -31,7 +32,8 @@ class ObjectPropertyExpression : public Expression,
public LocalEffectsContainer {
public:
ObjectPropertyExpression(EXPRESSION_CONSTRUCTOR_PARAMETERS,
ExpressionPtr object, ExpressionPtr property);
ExpressionPtr object, ExpressionPtr property,
PropAccessType propAccessType);

DECLARE_EXPRESSION_VIRTUAL_FUNCTIONS;
DECL_AND_IMPL_LOCAL_EFFECTS_METHODS;
Expand All @@ -43,6 +45,7 @@ class ObjectPropertyExpression : public Expression,

ExpressionPtr getObject() { return m_object;}
ExpressionPtr getProperty() { return m_property;}
bool isNullSafe() const { return m_nullsafe; }

bool isTemporary() const;
bool isNonPrivate(AnalysisResultPtr ar);
Expand All @@ -53,6 +56,7 @@ class ObjectPropertyExpression : public Expression,

unsigned m_valid : 1;
unsigned m_propSymValid : 1;
unsigned m_nullsafe : 1;

Symbol *m_propSym;
ClassScopeRawPtr m_objectClass;
Expand Down

0 comments on commit 35819cd

Please sign in to comment.