Skip to content

Commit

Permalink
Add new TestTraits to TestCodeRun and fix hphpc accordingly
Browse files Browse the repository at this point in the history 
Summary:
I've recently learned a few more nuances about traits.  I added some
new tests to TestCodeRun, and fixed the following cases:

1. abstract trait methods may be provided by a base class of the class
   using the trait

2. abstract trait methods may be provided by other traits used in the
   same class

3. trait methods and static properties can be accessed via
   trait_name::blah.

Item 3 abore required enabling code generation for trait classes.

Test Plan:
make fast_tests
make slow_tests
built www and checked output CPP code against previous

Reviewers: mwilliams, myang, qigao

Reviewed By: mwilliams

CC: andrewparoski, ps, mwilliams

Differential Revision: 350654

Task ID: 758990
  • Loading branch information
@ottoni
ottoni authored and macvicar committed Nov 29, 2011
1 parent 13c6712 commit d867f56
Show file tree
Hide file tree
Showing 8 changed files with 309 additions and 37 deletions.
10 changes: 5 additions & 5 deletions src/compiler/analysis/analysis_result.cpp
View file
Expand Up @@ -2596,7 +2596,7 @@ void AnalysisResult::outputCPPExtClassImpl(CodeGenerator &cg) {
iter != m_systemClasses.end(); ++iter) {
ClassScopePtr cls = iter->second;
bool extension = cls->getAttribute(ClassScope::Extension);
if (cls->isInterface() || cls->isTrait()) continue;
if (cls->isInterface()) continue;

classes.push_back(cls->getOriginalName().c_str());
merged[cls->getName()].push_back(cls);
Expand All @@ -2615,7 +2615,7 @@ void AnalysisResult::outputCPPExtClassImpl(CodeGenerator &cg) {
for (ClassScopePtrVec::const_iterator iter2 = iter->second.begin();
iter2 != iter->second.end(); ++iter2) {
ClassScopePtr cls = *iter2;
if (!cls->isInterface() && !cls->isTrait()) {
if (!cls->isInterface()) {
classes.push_back(cls->getOriginalName().c_str());
break;
}
Expand Down Expand Up @@ -3112,7 +3112,7 @@ void AnalysisResult::outputCPPDynamicClassTables(
for (ClassScopePtrVec::const_iterator iter2 = iter->second.begin();
iter2 != iter->second.end(); ++iter2) {
cls = *iter2;
if (cls->isUserClass() && !cls->isInterface() && !cls->isTrait()) {
if (cls->isUserClass() && !cls->isInterface()) {
classes.push_back(cls->getOriginalName().c_str());
classScopes[cls->getName()].push_back(cls);
if (!cls->isRedeclaring()) {
Expand All @@ -3125,14 +3125,14 @@ void AnalysisResult::outputCPPDynamicClassTables(
}
if (system) {
BOOST_FOREACH(tie(n, cls), m_systemClasses) {
if (!cls->isInterface() && !cls->isSepExtension() && !cls->isTrait()) {
if (!cls->isInterface() && !cls->isSepExtension()) {
classes.push_back(cls->getOriginalName().c_str());
}
}
outputCPPExtClassImpl(cg);
} else {
BOOST_FOREACH(tie(n, cls), m_systemClasses) {
if (!cls->isInterface() && cls->isSepExtension() && !cls->isTrait()) {
if (!cls->isInterface() && cls->isSepExtension()) {
classes.push_back(cls->getOriginalName().c_str());
cls->outputCPPDynamicClassDecl(cg);
cls->outputCPPGlobalTableWrappersDecl(cg, ar);
Expand Down
62 changes: 46 additions & 16 deletions src/compiler/analysis/class_scope.cpp
View file
Expand Up @@ -363,6 +363,14 @@ void ClassScope::importTraitMethod(const TraitMethod &traitMethod,
const string &origMethName = traitMethod.m_originalName;
ModifierExpressionPtr modifiers = traitMethod.m_modifiers;

// For abstract methods, simply return if method already exists in the class
if ((modifiers && modifiers->isAbstract()) ||
(!modifiers && meth->getModifiers()->isAbstract())) {
if (findFunction(ar, methName, true)) {
return;
}
}

// Check for errors before cloning
if (modifiers && modifiers->isStatic()) {
Compiler::Error(Compiler::InvalidAccessModifier, traitMethod.m_ruleStmt);
Expand Down Expand Up @@ -612,6 +620,37 @@ void ClassScope::applyTraitRules(AnalysisResultPtr ar) {
}
}

void ClassScope::removeImplTraitAbstractMethods(AnalysisResultPtr ar) {
for (MethodToTraitListMap::iterator iter = m_importMethToTraitMap.begin();
iter != m_importMethToTraitMap.end(); iter++) {

TraitMethodList& tMethList = iter->second;
// Check if there's any non-abstract method imported
bool hasNonAbstractMeth = false;
for (TraitMethodList::const_iterator traitMethIter = tMethList.begin();
traitMethIter != tMethList.end(); traitMethIter++) {
ModifierExpressionPtr modifiers = traitMethIter->m_modifiers ?
traitMethIter->m_modifiers : traitMethIter->m_method->getModifiers();
if (!(modifiers->isAbstract())) {
hasNonAbstractMeth = true;
break;
}
}
if (hasNonAbstractMeth) {
// Erase abstract declarations
for (TraitMethodList::iterator nextTraitIter = tMethList.begin();
nextTraitIter != tMethList.end(); ) {
TraitMethodList::iterator traitIter = nextTraitIter++;
ModifierExpressionPtr modifiers = traitIter->m_modifiers ?
traitIter->m_modifiers : traitIter->m_method->getModifiers();
if (modifiers->isAbstract()) {
tMethList.erase(traitIter);
}
}
}
}
}

void ClassScope::importUsedTraits(AnalysisResultPtr ar) {
if (m_traitStatus == FLATTENED) return;
if (m_traitStatus == BEING_FLATTENED) {
Expand Down Expand Up @@ -640,19 +679,21 @@ void ClassScope::importUsedTraits(AnalysisResultPtr ar) {
// Apply rules
applyTraitRules(ar);

// Check for trait abstract methods provided by other traits
removeImplTraitAbstractMethods(ar);

// Apply precedence of current class over used traits
for (map<string,TraitMethodList>::iterator
iter = m_importMethToTraitMap.begin();
for (MethodToTraitListMap::iterator iter = m_importMethToTraitMap.begin();
iter != m_importMethToTraitMap.end(); ) {
map<string,TraitMethodList>::iterator thisiter = iter;
MethodToTraitListMap::iterator thisiter = iter;
iter++;
if (findFunction(ar, thisiter->first, 0, 0) != FunctionScopePtr()) {
m_importMethToTraitMap.erase(thisiter);
}
}

// Actually import the methods
for (map<string,TraitMethodList>::const_iterator
for (MethodToTraitListMap::const_iterator
iter = m_importMethToTraitMap.begin();
iter != m_importMethToTraitMap.end(); iter++) {

Expand Down Expand Up @@ -894,7 +935,6 @@ void ClassScope::setSystem() {
}

bool ClassScope::needLazyStaticInitializer() {
if (isTrait()) return false;
return getVariables()->getAttribute(VariableTable::ContainsDynamicStatic) ||
getConstants()->hasDynamic();
}
Expand Down Expand Up @@ -1159,6 +1199,7 @@ void ClassScope::serialize(JSON::DocTarget::OutputStream &out) const {
if (m_kindOf == KindOfAbstractClass) mods |= ClassInfo::IsAbstract;
if (m_kindOf == KindOfFinalClass) mods |= ClassInfo::IsFinal;
if (m_kindOf == KindOfInterface) mods |= ClassInfo::IsInterface;
if (m_kindOf == KindOfTrait) mods |= ClassInfo::IsTrait;
ms.add("modifiers", mods);

FunctionScopePtrVec funcs;
Expand All @@ -1180,7 +1221,6 @@ void ClassScope::serialize(JSON::DocTarget::OutputStream &out) const {
}

void ClassScope::outputCPPDynamicClassDecl(CodeGenerator &cg) {
if (isTrait()) return;
string clsStr = getId();
const char *clsName = clsStr.c_str();
cg_printf("ObjectData *%s%s(%s) NEVER_INLINE;\n",
Expand All @@ -1198,7 +1238,6 @@ void ClassScope::outputCPPDynamicClassCreateDecl(CodeGenerator &cg) {

void ClassScope::outputCPPDynamicClassImpl(CodeGenerator &cg,
AnalysisResultPtr ar) {
if (isTrait()) return;
string clsStr = getId();
const char *clsName = clsStr.c_str();
cg_indentBegin("ObjectData *%s%s(%s) {\n",
Expand Down Expand Up @@ -2337,7 +2376,6 @@ string ClassScope::getHeaderFilename() {

void ClassScope::outputCPPHeader(AnalysisResultPtr ar,
CodeGenerator::Output output) {
if (isTrait()) return;
string filename = getHeaderFilename();
string root = ar->getOutputPath() + "/";
Util::mkdir(root + filename);
Expand Down Expand Up @@ -2479,7 +2517,6 @@ void ClassScope::outputCPPForwardHeader(CodeGenerator &cg,

void ClassScope::outputCPPSupportMethodsImpl(CodeGenerator &cg,
AnalysisResultPtr ar) {
if (isTrait()) return;
string clsNameStr = getId();
const char *clsName = clsNameStr.c_str();

Expand Down Expand Up @@ -2653,7 +2690,6 @@ void ClassScope::outputCPPStaticMethodWrappers(CodeGenerator &cg,
AnalysisResultPtr ar,
set<string> &done,
const char *cls) {
if (isTrait()) return;
for (FunctionScopePtrVec::const_iterator it = m_functionsVec.begin();
it != m_functionsVec.end(); ++it) {
const string &name = (*it)->getName();
Expand All @@ -2672,7 +2708,6 @@ void ClassScope::outputCPPStaticMethodWrappers(CodeGenerator &cg,

void ClassScope::outputCPPGlobalTableWrappersDecl(CodeGenerator &cg,
AnalysisResultPtr ar) {
if (isTrait()) return;
string id = getId();
cg_printf("extern const %sObjectStaticCallbacks %s%s;\n",
isRedeclaring() ? "Redeclared" : "",
Expand All @@ -2693,7 +2728,6 @@ string ClassScope::getClassPropTableId(AnalysisResultPtr ar) {

void ClassScope::outputCPPGlobalTableWrappersImpl(CodeGenerator &cg,
AnalysisResultPtr ar) {
if (isTrait()) return;
string id = getId();
string prop = getClassPropTableId(ar);
cg_indentBegin("const %sObjectStaticCallbacks %s%s = {\n",
Expand Down Expand Up @@ -2806,8 +2840,6 @@ void ClassScope::findJumpTableMethods(CodeGenerator &cg, AnalysisResultPtr ar,
void ClassScope::outputCPPMethodInvokeBareObjectSupport(
CodeGenerator &cg, AnalysisResultPtr ar,
FunctionScopePtr func, bool fewArgs) {
if (isTrait()) return;

const string &id(getId());
const string &lname(func->getName());

Expand Down Expand Up @@ -2849,7 +2881,6 @@ void ClassScope::outputCPPMethodInvokeBareObjectSupport(
void ClassScope::outputCPPMethodInvokeTableSupport(CodeGenerator &cg,
AnalysisResultPtr ar, const vector<const char*> &keys,
const StringToFunctionScopePtrMap &funcScopes, bool fewArgs) {
if (isTrait()) return;
string id = getId();
ClassScopePtr self = dynamic_pointer_cast<ClassScope>(shared_from_this());
for (vector<const char*>::const_iterator it = keys.begin();
Expand Down Expand Up @@ -3023,7 +3054,6 @@ void ClassScope::outputCPPMethodInvokeTable(

void ClassScope::outputCPPJumpTableDecl(CodeGenerator &cg,
AnalysisResultPtr ar) {
if (isTrait()) return;
for (FunctionScopePtrVec::const_iterator iter =
m_functionsVec.begin(); iter != m_functionsVec.end(); ++iter) {
FunctionScopePtr func = *iter;
Expand Down
5 changes: 4 additions & 1 deletion src/compiler/analysis/class_scope.h
View file
Expand Up @@ -556,7 +556,8 @@ class ClassScope : public BlockScope, public FunctionContainer,
};

typedef std::list<TraitMethod> TraitMethodList;
std::map<std::string, TraitMethodList> m_importMethToTraitMap;
typedef std::map<std::string, TraitMethodList> MethodToTraitListMap;
MethodToTraitListMap m_importMethToTraitMap;

mutable int m_attribute;
int m_redeclaring; // multiple definition of the same class
Expand Down Expand Up @@ -603,6 +604,8 @@ class ClassScope : public BlockScope, public FunctionContainer,
ClassScopePtr findSingleTraitWithMethod(AnalysisResultPtr ar,
const std::string &methodName) const;

void removeImplTraitAbstractMethods(AnalysisResultPtr ar);

bool usesTrait(const std::string &traitName) const;

bool hasMethod(const std::string &methodName) const;
Expand Down
3 changes: 1 addition & 2 deletions src/compiler/analysis/file_scope.cpp
View file
Expand Up @@ -303,7 +303,7 @@ void FileScope::getScopesSet(BlockScopeRawPtrQueue &v) {
for (ClassScopePtrVec::const_iterator it = iter->second.begin(),
e = iter->second.end(); it != e; ++it) {
ClassScopePtr cls = *it;
if (cls->isUserClass() && !cls->isTrait()) {
if (cls->isUserClass()) {
v.push_back(cls);
getFuncScopesSet(v, cls->getFunctions());
}
Expand Down Expand Up @@ -556,7 +556,6 @@ void FileScope::outputCPPDeclarations(CodeGenerator &cg,
for (StringToClassScopePtrVecMap::iterator it = m_classes.begin();
it != m_classes.end(); ++it) {
BOOST_FOREACH(ClassScopePtr cls, it->second) {
if (cls->isTrait()) continue;
cg_printInclude(cls->getHeaderFilename());
}
}
Expand Down
12 changes: 2 additions & 10 deletions src/compiler/analysis/variable_table.cpp
View file
Expand Up @@ -332,12 +332,6 @@ void VariableTable::addStaticVariable(Symbol *sym,
sym->setStatic();
m_hasStatic = true;

// don't add trait static vars to vectors of static vars, to avoid code gen
ClassScopeRawPtr clsScope = getClassScope();
if (clsScope && clsScope->isTrait()) {
return;
}

FunctionScopeRawPtr funcScope = getFunctionScope();
if (funcScope &&
(funcScope->isClosure() || funcScope->isGeneratorFromClosure())) {
Expand Down Expand Up @@ -410,10 +404,8 @@ TypePtr VariableTable::add(Symbol *sym, TypePtr type,
if (getAttribute(InsideStaticStatement)) {
addStaticVariable(sym, ar);
ClassScopeRawPtr clsScope = getClassScope();
if (!clsScope || !clsScope->isTrait()) {
if (ClassScope::NeedStaticArray(getClassScope(), getFunctionScope())) {
forceVariant(ar, sym->getName(), AnyVars);
}
if (ClassScope::NeedStaticArray(getClassScope(), getFunctionScope())) {
forceVariant(ar, sym->getName(), AnyVars);
}
} else if (getAttribute(InsideGlobalStatement)) {
sym->setGlobal();
Expand Down
2 changes: 1 addition & 1 deletion src/compiler/expression/simple_function_call.cpp
View file
Expand Up @@ -849,7 +849,7 @@ ExpressionPtr SimpleFunctionCall::preOptimize(AnalysisResultConstPtr ar) {
it != classes.end(); ++it) {
ClassScopePtr cls = *it;
if (cls->isUserClass()) cls->setVolatile();
if (!cls->isInterface()) {
if (!cls->isInterface() && !cls->isTrait()) {
classFound = true;
}
}
Expand Down
2 changes: 0 additions & 2 deletions src/compiler/statement/class_statement.cpp
View file
Expand Up @@ -286,7 +286,6 @@ void ClassStatement::outputCPPClassDecl(CodeGenerator &cg,
const char *originalName,
const char *parent) {
ClassScopeRawPtr classScope = getClassScope();
if (classScope->isTrait()) return;
VariableTablePtr variables = classScope->getVariables();
ConstantTablePtr constants = classScope->getConstants();
const char *sweep =
Expand Down Expand Up @@ -361,7 +360,6 @@ void ClassStatement::getCtorAndInitInfo(bool &needsCppCtor, bool &needsInit) {

void ClassStatement::outputCPPImpl(CodeGenerator &cg, AnalysisResultPtr ar) {
ClassScopeRawPtr classScope = getClassScope();
if (classScope->isTrait()) return;
if (cg.getContext() == CodeGenerator::NoContext) {
if (classScope->isVolatile()) {
string name = CodeGenerator::FormatLabel(m_name);
Expand Down

0 comments on commit d867f56

Please sign in to comment.