Skip to content

Commit

Permalink
Fix autoload logic in hphpi for interfaces
Browse files Browse the repository at this point in the history
Summary:
When I checked in some fixes to the error handling system, one of
the flib tests that was passing under hphpi started failing. After
doing some investigation I found that the failure was caused by a
bug in hphpi's autoload logic. Specifically, hphpi was not
autoloading interfaces correctly. This lead to incorrect results
when evaluating statements like "$obj instanceof SomeInterface",
because hphpi would return false even though $obj did implement
SomeInterface.

I also found a few places where autoload could get called twice
on the same class or the same interface, and I fixed these places
appropriately.

I have a test ~andrewparoski/hphptests/reflectbug/reflectbug.php
that tests autoloading, instanceof, and the ReflectionClass.
Unfortunately we don't currently have a way to add tests that
involve multiple PHP files. I will open a separate bug to update
the test infrastructure to support multiple PHP files, and then
I will check in the reflectbug.php test.

Task ID: #

Blame Rev:

Reviewers: iproctor, mwilliams

CC:

Test Plan:
reflectbug.php
make fast_tests
Run www under hphpi and browse the site

Revert Plan:

Tags: hphp

- begin *PUBLIC* platform impact section -
Bugzilla: #
- end platform impact -

DiffCamp Revision: 122397
  • Loading branch information
andrewparoski authored and macvicar committed Jun 12, 2010
1 parent 4e70694 commit b4f82c2
Show file tree
Hide file tree
Showing 5 changed files with 15 additions and 3 deletions.
7 changes: 7 additions & 0 deletions src/runtime/eval/ast/class_statement.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,12 @@ const ClassStatement *ClassStatement::parentStatement() const {
return NULL;
}

void ClassStatement::loadInterfaceStatements() const {
for (unsigned int i = 0; i < m_basesVec.size(); ++i) {
RequestEvalState::findClass(m_basesVec[i].c_str(), true);
}
}

void ClassStatement::
loadMethodTable(ClassEvalState &ce) const {
hphp_const_char_imap<const MethodStatement*> &mtable = ce.getMethodTable();
Expand Down Expand Up @@ -569,6 +575,7 @@ void ClassStatement::toArray(Array &props, Array &vals) const {

void ClassStatement::semanticCheck(const ClassStatement *cls)
const {
loadInterfaceStatements();
const ClassStatement *parent = parentStatement();
if (cls) {
if (getModifiers() & (Interface|Abstract)) {
Expand Down
1 change: 1 addition & 0 deletions src/runtime/eval/ast/class_statement.h
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@ class ClassStatement : public Statement {
const std::string &lname() const { return m_lname; }
const std::string &parent() const { return m_parent; }
const ClassStatement *parentStatement() const;
void loadInterfaceStatements() const;
void setModifiers(int m) { m_modifiers = m; }
int getModifiers() const { return m_modifiers; }
void addBases(const std::vector<String> &bases);
Expand Down
4 changes: 3 additions & 1 deletion src/runtime/eval/ext/ext.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -244,6 +244,7 @@ Variant EvalClassExists::invoke(CArrRef params) const {
String cname = params.rvalAt(0);
if (!f_class_exists(cname, false)) {
if ((params.size() == 1 || params.rvalAt(1).toBoolean()) &&
!f_interface_exists(cname, false) &&
eval_try_autoload(cname.data())) {
return f_class_exists(cname, false);
}
Expand All @@ -256,6 +257,7 @@ Variant EvalInterfaceExists::invoke(CArrRef params) const {
String cname = params.rvalAt(0);
if (!f_interface_exists(cname, false)) {
if ((params.size() == 1 || params.rvalAt(1).toBoolean()) &&
!f_class_exists(cname, false) &&
eval_try_autoload(cname.data())) {
return f_interface_exists(cname, false);
}
Expand All @@ -272,7 +274,7 @@ Variant EvalGetDefinedVars::invokeImpl(VariableEnvironment &env,

Variant EvalHphpGetClassInfo::invoke(CArrRef params) const {
String cname = params.rvalAt(0);
if (!f_class_exists(cname)) {
if (!f_class_exists(cname) && !f_interface_exists(cname)) {
eval_try_autoload(cname.data());
}
return f_hphp_get_class_info(cname);
Expand Down
4 changes: 3 additions & 1 deletion src/runtime/eval/runtime/eval_state.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -198,7 +198,9 @@ const ClassStatement *RequestEvalState::findClass(const char *name,
if (it != self->m_classes.end()) {
return it->second.getClass();
}
if (autoload && !ClassInfo::HasClass(name) && eval_try_autoload(name)) {
if (autoload &&
(!ClassInfo::HasClass(name) && !ClassInfo::HasInterface(name)) &&
eval_try_autoload(name)) {
return findClass(name, false);
}
return NULL;
Expand Down
2 changes: 1 addition & 1 deletion src/system/gen/php/classes/reflection.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4400,7 +4400,7 @@ Variant c_reflectionclass::t_issubclassof(Variant v_cls) {
}
{
const Object &tmp72((toObject(t_getparentclass())));
return tmp72-> BIND_CLASS_ARROW(ObjectData) o_invoke_few_args("isSubclassOf", 0x373333991926C97ELL, 1, v_cls);
return wrap_variant(tmp72-> BIND_CLASS_ARROW(ObjectData) o_invoke_few_args("isSubclassOf", 0x373333991926C97ELL, 1, v_cls));
}
} /* function */
/* SRC: classes/reflection.php line 460 */
Expand Down

0 comments on commit b4f82c2

Please sign in to comment.