Do all validation during validation phase.
When hidl-gen was originally written, validation occured
at two different times: parsing and generation.
Since then, hidl-gen was re-written to be a multi-pass compiler.
The basic phases are now (see main + AST::postParse):
parse, process, validate, generate
However, some validation still exists during the generation phase.
This validation code has been gradually cleaned up, but, here, I am
cleaning up all the rest.
By moving the code to validation, we are fixing several classes
of problems:
1. generate functions almost exclusively (except for the few exceptions
that this CL cleans up) always return status OK. We have a bunch of
data flow logic which exists solely to propagate "return OK". This
is just plain silly.
2. a large number of the returns from emit/generate functions are not
actually checked. We've had a several bugs from this and also many
CLs just to fix this when they've been discovered. This causes problems
where hidl-gen fails but nothing notices.
3. sometimes files are written before things are validated. This is
extremely frustrating and also the cause of other bugs. One common
case of this (while developing) is when updating makefiles, the hidl-gen
compiler complains something is wrong with an interface, but it has
already partially written new but invalid makefiles. This means that
they have to be manually fixed before another build can begin.
4. having status_t returns from generate functions indicates to someone
working on hidl-gen that they can return an error here, but they
should always return an error from the correct validation function.
Removing the ability to return ERROR makes sure that new errors are
caught/validated in the correct place. One instance in this CL of
this happening is that annotations are also checked on isHidlReserved
methods whereas before, these were incorrectly only checked on regular
methods.
(note, breaking text to avoid pinging these)
B ug: 65636464 (forward declarations, made this possible)
B ug: 34807779 (better tools for java compatibility, closer to being solved)
B ug: 32573681 (instance of 2 above).
Test: hidl's run_all_host_tests.sh
Change-Id: I8988e1fdf16e15b925c0a613122c41e6a41bd4dd
diff --git a/Interface.cpp b/Interface.cpp
index 7d2c83c..bf1690f 100644
--- a/Interface.cpp
+++ b/Interface.cpp
@@ -532,7 +532,12 @@
return UNKNOWN_ERROR;
}
- status_t err = validateUniqueNames();
+ status_t err;
+
+ err = validateUniqueNames();
+ if (err != OK) return err;
+
+ err = validateAnnotations();
if (err != OK) return err;
return Scope::validate();
@@ -577,6 +582,24 @@
return OK;
}
+status_t Interface::validateAnnotations() const {
+ for (const Method* method : methods()) {
+ for (const Annotation* annotation : method->annotations()) {
+ const std::string name = annotation->name();
+
+ if (name == "entry" || name == "exit" || name == "callflow") {
+ continue;
+ }
+
+ std::cerr << "ERROR: Unrecognized annotation '" << name
+ << "' for method: " << method->name() << ". An annotation should be one of: "
+ << "entry, exit, callflow." << std::endl;
+ return UNKNOWN_ERROR;
+ }
+ }
+ return OK;
+}
+
bool Interface::addAllReservedMethods() {
// use a sorted map to insert them in serial ID order.
std::map<int32_t, Method *> reservedMethodsById;
@@ -823,11 +846,8 @@
}
}
-status_t Interface::emitPackageTypeDeclarations(Formatter& out) const {
- status_t status = Scope::emitPackageTypeDeclarations(out);
- if (status != OK) {
- return status;
- }
+void Interface::emitPackageTypeDeclarations(Formatter& out) const {
+ Scope::emitPackageTypeDeclarations(out);
// TODO(b/65200821): remove these ifndefs
out << "#ifdef REALLY_IS_HIDL_INTERNAL_LIB" << gCurrentCompileName << "\n";
@@ -845,16 +865,11 @@
<< "return os;\n";
}).endl().endl();
out << "#endif // REALLY_IS_HIDL_INTERNAL_LIB\n";
-
- return OK;
}
-status_t Interface::emitTypeDefinitions(Formatter& out, const std::string& prefix) const {
+void Interface::emitTypeDefinitions(Formatter& out, const std::string& prefix) const {
std::string space = prefix.empty() ? "" : (prefix + "::");
- status_t err = Scope::emitTypeDefinitions(out, space + localName());
- if (err != OK) {
- return err;
- }
+ Scope::emitTypeDefinitions(out, space + localName());
// TODO(b/65200821): remove toString from .cpp once all prebuilts are rebuilt
out << "std::string toString("
@@ -868,8 +883,6 @@
<< "os += o->isRemote() ? \"@remote\" : \"@local\";\n"
<< "return os;\n";
}).endl().endl();
-
- return OK;
}
void Interface::emitJavaReaderWriter(
@@ -892,7 +905,7 @@
}
}
-status_t Interface::emitVtsAttributeDeclaration(Formatter &out) const {
+void Interface::emitVtsAttributeDeclaration(Formatter& out) const {
for (const auto &type : getSubTypes()) {
// Skip for TypeDef as it is just an alias of a defined type.
if (type->isTypeDef()) {
@@ -900,17 +913,13 @@
}
out << "attribute: {\n";
out.indent();
- status_t status = type->emitVtsTypeDeclarations(out);
- if (status != OK) {
- return status;
- }
+ type->emitVtsTypeDeclarations(out);
out.unindent();
out << "}\n\n";
}
- return OK;
}
-status_t Interface::emitVtsMethodDeclaration(Formatter &out) const {
+void Interface::emitVtsMethodDeclaration(Formatter& out) const {
for (const auto &method : methods()) {
if (method->isHidlReserved()) {
continue;
@@ -923,10 +932,7 @@
for (const auto &result : method->results()) {
out << "return_type_hidl: {\n";
out.indent();
- status_t status = result->type().emitVtsAttributeType(out);
- if (status != OK) {
- return status;
- }
+ result->type().emitVtsAttributeType(out);
out.unindent();
out << "}\n";
}
@@ -934,10 +940,7 @@
for (const auto &arg : method->args()) {
out << "arg: {\n";
out.indent();
- status_t status = arg->type().emitVtsAttributeType(out);
- if (status != OK) {
- return status;
- }
+ arg->type().emitVtsAttributeType(out);
out.unindent();
out << "}\n";
}
@@ -945,7 +948,7 @@
for (const auto &annotation : method->annotations()) {
out << "callflow: {\n";
out.indent();
- std::string name = annotation->name();
+ const std::string name = annotation->name();
if (name == "entry") {
out << "entry: true\n";
} else if (name == "exit") {
@@ -959,11 +962,7 @@
}
}
} else {
- std::cerr << "ERROR: Unrecognized annotation '" << name
- << "' for method: " << method->name()
- << ". A VTS annotation should be one of: "
- << "entry, exit, callflow." << std::endl;
- return UNKNOWN_ERROR;
+ CHECK(false);
}
out.unindent();
out << "}\n";
@@ -971,15 +970,13 @@
out.unindent();
out << "}\n\n";
}
- return OK;
}
-status_t Interface::emitVtsAttributeType(Formatter &out) const {
+void Interface::emitVtsAttributeType(Formatter& out) const {
out << "type: " << getVtsType() << "\n"
<< "predefined_type: \""
<< fullName()
<< "\"\n";
- return OK;
}
bool Interface::hasOnewayMethods() const {