Move type-related stuff ouside of parsing
3 new passes: resolving inheritance, evaluating constant expressions and
validation.
`resolveInheritance` completes type fields definition which depends on
type base class (so it needs to be looked up before this pass). That
includes interface method serial autofill and enum value autofill.
`evaluate` evaluates constant expressions. This pass depends on the
previous one as enum autofill creates new expressions (+1s).
`validate` proceedes all type-related checks.
`callForReference` is a special way of calling passes for types:
it is used for types that are defined in reference.
Currently that is only array type (size is defined in reference only)
and template type as it could contain an array.
We need such special way to avoid cyclic evaluate call:
struct S { S[42] arr; };
Test: full build, device boot
Test: hidl_test
Test: full build on mac
Test: generated files differ only in constant expression comments
Change-Id: I499e62ae41c52cc86b13d0014eed790454137af6
diff --git a/AST.cpp b/AST.cpp
index b9c3b9c..cf1f760 100644
--- a/AST.cpp
+++ b/AST.cpp
@@ -82,6 +82,18 @@
return mRootScope.containsInterfaces();
}
+status_t AST::resolveInheritance() {
+ return mRootScope.resolveInheritance();
+}
+
+status_t AST::evaluate() {
+ return mRootScope.evaluate();
+}
+
+status_t AST::validate() const {
+ return mRootScope.validate();
+}
+
bool AST::addImport(const char *import) {
FQName fqName(import);
CHECK(fqName.isValid());
diff --git a/AST.h b/AST.h
index d48fba4..b9d8dba 100644
--- a/AST.h
+++ b/AST.h
@@ -67,6 +67,17 @@
void addImportedAST(AST *ast);
+ // Recursive tree pass that completes type declarations
+ // that depend on super types
+ status_t resolveInheritance();
+
+ // Recursive tree pass that evaluates constant expressions
+ status_t evaluate();
+
+ // Recursive tree pass that validates all type-related
+ // syntax restrictions
+ status_t validate() const;
+
status_t generateCpp(const std::string &outputPath) const;
status_t generateCppHeaders(const std::string &outputPath) const;
status_t generateCppSources(const std::string &outputPath) const;
diff --git a/Annotation.cpp b/Annotation.cpp
index 7f3fc88..149182a 100644
--- a/Annotation.cpp
+++ b/Annotation.cpp
@@ -29,6 +29,14 @@
return mName;
}
+status_t AnnotationParam::evaluate() {
+ return OK;
+}
+
+status_t AnnotationParam::validate() const {
+ return OK;
+}
+
std::string AnnotationParam::getSingleString() const {
std::string value = getSingleValue();
@@ -71,12 +79,15 @@
const std::string& name, std::vector<ConstantExpression*>* values)
: AnnotationParam(name), mValues(values) {}
+std::string convertToString(const ConstantExpression* value) {
+ return value->value() + " /* " + value->description() + " */";
+}
+
std::vector<std::string> ConstantExpressionAnnotationParam::getValues() const {
std::vector<std::string> ret;
- for (const auto* ce : *mValues) {
- ret.push_back(convertToString(ce));
+ for (const auto* value : *mValues) {
+ ret.push_back(convertToString(value));
};
-
return ret;
}
@@ -85,8 +96,11 @@
return convertToString(mValues->at(0));
}
-std::string ConstantExpressionAnnotationParam::convertToString(const ConstantExpression* ce) {
- return ce->value() + " /* " + ce->description() + " */";
+status_t ConstantExpressionAnnotationParam::evaluate() {
+ for (auto* value : *mValues) {
+ value->evaluate();
+ }
+ return AnnotationParam::evaluate();
}
Annotation::Annotation(const char* name, AnnotationParamVector* params)
@@ -110,6 +124,22 @@
return nullptr;
}
+status_t Annotation::evaluate() {
+ for (auto* param : *mParams) {
+ status_t err = param->evaluate();
+ if (err != OK) return err;
+ }
+ return OK;
+}
+
+status_t Annotation::validate() const {
+ for (const auto* param : *mParams) {
+ status_t err = param->validate();
+ if (err != OK) return err;
+ }
+ return OK;
+}
+
void Annotation::dump(Formatter &out) const {
out << "@" << mName;
diff --git a/Annotation.h b/Annotation.h
index 776d723..e41d304 100644
--- a/Annotation.h
+++ b/Annotation.h
@@ -42,6 +42,9 @@
/* Returns value interpretted as a boolean */
bool getSingleBool() const;
+ virtual status_t evaluate();
+ virtual status_t validate() const;
+
protected:
const std::string mName;
@@ -65,10 +68,10 @@
std::vector<std::string> getValues() const override;
std::string getSingleValue() const override;
+ status_t evaluate() override;
+
private:
std::vector<ConstantExpression*>* const mValues;
-
- static std::string convertToString(const ConstantExpression* ce);
};
using AnnotationParamVector = std::vector<AnnotationParam*>;
@@ -80,9 +83,12 @@
const AnnotationParamVector ¶ms() const;
const AnnotationParam *getParam(const std::string &name) const;
+ status_t evaluate();
+ status_t validate() const;
+
void dump(Formatter &out) const;
-private:
+ private:
std::string mName;
AnnotationParamVector *mParams;
diff --git a/ArrayType.cpp b/ArrayType.cpp
index ccbccb6..3175c53 100644
--- a/ArrayType.cpp
+++ b/ArrayType.cpp
@@ -67,6 +67,30 @@
return std::to_string(dimension()) + "d array of " + mElementType->typeName();
}
+status_t ArrayType::evaluate() {
+ for (auto* size : mSizes) {
+ size->evaluate();
+ }
+
+ status_t err = mElementType->callForReference(&Type::evaluate);
+ if (err != OK) return err;
+
+ return Type::evaluate();
+}
+
+status_t ArrayType::validate() const {
+ status_t err = mElementType->callForReference(&Type::validate);
+ if (err != OK) return err;
+
+ if (mElementType->isBinder()) {
+ std::cerr << "ERROR: Arrays of interface types are not supported"
+ << " at " << mElementType.location() << "\n";
+
+ return UNKNOWN_ERROR;
+ }
+ return Type::validate();
+}
+
std::string ArrayType::getCppType(StorageMode mode,
bool specifyNamespaces) const {
const std::string base = mElementType->getCppStackType(specifyNamespaces);
diff --git a/ArrayType.h b/ArrayType.h
index ab0590c..3fdb724 100644
--- a/ArrayType.h
+++ b/ArrayType.h
@@ -44,6 +44,9 @@
std::string typeName() const override;
+ status_t evaluate() override;
+ status_t validate() const override;
+
std::string getCppType(StorageMode mode,
bool specifyNamespaces) const override;
diff --git a/CompoundType.cpp b/CompoundType.cpp
index c8d56b0..976a8a4 100644
--- a/CompoundType.cpp
+++ b/CompoundType.cpp
@@ -18,8 +18,10 @@
#include "ArrayType.h"
#include "VectorType.h"
-#include <hidl-util/Formatter.h>
+
#include <android-base/logging.h>
+#include <hidl-util/Formatter.h>
+#include <unordered_set>
namespace android {
@@ -31,35 +33,63 @@
return mStyle;
}
-bool CompoundType::setFields(std::vector<NamedReference<Type>*>* fields, std::string* errorMsg) {
+void CompoundType::setFields(std::vector<NamedReference<Type>*>* fields) {
mFields = fields;
+}
- for (const auto &field : *fields) {
- const Type &type = field->type();
+status_t CompoundType::evaluate() {
+ for (auto* field : *mFields) {
+ status_t err = (*field)->callForReference(&Type::evaluate);
+ if (err != OK) return err;
+ }
+
+ return Scope::evaluate();
+}
+
+status_t CompoundType::validate() const {
+ for (const auto* field : *mFields) {
+ status_t err = (*field)->callForReference(&Type::validate);
+ if (err != OK) return err;
+
+ const Type& type = field->type();
if (type.isBinder()
|| (type.isVector()
&& static_cast<const VectorType *>(
&type)->isVectorOfBinders())) {
- *errorMsg =
- "Structs/Unions must not contain references to interfaces.";
-
- return false;
+ std::cerr << "ERROR: Struct/Union must not contain references to interfaces at "
+ << field->location() << "\n";
+ return UNKNOWN_ERROR;
}
if (mStyle == STYLE_UNION) {
if (type.needsEmbeddedReadWrite()) {
- // Can't have those in a union.
-
- *errorMsg =
- "Unions must not contain any types that need fixup.";
-
- return false;
+ std::cerr << "ERROR: Union must not contain any types that need fixup at "
+ << field->location() << "\n";
+ return UNKNOWN_ERROR;
}
}
}
- return true;
+ status_t err = validateUniqueNames();
+ if (err != OK) return err;
+
+ return Scope::validate();
+}
+
+status_t CompoundType::validateUniqueNames() const {
+ std::unordered_set<std::string> names;
+
+ for (const auto* field : *mFields) {
+ if (names.find(field->name()) != names.end()) {
+ std::cerr << "ERROR: Redefinition of field '" << field->name() << "' at "
+ << field->location() << "\n";
+ return UNKNOWN_ERROR;
+ }
+ names.insert(field->name());
+ }
+
+ return OK;
}
bool CompoundType::isCompoundType() const {
diff --git a/CompoundType.h b/CompoundType.h
index 80c7435..ca16cab 100644
--- a/CompoundType.h
+++ b/CompoundType.h
@@ -35,7 +35,7 @@
Style style() const;
- bool setFields(std::vector<NamedReference<Type>*>* fields, std::string* errorMsg);
+ void setFields(std::vector<NamedReference<Type>*>* fields);
bool isCompoundType() const override;
@@ -43,6 +43,10 @@
std::string typeName() const override;
+ status_t evaluate() override;
+ status_t validate() const override;
+ status_t validateUniqueNames() const;
+
std::string getCppType(StorageMode mode,
bool specifyNamespaces) const override;
diff --git a/Coordinator.cpp b/Coordinator.cpp
index f37c8a9..063d679 100644
--- a/Coordinator.cpp
+++ b/Coordinator.cpp
@@ -105,15 +105,15 @@
ast->addImportedAST(typesAST);
}
- status_t err = parseFile(ast);
-
- if (err != OK) {
+ if (parseFile(ast) != OK || ast->resolveInheritance() != OK || ast->evaluate() != OK ||
+ ast->validate() != OK) {
delete ast;
ast = nullptr;
return nullptr;
}
+ status_t err = OK;
if (ast->package().package() != fqName.package()
|| ast->package().version() != fqName.version()) {
fprintf(stderr,
diff --git a/Coordinator.h b/Coordinator.h
index 2e761df..6f589cf 100644
--- a/Coordinator.h
+++ b/Coordinator.h
@@ -19,7 +19,6 @@
#define COORDINATOR_H_
#include <android-base/macros.h>
-#include <functional>
#include <map>
#include <set>
#include <string>
diff --git a/EnumType.cpp b/EnumType.cpp
index 64c50f9..508328f 100644
--- a/EnumType.cpp
+++ b/EnumType.cpp
@@ -16,14 +16,14 @@
#include "EnumType.h"
+#include <hidl-util/Formatter.h>
+#include <inttypes.h>
+#include <unordered_map>
+
#include "Annotation.h"
#include "Location.h"
#include "ScalarType.h"
-#include <inttypes.h>
-#include <hidl-util/Formatter.h>
-#include <android-base/logging.h>
-
namespace android {
EnumType::EnumType(const char* localName, const Location& location,
@@ -42,21 +42,95 @@
return mValues;
}
-void EnumType::addValue(EnumValue *value) {
+void EnumType::addValue(EnumValue* value) {
CHECK(value != nullptr);
+ mValues.push_back(value);
+}
- EnumValue *prev = nullptr;
- std::vector<const EnumType*> chain = typeChain();
- for (auto it = chain.begin(); it != chain.end(); ++it) {
- const auto &type = *it;
- if(!type->values().empty()) {
- prev = type->values().back();
+status_t EnumType::resolveInheritance() {
+ const EnumType* prevType = nullptr;
+ EnumValue* prevValue = nullptr;
+
+ for (const auto* type : superTypeChain()) {
+ if (!type->values().empty()) {
+ prevType = type;
+ prevValue = type->values().back();
break;
}
}
- value->autofill(prev, resolveToScalarType());
- mValues.push_back(value);
+ for (auto* value : mValues) {
+ value->autofill(prevType, prevValue, mStorageType->resolveToScalarType());
+ prevType = this;
+ prevValue = value;
+ }
+
+ return Scope::resolveInheritance();
+}
+
+status_t EnumType::evaluate() {
+ status_t err = mStorageType->callForReference(&Type::evaluate);
+ if (err != OK) return err;
+
+ for (auto* value : mValues) {
+ err = value->evaluate();
+ if (err != OK) return err;
+ }
+ return Scope::evaluate();
+}
+
+status_t EnumType::validate() const {
+ status_t err = mStorageType->callForReference(&Type::validate);
+ if (err != OK) return err;
+
+ for (auto* value : mValues) {
+ err = value->validate();
+ if (err != OK) return err;
+ }
+
+ if (!isElidableType() || !mStorageType->isValidEnumStorageType()) {
+ std::cerr << "ERROR: Invalid enum storage type (" << (mStorageType)->typeName()
+ << ") specified at " << mStorageType.location() << "\n";
+ return UNKNOWN_ERROR;
+ }
+
+ err = validateUniqueNames();
+ if (err != OK) return err;
+
+ return Scope::validate();
+}
+
+status_t EnumType::validateUniqueNames() const {
+ std::unordered_map<std::string, const EnumType*> registeredValueNames;
+ for (const auto* type : superTypeChain()) {
+ for (const auto* enumValue : type->mValues) {
+ // No need to check super value uniqueness
+ registeredValueNames[enumValue->name()] = type;
+ }
+ }
+
+ for (const auto* value : mValues) {
+ auto registered = registeredValueNames.find(value->name());
+
+ if (registered != registeredValueNames.end()) {
+ const EnumType* definedInType = registered->second;
+
+ if (definedInType == this) {
+ // Defined in this enum
+ std::cerr << "ERROR: Redefinition of value '" << value->name() << "'";
+ } else {
+ // Defined in super enum
+ std::cerr << "ERROR: Redefinition of value '" << value->name() << "define in enum '"
+ << definedInType->fullName() << "'";
+ }
+ std::cerr << " at " << value->location() << "\n";
+ return UNKNOWN_ERROR;
+ }
+
+ registeredValueNames[value->name()] = this;
+ }
+
+ return OK;
}
bool EnumType::isElidableType() const {
@@ -661,11 +735,8 @@
////////////////////////////////////////////////////////////////////////////////
-EnumValue::EnumValue(const char *name, ConstantExpression *value)
- : mName(name),
- mValue(value),
- mIsAutoFill(false) {
-}
+EnumValue::EnumValue(const char* name, ConstantExpression* value, const Location& location)
+ : mName(name), mValue(value), mLocation(location), mIsAutoFill(false) {}
std::string EnumValue::name() const {
return mName;
@@ -695,16 +766,21 @@
return mValue;
}
-void EnumValue::autofill(const EnumValue* prev, const ScalarType* type) {
+void EnumValue::autofill(const EnumType* prevType, EnumValue* prevValue, const ScalarType* type) {
+ // Value is defined explicitly
if (mValue != nullptr) return;
+
+ CHECK((prevType == nullptr) == (prevValue == nullptr));
+
mIsAutoFill = true;
- if (prev == nullptr) {
+ if (prevValue == nullptr) {
mValue = ConstantExpression::Zero(type->getKind()).release();
} else {
- CHECK(prev->mValue != nullptr);
- mValue = prev->mValue->addOne(type->getKind()).release();
+ std::string description = prevType->fullName() + "." + prevValue->name() + " implicitly";
+ auto* prevReference = new ReferenceConstantExpression(
+ Reference<LocalIdentifier>(prevValue, mLocation), description);
+ mValue = prevReference->addOne(type->getKind()).release();
}
- mValue->evaluate();
}
bool EnumValue::isAutoFill() const {
@@ -715,6 +791,20 @@
return true;
}
+status_t EnumValue::evaluate() {
+ mValue->evaluate();
+ return OK;
+}
+
+status_t EnumValue::validate() const {
+ // TODO(b/64358435) move isSupported from ConstantExpression
+ return OK;
+}
+
+const Location& EnumValue::location() const {
+ return mLocation;
+}
+
////////////////////////////////////////////////////////////////////////////////
bool BitFieldType::isBitField() const {
diff --git a/EnumType.h b/EnumType.h
index 518b5ea..ceabb5a 100644
--- a/EnumType.h
+++ b/EnumType.h
@@ -60,6 +60,11 @@
// Return the type that corresponds to bitfield<T>.
BitFieldType *getBitfieldType() const;
+ status_t resolveInheritance() override;
+ status_t evaluate() override;
+ status_t validate() const override;
+ status_t validateUniqueNames() const;
+
void emitReaderWriter(
Formatter &out,
const std::string &name,
@@ -125,22 +130,28 @@
};
struct EnumValue : public LocalIdentifier {
- EnumValue(const char *name, ConstantExpression *value = nullptr);
+ EnumValue(const char* name, ConstantExpression* value, const Location& location);
std::string name() const;
std::string value(ScalarType::Kind castKind) const;
std::string cppValue(ScalarType::Kind castKind) const;
std::string javaValue(ScalarType::Kind castKind) const;
std::string comment() const;
- void autofill(const EnumValue *prev, const ScalarType *type);
+ void autofill(const EnumType* prevType, EnumValue* prevValue, const ScalarType* type);
ConstantExpression* constExpr() const override;
bool isAutoFill() const;
bool isEnumValue() const override;
+ status_t evaluate() override;
+ status_t validate() const override;
+ const Location& location() const;
+
+ private:
std::string mName;
ConstantExpression* mValue;
+ const Location mLocation;
bool mIsAutoFill;
DISALLOW_COPY_AND_ASSIGN(EnumValue);
diff --git a/Interface.cpp b/Interface.cpp
index 208a83d..f55be18 100644
--- a/Interface.cpp
+++ b/Interface.cpp
@@ -30,6 +30,7 @@
#include <iostream>
#include <memory>
#include <sstream>
+#include <unordered_map>
#include <android-base/logging.h>
#include <hidl-hash/Hash.h>
@@ -454,28 +455,89 @@
}
CHECK(!method->isHidlReserved());
- if (lookupMethod(method->name()) != nullptr) {
- LOG(ERROR) << "Redefinition of method " << method->name();
- return false;
- }
- size_t serial = FIRST_CALL_TRANSACTION;
-
- serial += userDefinedMethods().size();
-
- const Interface* ancestor = superType();
- while (ancestor != nullptr) {
- serial += ancestor->userDefinedMethods().size();
- ancestor = ancestor->superType();
- }
-
- CHECK(serial <= LAST_CALL_TRANSACTION) << "More than "
- << LAST_CALL_TRANSACTION << " methods are not allowed.";
- method->setSerialId(serial);
mUserMethods.push_back(method);
return true;
}
+status_t Interface::resolveInheritance() {
+ size_t serial = FIRST_CALL_TRANSACTION;
+ for (const auto* ancestor : superTypeChain()) {
+ serial += ancestor->mUserMethods.size();
+ }
+
+ for (Method* method : mUserMethods) {
+ if (serial > LAST_CALL_TRANSACTION) {
+ std::cerr << "ERROR: More than " << LAST_CALL_TRANSACTION
+ << " methods (including super and reserved) are not allowed at " << location()
+ << "\n";
+ return UNKNOWN_ERROR;
+ }
+
+ method->setSerialId(serial);
+ serial++;
+ }
+
+ return Scope::resolveInheritance();
+}
+
+status_t Interface::evaluate() {
+ for (auto* method : methods()) {
+ status_t err = method->evaluate();
+ if (err != OK) return err;
+ }
+
+ return Scope::evaluate();
+}
+
+status_t Interface::validate() const {
+ CHECK(isIBase() == mSuperType.isEmptyReference());
+
+ for (const auto* method : methods()) {
+ status_t err = method->validate();
+ if (err != OK) return err;
+ }
+
+ status_t err = validateUniqueNames();
+ if (err != OK) return err;
+
+ return Scope::validate();
+}
+
+status_t Interface::validateUniqueNames() const {
+ std::unordered_map<std::string, const Interface*> registeredMethodNames;
+ for (auto const& tuple : allSuperMethodsFromRoot()) {
+ // No need to check super method uniqueness
+ registeredMethodNames[tuple.method()->name()] = tuple.interface();
+ }
+
+ for (const Method* method : mUserMethods) {
+ auto registered = registeredMethodNames.find(method->name());
+
+ if (registered != registeredMethodNames.end()) {
+ const Interface* definedInType = registered->second;
+
+ if (definedInType == this) {
+ // Defined in this interface
+ std::cerr << "ERROR: Redefinition of method '" << method->name() << "'";
+ } else if (definedInType->isIBase()) {
+ // Defined in IBase
+ std::cerr << "ERROR: Redefinition of reserved method '" << method->name() << "'";
+ } else {
+ // Defined in super not IBase
+ std::cerr << "ERROR: Redefinition of method '" << method->name()
+ << "'' defined in interface '" << definedInType->fullName() << "'";
+ }
+ std::cerr << " at " << method->location() << "\n";
+ return UNKNOWN_ERROR;
+ }
+
+ registeredMethodNames[method->name()] = this;
+ }
+
+ return OK;
+}
+
bool Interface::addAllReservedMethods() {
// use a sorted map to insert them in serial ID order.
std::map<int32_t, Method *> reservedMethodsById;
diff --git a/Interface.h b/Interface.h
index a8b562d..1dddfe5 100644
--- a/Interface.h
+++ b/Interface.h
@@ -87,6 +87,11 @@
std::string getJavaType(bool forInitializer) const override;
std::string getVtsType() const override;
+ status_t resolveInheritance() override;
+ status_t evaluate() override;
+ status_t validate() const override;
+ status_t validateUniqueNames() const;
+
void emitReaderWriter(
Formatter &out,
const std::string &name,
diff --git a/Method.cpp b/Method.cpp
index 48eb80e..ad5bca8 100644
--- a/Method.cpp
+++ b/Method.cpp
@@ -28,8 +28,13 @@
Method::Method(const char* name, std::vector<NamedReference<Type>*>* args,
std::vector<NamedReference<Type>*>* results, bool oneway,
- std::vector<Annotation*>* annotations)
- : mName(name), mArgs(args), mResults(results), mOneway(oneway), mAnnotations(annotations) {}
+ std::vector<Annotation*>* annotations, const Location& location)
+ : mName(name),
+ mArgs(args),
+ mResults(results),
+ mOneway(oneway),
+ mAnnotations(annotations),
+ mLocation(location) {}
void Method::fillImplementation(
size_t serial,
@@ -63,6 +68,44 @@
return *mAnnotations;
}
+status_t Method::evaluate() {
+ for (auto* arg : *mArgs) {
+ status_t err = (*arg)->callForReference(&Type::evaluate);
+ if (err != OK) return err;
+ }
+
+ for (auto* result : *mResults) {
+ status_t err = (*result)->callForReference(&Type::evaluate);
+ if (err != OK) return err;
+ }
+
+ for (auto* annotaion : *mAnnotations) {
+ status_t err = annotaion->evaluate();
+ if (err != OK) return err;
+ }
+
+ return OK;
+}
+
+status_t Method::validate() const {
+ for (const auto* arg : *mArgs) {
+ status_t err = (*arg)->callForReference(&Type::validate);
+ if (err != OK) return err;
+ }
+
+ for (const auto* result : *mResults) {
+ status_t err = (*result)->callForReference(&Type::validate);
+ if (err != OK) return err;
+ }
+
+ for (const auto* annotaion : *mAnnotations) {
+ status_t err = annotaion->validate();
+ if (err != OK) return err;
+ }
+
+ return OK;
+}
+
void Method::cppImpl(MethodImplType type, Formatter &out) const {
CHECK(mIsHidlReserved);
auto it = mCppImpl.find(type);
@@ -98,7 +141,7 @@
}
Method *Method::copySignature() const {
- return new Method(mName.c_str(), mArgs, mResults, mOneway, mAnnotations);
+ return new Method(mName.c_str(), mArgs, mResults, mOneway, mAnnotations, Location());
}
void Method::setSerialId(size_t serial) {
@@ -233,6 +276,10 @@
return nullptr;
}
+const Location& Method::location() const {
+ return mLocation;
+}
+
////////////////////////////////////////////////////////////////////////////////
bool TypedVarVector::add(NamedReference<Type>* v) {
diff --git a/Method.h b/Method.h
index bb4e20d..4a78933 100644
--- a/Method.h
+++ b/Method.h
@@ -19,13 +19,15 @@
#define METHOD_H_
#include <android-base/macros.h>
-#include <functional>
#include <hidl-util/Formatter.h>
+#include <utils/Errors.h>
+#include <functional>
#include <map>
#include <set>
#include <string>
#include <vector>
+#include "Location.h"
#include "Reference.h"
namespace android {
@@ -49,7 +51,7 @@
struct Method {
Method(const char* name, std::vector<NamedReference<Type>*>* args,
std::vector<NamedReference<Type>*>* results, bool oneway,
- std::vector<Annotation*>* annotations);
+ std::vector<Annotation*>* annotations, const Location& location);
std::string name() const;
const std::vector<NamedReference<Type>*>& args() const;
@@ -63,6 +65,9 @@
bool isHiddenFromJava() const;
const std::vector<Annotation *> &annotations() const;
+ status_t evaluate();
+ status_t validate() const;
+
// Make a copy with the same name, args, results, oneway, annotations.
// Implementations, serial are not copied.
Method *copySignature() const;
@@ -95,7 +100,9 @@
bool isJavaCompatible() const;
-private:
+ const Location& location() const;
+
+ private:
std::string mName;
size_t mSerial = 0;
std::vector<NamedReference<Type>*>* mArgs;
@@ -109,6 +116,8 @@
MethodImpl mCppImpl;
MethodImpl mJavaImpl;
+ const Location& mLocation;
+
DISALLOW_COPY_AND_ASSIGN(Method);
};
diff --git a/Reference.h b/Reference.h
index 8a73609..e774bd2 100644
--- a/Reference.h
+++ b/Reference.h
@@ -73,7 +73,7 @@
bool hasLocation() const { return mLocation.isValid(); }
- const Location& getLocation() const {
+ const Location& location() const {
CHECK(hasLocation());
return mLocation;
}
diff --git a/Scope.cpp b/Scope.cpp
index f9a844c..acedb28 100644
--- a/Scope.cpp
+++ b/Scope.cpp
@@ -126,6 +126,36 @@
return OK;
}
+status_t Scope::resolveInheritance() {
+ status_t err = forEachType(&Type::resolveInheritance);
+ if (err != OK) return err;
+ return NamedType::resolveInheritance();
+}
+
+status_t Scope::evaluate() {
+ status_t err = forEachType(&Type::evaluate);
+ if (err != OK) return err;
+
+ for (auto* annotation : mAnnotations) {
+ err = annotation->evaluate();
+ if (err != OK) return err;
+ }
+
+ return NamedType::evaluate();
+}
+
+status_t Scope::validate() const {
+ status_t err = forEachType(&Type::validate);
+ if (err != OK) return err;
+
+ for (const auto* annotation : mAnnotations) {
+ err = annotation->validate();
+ if (err != OK) return err;
+ }
+
+ return NamedType::validate();
+}
+
status_t Scope::emitTypeDeclarations(Formatter &out) const {
return forEachType([&](Type *type) {
return type->emitTypeDeclarations(out);
@@ -195,6 +225,8 @@
});
}
+////////////////////////////////////////
+
RootScope::RootScope(const char* localName, const Location& location, Scope* parent)
: Scope(localName, location, parent) {}
RootScope::~RootScope() {}
@@ -203,6 +235,13 @@
return "(root scope)";
}
+status_t RootScope::validate() const {
+ CHECK(annotations().empty());
+ return Scope::validate();
+}
+
+////////////////////////////////////////
+
LocalIdentifier::LocalIdentifier(){}
LocalIdentifier::~LocalIdentifier(){}
@@ -214,5 +253,14 @@
return nullptr;
}
+status_t LocalIdentifier::evaluate() {
+ return OK;
+}
+
+status_t LocalIdentifier::validate() const {
+ CHECK(isEnumValue());
+ return OK;
+}
+
} // namespace android
diff --git a/Scope.h b/Scope.h
index 63bfbfd..f1d5401 100644
--- a/Scope.h
+++ b/Scope.h
@@ -54,6 +54,10 @@
void setAnnotations(std::vector<Annotation*>* annotations);
+ virtual status_t resolveInheritance() override;
+ virtual status_t evaluate() override;
+ virtual status_t validate() const override;
+
status_t emitTypeDeclarations(Formatter &out) const override;
status_t emitGlobalTypeDeclarations(Formatter &out) const override;
status_t emitGlobalHwDeclarations(Formatter &out) const override;
@@ -87,6 +91,8 @@
RootScope(const char* localName, const Location& location, Scope* parent);
virtual ~RootScope();
+ virtual status_t validate() const override;
+
std::string typeName() const override;
};
@@ -95,6 +101,9 @@
virtual ~LocalIdentifier();
virtual bool isEnumValue() const;
+ virtual status_t evaluate();
+ virtual status_t validate() const;
+
virtual ConstantExpression* constExpr() const;
};
diff --git a/Type.cpp b/Type.cpp
index d678cd1..df64ca5 100644
--- a/Type.cpp
+++ b/Type.cpp
@@ -91,6 +91,28 @@
return false;
}
+status_t Type::resolveInheritance() {
+ return OK;
+}
+
+status_t Type::evaluate() {
+ return OK;
+}
+
+status_t Type::validate() const {
+ return OK;
+}
+
+status_t Type::callForReference(std::function<status_t(Type*)> func) {
+ if (isScope()) return OK;
+ return func(this);
+}
+
+status_t Type::callForReference(std::function<status_t(const Type*)> func) const {
+ if (isScope()) return OK;
+ return func(this);
+}
+
const ScalarType *Type::resolveToScalarType() const {
return NULL;
}
@@ -465,7 +487,6 @@
CHECK(mElementType.isEmptyReference());
CHECK(!elementType.isEmptyReference());
- CHECK(isCompatibleElementType(elementType));
mElementType = elementType;
}
@@ -477,6 +498,24 @@
return true;
}
+status_t TemplatedType::evaluate() {
+ status_t err = mElementType->callForReference(&Type::evaluate);
+ if (err != OK) return err;
+ return Type::evaluate();
+}
+
+status_t TemplatedType::validate() const {
+ if (!isCompatibleElementType(mElementType)) {
+ std::cerr << "ERROR: " << typeName() /* contains element type */
+ << " is not supported at " << mElementType.location() << "\n";
+ return UNKNOWN_ERROR;
+ }
+
+ status_t err = mElementType->callForReference(&Type::validate);
+ if (err != OK) return err;
+ return Type::validate();
+}
+
status_t TemplatedType::emitVtsTypeDeclarations(Formatter &out) const {
out << "type: " << getVtsType() << "\n";
out << getVtsValueName() << ": {\n";
diff --git a/Type.h b/Type.h
index bb53573..64f5bde 100644
--- a/Type.h
+++ b/Type.h
@@ -19,10 +19,10 @@
#define TYPE_H_
#include <android-base/macros.h>
-#include <string>
#include <utils/Errors.h>
-#include <vector>
#include <set>
+#include <string>
+#include <vector>
#include "Reference.h"
@@ -53,6 +53,26 @@
virtual bool isTypeDef() const;
virtual bool isVector() const;
+ // Recursive tree pass that completes type declarations
+ // that depend on super types
+ virtual status_t resolveInheritance();
+
+ // Recursive tree pass that evaluates constant expressions
+ virtual status_t evaluate();
+
+ // Recursive tree pass that validates all type-related
+ // syntax restrictions
+ virtual status_t validate() const;
+
+ // Runs recursive tree pass for completing type declaration in references
+ // The only current case when type is declared in reference is
+ // array (array size is reference-related) and templates (as they can
+ // contain array as element type)
+ // A special call is needed to avoid recursive loop. ex:
+ // struct S { S[42] arr; }; -- we need to avoid pass call for S from array
+ status_t callForReference(std::function<status_t(Type*)> func);
+ status_t callForReference(std::function<status_t(const Type*)> func) const;
+
virtual const ScalarType *resolveToScalarType() const;
virtual std::string typeName() const = 0;
@@ -256,7 +276,11 @@
virtual bool isCompatibleElementType(Type* elementType) const = 0;
status_t emitVtsTypeDeclarations(Formatter &out) const override;
status_t emitVtsAttributeType(Formatter &out) const override;
-protected:
+
+ virtual status_t evaluate() override;
+ virtual status_t validate() const override;
+
+ protected:
TemplatedType();
Reference<Type> mElementType;
diff --git a/hidl-gen_y.yy b/hidl-gen_y.yy
index 603cddb..d834eae 100644
--- a/hidl-gen_y.yy
+++ b/hidl-gen_y.yy
@@ -425,7 +425,6 @@
: const_expr
{
$$ = new std::vector<ConstantExpression *>;
- $1->evaluate();
$$->push_back($1);
}
| '{' annotation_const_expr_values '}' { $$ = $2; }
@@ -435,13 +434,11 @@
: const_expr
{
$$ = new std::vector<ConstantExpression *>;
- $1->evaluate();
$$->push_back($1);
}
| annotation_const_expr_values ',' const_expr
{
$$ = $1;
- $3->evaluate();
$$->push_back($3);
}
;
@@ -571,8 +568,10 @@
YYERROR;
}
- $$ = new Reference<Interface>(static_cast<Interface*>($2->get()), $2->getLocation());
+ $$ = new Reference<Interface>(
+ static_cast<Interface*>($2->get()), $2->location());
}
+ ;
interface_declarations
: /* empty */
@@ -754,11 +753,6 @@
<< " could not be found at " << @1 << ".\n";
YYERROR;
}
- if(!iden->isEnumValue()) {
- std::cerr << "ERROR: identifier " << $1->string()
- << " is not an enum value at " << @1 << ".\n";
- YYERROR;
- }
$$ = new ReferenceConstantExpression(
Reference<LocalIdentifier>(iden, convertYYLoc(@1)), $1->string());
} else {
@@ -812,15 +806,30 @@
: error_stmt { $$ = nullptr; }
| opt_annotations valid_identifier '(' typed_vars ')' require_semicolon
{
- $$ = new Method($2, $4, new std::vector<NamedReference<Type>*>, false, $1);
+ $$ = new Method($2 /* name */,
+ $4 /* args */,
+ new std::vector<NamedReference<Type>*> /* results */,
+ false /* oneway */,
+ $1 /* annotations */,
+ convertYYLoc(@$));
}
| opt_annotations ONEWAY valid_identifier '(' typed_vars ')' require_semicolon
{
- $$ = new Method($3, $5, new std::vector<NamedReference<Type>*>, true, $1);
+ $$ = new Method($3 /* name */,
+ $5 /* args */,
+ new std::vector<NamedReference<Type>*> /* results */,
+ true /* oneway */,
+ $1 /* annotations */,
+ convertYYLoc(@$));
}
| opt_annotations valid_identifier '(' typed_vars ')' GENERATES '(' typed_vars ')' require_semicolon
{
- $$ = new Method($2, $4, $8, false, $1);
+ $$ = new Method($2 /* name */,
+ $4 /* args */,
+ $8 /* results */,
+ false /* oneway */,
+ $1 /* annotations */,
+ convertYYLoc(@$));
}
;
@@ -868,15 +877,11 @@
{
CHECK((*scope)->isCompoundType());
CompoundType *container = static_cast<CompoundType *>(*scope);
-
- std::string errorMsg;
- if (!container->setFields($4, &errorMsg)) {
- std::cerr << "ERROR: " << errorMsg << " at " << @4 << "\n";
- YYERROR;
- }
+ container->setFields($4);
leaveScope(ast, scope);
+ std::string errorMsg;
if (!ast->addScopedType(container, &errorMsg, *scope)) {
std::cerr << "ERROR: " << errorMsg << " at " << @2 << "\n";
YYERROR;
@@ -950,19 +955,7 @@
;
enum_storage_type
- : ':' fqtype
- {
- $$ = $2;
-
- if ($$ != NULL && !(*$$)->isValidEnumStorageType()) {
- std::cerr << "ERROR: Invalid enum storage type ("
- << (*$2)->typeName()
- << ") specified. at "
- << @2 << "\n";
-
- YYERROR;
- }
- }
+ : ':' fqtype { $$ = $2; }
;
opt_comma
@@ -997,8 +990,14 @@
;
enum_value
- : valid_identifier { $$ = new EnumValue($1); }
- | valid_identifier '=' const_expr { $3->evaluate(); $$ = new EnumValue($1, $3); }
+ : valid_identifier
+ {
+ $$ = new EnumValue($1 /* name */, nullptr /* value */, convertYYLoc(@$));
+ }
+ | valid_identifier '=' const_expr
+ {
+ $$ = new EnumValue($1 /* name */, $3 /* value */, convertYYLoc(@$));
+ }
;
enum_values
@@ -1034,30 +1033,12 @@
: fqtype { $$ = $1; }
| TEMPLATED '<' type '>'
{
- if (!$1->isCompatibleElementType($3->get())) {
- std::cerr << "ERROR: " << $1->typeName() << " of " << (*$3)->typeName()
- << " is not supported. at " << @3 << "\n";
-
- YYERROR;
- }
$1->setElementType(*$3);
$$ = new Reference<Type>($1, convertYYLoc(@1));
}
| TEMPLATED '<' TEMPLATED '<' type RSHIFT
{
- if (!$3->isCompatibleElementType($5->get())) {
- std::cerr << "ERROR: " << $3->typeName() << " of " << (*$5)->typeName()
- << " is not supported. at " << @5 << "\n";
-
- YYERROR;
- }
$3->setElementType(*$5);
- if (!$1->isCompatibleElementType($3)) {
- std::cerr << "ERROR: " << $1->typeName() << " of " << $3->typeName()
- << " is not supported. at " << @3 << "\n";
-
- YYERROR;
- }
$1->setElementType(Reference<Type>($3, convertYYLoc(@3)));
$$ = new Reference<Type>($1, convertYYLoc(@1));
}
@@ -1068,14 +1049,6 @@
{
Reference<Type> type = *$1;
- if (type->isBinder()) {
- std::cerr << "ERROR: Arrays of interface types are not supported."
- << " at " << @1 << "\n";
-
- YYERROR;
- }
-
- $3->evaluate();
if (type.isResolved() && type->isArray()) {
$$ = new ArrayType(static_cast<ArrayType*>(type.get()), $3);
} else {
@@ -1085,7 +1058,6 @@
| array_type '[' const_expr ']'
{
$$ = $1;
- $3->evaluate();
$$->appendDimension($3);
}
;