Improved name lookup in order to allow partial names to refer to external
(imported) packages.
Bug: 30977424
Change-Id: I0a3757d6c3b10229627e2c1d6bc6176f88e13151
diff --git a/AST.cpp b/AST.cpp
index 7e90ed0..5939e85 100644
--- a/AST.cpp
+++ b/AST.cpp
@@ -9,6 +9,7 @@
#include "TypeDef.h"
#include <android-base/logging.h>
+#include <iostream>
#include <stdlib.h>
namespace android {
@@ -83,7 +84,8 @@
}
for (const auto &subFQName : packageInterfaces) {
- if (mCoordinator->parse(subFQName) == NULL) {
+ AST *ast = mCoordinator->parse(subFQName, &mImportedASTs);
+ if (ast == NULL) {
return false;
}
}
@@ -91,7 +93,7 @@
return true;
}
- AST *importAST = mCoordinator->parse(fqName);
+ AST *importAST = mCoordinator->parse(fqName, &mImportedASTs);
if (importAST == NULL) {
return false;
@@ -100,6 +102,10 @@
return true;
}
+void AST::addImportedAST(AST *ast) {
+ mImportedASTs.insert(ast);
+}
+
void AST::enterScope(Scope *container) {
mScopePath.push_back(container);
}
@@ -113,8 +119,33 @@
return mScopePath.top();
}
+bool AST::addTypeDef(
+ const char *localName, Type *type, std::string *errorMsg) {
+ // The reason we wrap the given type in a TypeDef is simply to suppress
+ // emitting any type definitions later on, since this is just an alias
+ // to a type defined elsewhere.
+ return addScopedTypeInternal(
+ localName, new TypeDef(type), errorMsg, true /* isTypeDef */);
+}
+
bool AST::addScopedType(
const char *localName, NamedType *type, std::string *errorMsg) {
+ return addScopedTypeInternal(
+ localName, type, errorMsg, false /* isTypeDef */);
+}
+
+bool AST::addScopedTypeInternal(
+ const char *localName,
+ Type *type,
+ std::string *errorMsg,
+ bool isTypeDef) {
+ if (!isTypeDef) {
+ // Resolve typeDefs to the target type.
+ while (type->isTypeDef()) {
+ type = static_cast<TypeDef *>(type)->referencedType();
+ }
+ }
+
std::string anonName;
if (localName == nullptr) {
@@ -137,10 +168,17 @@
}
path.append(localName);
- type->setLocalName(localName);
-
FQName fqName(mPackage.package(), mPackage.version(), path);
- type->setFullName(fqName);
+
+ if (!isTypeDef) {
+ CHECK(type->isNamedType());
+
+ NamedType *namedType = static_cast<NamedType *>(type);
+ namedType->setLocalName(localName);
+ namedType->setFullName(fqName);
+ }
+
+ mDefinedTypesByFullName.add(fqName, type);
return true;
}
@@ -171,16 +209,52 @@
}
}
- fqName.applyDefaults(mPackage.package(), mPackage.version());
+ Type *resolvedType = nullptr;
+ FQName resolvedName;
- // LOG(INFO) << "lookupType now looking for " << fqName.string();
+ for (const auto &importedAST : mImportedASTs) {
+ FQName matchingName;
+ Type *match = importedAST->findDefinedType(fqName, &matchingName);
- Type *resultType = mCoordinator->lookupType(fqName);
+ if (match != nullptr) {
+ if (resolvedType != nullptr) {
+ std::cerr << "ERROR: Unable to resolve type name '"
+ << fqName.string()
+ << "', multiple matches found:\n";
- if (resultType) {
- if (!resultType->isInterface()) {
+ std::cerr << " " << resolvedName.string() << "\n";
+ std::cerr << " " << matchingName.string() << "\n";
+
+ return NULL;
+ }
+
+ resolvedType = match;
+ resolvedName = matchingName;
+
+ // Keep going even after finding a match.
+ }
+ }
+
+ if (resolvedType) {
+#if 0
+ LOG(INFO) << "found '"
+ << resolvedName.string()
+ << "' after looking for '"
+ << fqName.string()
+ << "'.";
+#endif
+
+ // Resolve typeDefs to the target type.
+ while (resolvedType->isTypeDef()) {
+ resolvedType =
+ static_cast<TypeDef *>(resolvedType)->referencedType();
+ }
+
+ if (!resolvedType->isInterface()) {
// Non-interface types are declared in the associated types header.
- FQName typesName(fqName.package(), fqName.version(), "types");
+ FQName typesName(
+ resolvedName.package(), resolvedName.version(), "types");
+
mImportedNames.insert(typesName);
} else {
// Do _not_ use fqName, i.e. the name we used to look up the type,
@@ -190,49 +264,24 @@
// name of the typedef instead of the proper name of the interface.
mImportedNames.insert(
- static_cast<Interface *>(resultType)->fqName());
+ static_cast<Interface *>(resolvedType)->fqName());
}
}
- return resultType;
+ return resolvedType->ref();
}
-Type *AST::lookupTypeInternal(const std::string &namePath) const {
- Scope *scope = mRootScope;
+Type *AST::findDefinedType(const FQName &fqName, FQName *matchingName) const {
+ for (size_t i = 0; i < mDefinedTypesByFullName.size(); ++i) {
+ const FQName &key = mDefinedTypesByFullName.keyAt(i);
- size_t startPos = 0;
- for (;;) {
- size_t dotPos = namePath.find('.', startPos);
-
- std::string component;
- if (dotPos == std::string::npos) {
- component = namePath.substr(startPos);
- } else {
- component = namePath.substr(startPos, dotPos - startPos);
+ if (key.endsWith(fqName)) {
+ *matchingName = key;
+ return mDefinedTypesByFullName.valueAt(i);
}
-
- Type *type = scope->lookupType(component.c_str());
-
- if (type == NULL) {
- return NULL;
- }
-
- if (dotPos == std::string::npos) {
- // Resolve typeDefs to the target type.
- while (type->isTypeDef()) {
- type = static_cast<TypeDef *>(type)->referencedType();
- }
-
- return type;
- }
-
- if (!type->isScope()) {
- return NULL;
- }
-
- scope = static_cast<Scope *>(type);
- startPos = dotPos + 1;
}
+
+ return nullptr;
}
void AST::getImportedPackages(std::set<FQName> *importSet) const {
diff --git a/AST.h b/AST.h
index 2d674ba..e2b68cf 100644
--- a/AST.h
+++ b/AST.h
@@ -38,10 +38,11 @@
Scope *scope();
// Returns true iff successful.
+ bool addTypeDef(const char *localName, Type *type, std::string *errorMsg);
+
+ // Returns true iff successful.
bool addScopedType(
- const char *localName,
- NamedType *type,
- std::string *errorMsg);
+ const char *localName, NamedType *type, std::string *errorMsg);
void *scanner();
void setScanner(void *scanner);
@@ -53,10 +54,7 @@
// After that lookup proceeds to imports.
Type *lookupType(const char *name);
- // Takes dot-separated path components to a type possibly inside this AST.
- // Name resolution goes from root scope downwards, i.e. the path must be
- // absolute.
- Type *lookupTypeInternal(const std::string &namePath) const;
+ void addImportedAST(AST *ast);
status_t generateCpp(const std::string &outputPath) const;
status_t generateJava(const std::string &outputPath) const;
@@ -75,8 +73,27 @@
FQName mPackage;
+ // A set of all external interfaces/types that are _actually_ referenced
+ // in this AST, this is a subset of those specified in import statements.
std::set<FQName> mImportedNames;
+ // A set of all ASTs we explicitly or implicitly (types.hal) import.
+ std::set<AST *> mImportedASTs;
+
+ // Types keyed by full names defined in this AST.
+ KeyedVector<FQName, Type *> mDefinedTypesByFullName;
+
+ bool addScopedTypeInternal(
+ const char *localName,
+ Type *type,
+ std::string *errorMsg,
+ bool isTypeDef);
+
+ // Find a type matching fqName (which may be partial) and if found
+ // return the associated type and fill in the full "matchingName".
+ // Only types defined in this very AST are considered.
+ Type *findDefinedType(const FQName &fqName, FQName *matchingName) const;
+
void getPackageComponents(std::vector<std::string> *components) const;
void getPackageAndVersionComponents(
diff --git a/Coordinator.cpp b/Coordinator.cpp
index fb6b9e4..29276ee 100644
--- a/Coordinator.cpp
+++ b/Coordinator.cpp
@@ -23,7 +23,7 @@
// empty
}
-AST *Coordinator::parse(const FQName &fqName) {
+AST *Coordinator::parse(const FQName &fqName, std::set<AST *> *parsedASTs) {
CHECK(fqName.isFullyQualified());
// LOG(INFO) << "parsing " << fqName.string();
@@ -32,16 +32,22 @@
if (index >= 0) {
AST *ast = mCache.valueAt(index);
+ if (ast != nullptr && parsedASTs != nullptr) {
+ parsedASTs->insert(ast);
+ }
+
return ast;
}
// Add this to the cache immediately, so we can discover circular imports.
- mCache.add(fqName, NULL);
+ mCache.add(fqName, nullptr);
+
+ AST *typesAST = nullptr;
if (fqName.name() != "types") {
// Any interface file implicitly imports its package's types.hal.
FQName typesName(fqName.package(), fqName.version(), "types");
- (void)parse(typesName);
+ typesAST = parse(typesName, parsedASTs);
// fall through.
}
@@ -52,15 +58,22 @@
path.append(".hal");
AST *ast = new AST(this, path);
+
+ if (typesAST != NULL) {
+ // If types.hal for this AST's package existed, make it's defined
+ // types available to the (about to be parsed) AST right away.
+ ast->addImportedAST(typesAST);
+ }
+
status_t err = parseFile(ast);
if (err != OK) {
// LOG(ERROR) << "parsing '" << path << "' FAILED.";
delete ast;
- ast = NULL;
+ ast = nullptr;
- return NULL;
+ return nullptr;
}
if (ast->package().package() != fqName.package()
@@ -104,11 +117,13 @@
if (err != OK) {
delete ast;
- ast = NULL;
+ ast = nullptr;
- return NULL;
+ return nullptr;
}
+ if (parsedASTs != nullptr) { parsedASTs->insert(ast); }
+
mCache.add(fqName, ast);
return ast;
@@ -195,50 +210,6 @@
return packagePath;
}
-Type *Coordinator::lookupType(const FQName &fqName) const {
- // Fully qualified.
- CHECK(fqName.isFullyQualified());
-
- std::string topType;
- size_t dotPos = fqName.name().find('.');
- if (dotPos == std::string::npos) {
- topType = fqName.name();
- } else {
- topType = fqName.name().substr(0, dotPos);
- }
-
- // Assuming {topType} is the name of an interface type, let's see if the
- // associated {topType}.hal file was imported.
- FQName ifaceName(fqName.package(), fqName.version(), topType);
- ssize_t index = mCache.indexOfKey(ifaceName);
- if (index >= 0) {
- AST *ast = mCache.valueAt(index);
- CHECK(ast != NULL);
-
- Type *type = ast->lookupTypeInternal(fqName.name());
-
- if (type != NULL) {
- return type->ref();
- }
- }
-
- FQName typesName(fqName.package(), fqName.version(), "types");
- index = mCache.indexOfKey(typesName);
- if (index >= 0) {
- AST *ast = mCache.valueAt(index);
- if (ast != NULL) {
- // ast could be NULL if types.hal didn't exist, which is valid.
- Type *type = ast->lookupTypeInternal(fqName.name());
-
- if (type != NULL) {
- return type->ref();
- }
- }
- }
-
- return NULL;
-}
-
status_t Coordinator::getPackageInterfaceFiles(
const FQName &package,
std::vector<std::string> *fileNames) const {
diff --git a/Coordinator.h b/Coordinator.h
index d6b9852..5f76fc1 100644
--- a/Coordinator.h
+++ b/Coordinator.h
@@ -4,6 +4,7 @@
#include <android-base/macros.h>
#include <functional>
+#include <set>
#include <string>
#include <utils/KeyedVector.h>
#include <vector>
@@ -21,9 +22,12 @@
~Coordinator();
- AST *parse(const FQName &fqName);
-
- Type *lookupType(const FQName &fqName) const;
+ // Attempts to parse the interface/types referred to by fqName.
+ // Parsing an interface also parses the associated package's types.hal
+ // file if it exists.
+ // If "parsedASTs" is non-NULL, successfully parsed ASTs are inserted
+ // into the set.
+ AST *parse(const FQName &fqName, std::set<AST *> *parsedASTs = nullptr);
// Given package-root paths of ["hardware/interfaces",
// "vendor/<something>/interfaces"], package roots of
diff --git a/FQName.cpp b/FQName.cpp
index dc35f70..413a20a 100644
--- a/FQName.cpp
+++ b/FQName.cpp
@@ -225,5 +225,29 @@
components->push_back(versionString);
}
+bool FQName::endsWith(const FQName &other) const {
+ std::string s1 = string();
+ std::string s2 = other.string();
+
+ size_t pos = s1.rfind(s2);
+ if (pos == std::string::npos || pos + s2.size() != s1.size()) {
+ return false;
+ }
+
+ if (pos > 0) {
+ // A match is only a match if it is preceded by a "boundary", i.e.
+ // we perform a component-wise match from the end.
+ // "az" is not a match for "android.hardware.foo@1.0::IFoo.bar.baz",
+ // "baz", "bar.baz", "IFoo.bar.baz" are.
+
+ char separator = s1[pos - 1];
+ if (separator != '.' && separator != ':') {
+ return false;
+ }
+ }
+
+ return true;
+}
+
} // namespace android
diff --git a/FQName.h b/FQName.h
index ae7fe80..9570386 100644
--- a/FQName.h
+++ b/FQName.h
@@ -50,6 +50,8 @@
// i.e. "android.hardware.Foo.V1_0.IBar"
std::string javaName() const;
+ bool endsWith(const FQName &other) const;
+
void getPackageComponents(std::vector<std::string> *components) const;
void getPackageAndVersionComponents(
diff --git a/NamedType.cpp b/NamedType.cpp
index ed1ffe8..ddefff7 100644
--- a/NamedType.cpp
+++ b/NamedType.cpp
@@ -4,6 +4,10 @@
NamedType::NamedType() {}
+bool NamedType::isNamedType() const {
+ return true;
+}
+
void NamedType::setLocalName(const std::string &localName) {
mLocalName = localName;
}
diff --git a/NamedType.h b/NamedType.h
index d0d24ea..52a7434 100644
--- a/NamedType.h
+++ b/NamedType.h
@@ -13,6 +13,8 @@
struct NamedType : public Type {
NamedType();
+ bool isNamedType() const override;
+
void setLocalName(const std::string &localName);
void setFullName(const FQName &fullName);
diff --git a/Scope.cpp b/Scope.cpp
index 8b6385a..28570e0 100644
--- a/Scope.cpp
+++ b/Scope.cpp
@@ -9,8 +9,7 @@
Scope::Scope() {}
-bool Scope::addType(
- const char *localName, NamedType *type, std::string *errorMsg) {
+bool Scope::addType(const char *localName, Type *type, std::string *errorMsg) {
if (mTypeIndexByName.indexOfKey(localName) >= 0) {
*errorMsg = "A type named '";
(*errorMsg) += localName;
diff --git a/Scope.h b/Scope.h
index 7883ad0..4a77a7c 100644
--- a/Scope.h
+++ b/Scope.h
@@ -15,7 +15,7 @@
struct Scope : public NamedType {
Scope();
- bool addType(const char *localName, NamedType *type, std::string *errorMsg);
+ bool addType(const char *localName, Type *type, std::string *errorMsg);
Type *lookupType(const char *name) const;
diff --git a/Type.cpp b/Type.cpp
index 912bc25..da45677 100644
--- a/Type.cpp
+++ b/Type.cpp
@@ -30,6 +30,10 @@
return false;
}
+bool Type::isNamedType() const {
+ return false;
+}
+
const ScalarType *Type::resolveToScalarType() const {
return NULL;
}
diff --git a/Type.h b/Type.h
index aef307d..dbb190b 100644
--- a/Type.h
+++ b/Type.h
@@ -22,6 +22,7 @@
virtual bool isEnum() const;
virtual bool isTypeDef() const;
virtual bool isBinder() const;
+ virtual bool isNamedType() const;
virtual const ScalarType *resolveToScalarType() const;
diff --git a/TypeDef.cpp b/TypeDef.cpp
index ba84700..67bf0f7 100644
--- a/TypeDef.cpp
+++ b/TypeDef.cpp
@@ -7,7 +7,7 @@
namespace android {
TypeDef::TypeDef(Type *type)
- : NamedType(),
+ : Type(),
mReferencedType(type) {
}
diff --git a/TypeDef.h b/TypeDef.h
index 23d939d..9760208 100644
--- a/TypeDef.h
+++ b/TypeDef.h
@@ -2,11 +2,11 @@
#define TYPE_DEF_H_
-#include "NamedType.h"
+#include "Type.h"
namespace android {
-struct TypeDef : public NamedType {
+struct TypeDef : public Type {
TypeDef(Type *type);
const ScalarType *resolveToScalarType() const override;
diff --git a/hidl-gen_l.ll b/hidl-gen_l.ll
index 76f6cdd..68112da 100644
--- a/hidl-gen_l.ll
+++ b/hidl-gen_l.ll
@@ -145,18 +145,14 @@
char c, c1;
loop:
- while ((c = yyinput(yyscanner)) != '*' && c != 0)
- putchar(c);
+ while ((c = yyinput(yyscanner)) != '*' && c != 0) {
+ }
if ((c1 = yyinput(yyscanner)) != '/' && c != 0)
{
unput(c1);
goto loop;
}
-
- if (c != 0) {
- putchar(c1);
- }
}
status_t parseFile(AST *ast) {
diff --git a/hidl-gen_y.yy b/hidl-gen_y.yy
index 55fee10..5867f4a 100644
--- a/hidl-gen_y.yy
+++ b/hidl-gen_y.yy
@@ -9,7 +9,6 @@
#include "GenericBinder.h"
#include "Interface.h"
#include "Method.h"
-#include "TypeDef.h"
#include "VectorType.h"
#include "hidl-gen_y.h"
@@ -328,10 +327,8 @@
typedef_declaration
: TYPEDEF type IDENTIFIER
{
- TypeDef *def = new TypeDef($2);
-
std::string errorMsg;
- if (!ast->addScopedType($3, def, &errorMsg)) {
+ if (!ast->addTypeDef($3, $2, &errorMsg)) {
std::cerr << "ERROR: " << errorMsg << " at " << @3 << "\n";
YYERROR;
}