better error-handling, importing entire packages. Cache now keyed by FQName
diff --git a/AST.cpp b/AST.cpp
index 3a948c3..798f371 100644
--- a/AST.cpp
+++ b/AST.cpp
@@ -9,6 +9,7 @@
#include <android-base/logging.h>
#include <stdlib.h>
+#include <sys/dir.h>
namespace android {
@@ -20,9 +21,6 @@
}
AST::~AST() {
- CHECK(scope() == mRootScope);
- leaveScope();
-
delete mRootScope;
mRootScope = NULL;
@@ -58,20 +56,66 @@
fqName.applyDefaults(mPackage.package(), mPackage.version());
- LOG(INFO) << "importing " << fqName.debugString();
- const std::string packagePath = Coordinator::GetPackagePath(fqName);
+ // LOG(INFO) << "importing " << fqName.string();
if (fqName.name().empty()) {
- // TODO: Import the whole package.
- CHECK(!"Should not be here");
- return false;
+ const std::string packagePath = Coordinator::GetPackagePath(fqName);
+ DIR *dir = opendir(packagePath.c_str());
+
+ if (dir == NULL) {
+ return false;
+ }
+
+ // Enumerate all regular files ending in ".hal" in the package directory
+ // and attempt to import all of them.
+
+ Vector<std::string> fileNames;
+
+ struct dirent *ent;
+ while ((ent = readdir(dir)) != NULL) {
+ if (ent->d_type != DT_REG) {
+ continue;
+ }
+
+ if (ent->d_namlen < 4
+ || strcmp(ent->d_name + ent->d_namlen - 4, ".hal")) {
+ continue;
+ }
+
+ fileNames.push_back(std::string(ent->d_name, ent->d_namlen - 4));
+ }
+
+ closedir(dir);
+ dir = NULL;
+
+ for (size_t i = 0; i < fileNames.size(); ++i) {
+ FQName subFQName(
+ fqName.package() + fqName.version() + "::" + fileNames[i]);
+
+ if (!subFQName.isValid()) {
+ LOG(WARNING)
+ << "Whole-package import encountered invalid filename '"
+ << fileNames[i]
+ << "' in package "
+ << fqName.package()
+ << fqName.version();
+
+ continue;
+ }
+
+ if (mCoordinator->parse(subFQName) == NULL) {
+ return false;
+ }
+ }
+
+ return true;
}
- std::string path = packagePath;
- path.append(fqName.name());
- path.append(".hal");
+ AST *importAST = mCoordinator->parse(fqName);
- AST *importAST = mCoordinator->parse(path.c_str());
+ if (importAST == NULL) {
+ return false;
+ }
return true;
}
@@ -112,7 +156,7 @@
fqName.applyDefaults(mPackage.package(), mPackage.version());
- // LOG(INFO) << "lookupType now looking for " << fqName.debugString();
+ // LOG(INFO) << "lookupType now looking for " << fqName.string();
return mCoordinator->lookupType(fqName);
}
diff --git a/Coordinator.cpp b/Coordinator.cpp
index e8b4c83..c89c2da 100644
--- a/Coordinator.cpp
+++ b/Coordinator.cpp
@@ -5,27 +5,51 @@
#include <android-base/logging.h>
-extern void parseFile(android::AST *ast, const char *path);
+extern android::status_t parseFile(android::AST *ast, const char *path);
namespace android {
Coordinator::Coordinator() {}
Coordinator::~Coordinator() {}
-AST *Coordinator::parse(const char *path) {
- ssize_t index = mCache.indexOfKey(path);
+AST *Coordinator::parse(const FQName &fqName) {
+ CHECK(fqName.isFullyQualified());
+
+ ssize_t index = mCache.indexOfKey(fqName);
if (index >= 0) {
AST *ast = mCache.valueAt(index);
return ast;
}
- mCache.add(path, NULL);
+ // Add this to the cache immediately, so we can discover circular imports.
+ mCache.add(fqName, NULL);
+
+ const std::string packagePath = GetPackagePath(fqName);
+
+ if (fqName.name() != "types") {
+ // Any interface file implicitly imports its package's types.hal.
+ FQName typesName(fqName.package(), fqName.version(), "types");
+ (void)parse(typesName);
+
+ // fall through.
+ }
+
+ std::string path = packagePath;
+ path.append(fqName.name());
+ path.append(".hal");
AST *ast = new AST(this);
- parseFile(ast, path);
+ status_t err = parseFile(ast, path.c_str());
- mCache.add(path, ast);
+ if (err != OK) {
+ delete ast;
+ ast = NULL;
+
+ return NULL;
+ }
+
+ mCache.add(fqName, ast);
return ast;
}
@@ -63,11 +87,7 @@
Type *Coordinator::lookupType(const FQName &fqName) const {
// Fully qualified.
- CHECK(!fqName.package().empty());
- CHECK(!fqName.version().empty());
- CHECK(!fqName.name().empty());
-
- const std::string packagePath = GetPackagePath(fqName);
+ CHECK(fqName.isFullyQualified());
std::string topType;
size_t dotPos = fqName.name().find('.');
@@ -77,30 +97,27 @@
topType = fqName.name().substr(0, dotPos);
}
- std::string path = packagePath;
- path.append(topType);
- path.append(".hal");
-
- ssize_t index = mCache.indexOfKey(path);
+ // 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);
Type *type = ast->lookupTypeInternal(fqName.name());
if (type != NULL) {
- return new RefType(fqName.debugString().c_str(), type);
+ return new RefType(fqName.string().c_str(), type);
}
}
- path = packagePath;
- path.append("Types.hal");
-
- index = mCache.indexOfKey(path);
+ FQName typesName(fqName.package(), fqName.version(), "types");
+ index = mCache.indexOfKey(typesName);
if (index >= 0) {
AST *ast = mCache.valueAt(index);
Type *type = ast->lookupTypeInternal(fqName.name());
if (type != NULL) {
- return new RefType(fqName.debugString().c_str(), type);
+ return new RefType(fqName.string().c_str(), type);
}
}
diff --git a/Coordinator.h b/Coordinator.h
index 54eaa7e..00e7f0b 100644
--- a/Coordinator.h
+++ b/Coordinator.h
@@ -16,14 +16,14 @@
Coordinator();
~Coordinator();
- AST *parse(const char *path);
+ AST *parse(const FQName &fqName);
Type *lookupType(const FQName &fqName) const;
static std::string GetPackagePath(const FQName &fqName);
private:
- KeyedVector<std::string, AST *> mCache;
+ KeyedVector<FQName, AST *> mCache;
DISALLOW_COPY_AND_ASSIGN(Coordinator);
};
diff --git a/FQName.cpp b/FQName.cpp
index 0b241d6..bb45376 100644
--- a/FQName.cpp
+++ b/FQName.cpp
@@ -23,10 +23,24 @@
setTo(s);
}
+FQName::FQName(
+ const std::string &package,
+ const std::string &version,
+ const std::string &name)
+ : mValid(true),
+ mPackage(package),
+ mVersion(version),
+ mName(name) {
+}
+
bool FQName::isValid() const {
return mValid;
}
+bool FQName::isFullyQualified() const {
+ return !mPackage.empty() && !mVersion.empty() && !mName.empty();
+}
+
bool FQName::setTo(const std::string &s) {
mPackage.clear();
mVersion.clear();
@@ -84,7 +98,7 @@
}
}
-std::string FQName::debugString() const {
+std::string FQName::string() const {
CHECK(mValid);
std::string out;
@@ -106,7 +120,11 @@
return;
}
- LOG(INFO) << debugString();
+ LOG(INFO) << string();
+}
+
+bool FQName::operator<(const FQName &other) const {
+ return string() < other.string();
}
} // namespace android
diff --git a/FQName.h b/FQName.h
index dec47dd..ba777d0 100644
--- a/FQName.h
+++ b/FQName.h
@@ -11,6 +11,10 @@
explicit FQName();
explicit FQName(const std::string &s);
+ FQName(const std::string &package,
+ const std::string &version,
+ const std::string &name);
+
bool isValid() const;
bool setTo(const std::string &s);
@@ -22,16 +26,18 @@
std::string version() const;
std::string name() const;
+ bool isFullyQualified() const;
+
void print() const;
- std::string debugString() const;
+ std::string string() const;
+
+ bool operator<(const FQName &other) const;
private:
bool mValid;
std::string mPackage;
std::string mVersion;
std::string mName;
-
- DISALLOW_COPY_AND_ASSIGN(FQName);
};
} // namespace android
diff --git a/hidl-gen_l.ll b/hidl-gen_l.ll
index 67239a4..fe95c3f 100644
--- a/hidl-gen_l.ll
+++ b/hidl-gen_l.ll
@@ -148,20 +148,29 @@
ECHO;
}
-void parseFile(AST *ast, const char *path) {
+status_t parseFile(AST *ast, const char *path) {
FILE *file = fopen(path, "rb");
+ if (file == NULL) {
+ return -errno;
+ }
+
yyscan_t scanner;
yylex_init_extra(ast, &scanner);
ast->setScanner(scanner);
yyset_in(file, scanner);
int res = yyparse(ast);
- assert(res == 0);
yylex_destroy(scanner);
ast->setScanner(NULL);
fclose(file);
file = NULL;
+
+ if (res != 0) {
+ return UNKNOWN_ERROR;
+ }
+
+ return OK;
}
diff --git a/hidl-gen_y.yy b/hidl-gen_y.yy
index c63448a..93e7b0b 100644
--- a/hidl-gen_y.yy
+++ b/hidl-gen_y.yy
@@ -131,11 +131,23 @@
: /* empty */
| imports IMPORT FQNAME ';'
{
- ast->addImport($3);
+ if (!ast->addImport($3)) {
+ yyerror(ast,
+ android::String8::format(
+ "Unable to import '%s'.", $3));
+
+ YYERROR;
+ }
}
| imports IMPORT IDENTIFIER ';'
{
- ast->addImport($3);
+ if (!ast->addImport($3)) {
+ yyerror(ast,
+ android::String8::format(
+ "Unable to import '%s'.", $3));
+
+ YYERROR;
+ }
}
;
diff --git a/main.cpp b/main.cpp
index 68240ae..7fa34b9 100644
--- a/main.cpp
+++ b/main.cpp
@@ -3,6 +3,7 @@
#include "Formatter.h"
#include "FQName.h"
+#include <android-base/logging.h>
#include <stdio.h>
using namespace android;
@@ -11,7 +12,14 @@
Coordinator coordinator;
for (int i = 1; i < argc; ++i) {
- AST *ast = coordinator.parse(argv[i]);
+ FQName fqName(argv[i]);
+ CHECK(fqName.isValid() && fqName.isFullyQualified());
+
+ AST *ast = coordinator.parse(fqName);
+
+ if (ast == NULL) {
+ continue;
+ }
Formatter out;
@@ -23,14 +31,5 @@
ast = NULL;
}
-#if 0
- FQName("a.b.c.d@2.3::foo").print();
- FQName("a.b.c.d::foo").print();
- FQName("@3.4::foo").print();
- FQName("foo").print();
- FQName("::foo").print();
- FQName("some.package.somewhere@1.2").print();
-#endif
-
return 0;
}