Type pkgs w/ unsupported types in Java now work
Previously, if a package (typically a types-only package) declares types
which are not supported in Java, the type can't be compiled in Java.
However, this unnecessarily restricts some otherwise useful HALs from
being used in Java.
Note that all interfaces in a package must be Java compatible in order
for the package to be compiled in Java (this doesn't support compiling a
subset of the interfaces in Java). This is because in HIDL, the build
system must know exactly which files are outputed, and interfaces always
come with their own files. It would be much more work to handle this
other case, and there isn't a usecase for it now.
Note also that isJavaCompatible interfaces will always compile after
this CL since in order to be compiled in Java, they must only reference
java compatible types. This logic shouldn't rely on assumptions made at
the scope level, so it's safe to remove the scope logic (and if there is
a bug, the worst that will happen is a failed javac compile, which while
potentially confusing, the underlying cause can be fixed in hidl-gen).
Bug: 143566068
Test: cd $ANDROID_BUILD_TOP/hardware/interfaces && update-makefiles.sh
&& mma
Change-Id: I4c81ec269a766be9b1e1ef1fbbb3b1e55bcbce46
diff --git a/AST.cpp b/AST.cpp
index 5453e85..a4f7ef9 100644
--- a/AST.cpp
+++ b/AST.cpp
@@ -510,7 +510,8 @@
mDefinedTypesByFullName[type->fqName()] = type;
}
-LocalIdentifier* AST::lookupLocalIdentifier(const Reference<LocalIdentifier>& ref, Scope* scope) {
+LocalIdentifier* AST::lookupLocalIdentifier(const Reference<LocalIdentifier>& ref,
+ const Scope* scope) {
const FQName& fqName = ref.getLookupFqName();
if (fqName.isIdentifier()) {
@@ -532,7 +533,7 @@
}
}
-EnumValue* AST::lookupEnumValue(const FQName& fqName, std::string* errorMsg, Scope* scope) {
+EnumValue* AST::lookupEnumValue(const FQName& fqName, std::string* errorMsg, const Scope* scope) {
FQName enumTypeName = fqName.typeName();
std::string enumValueName = fqName.valueName();
@@ -561,7 +562,7 @@
return v;
}
-Type* AST::lookupType(const FQName& fqName, Scope* scope) {
+Type* AST::lookupType(const FQName& fqName, const Scope* scope) {
if (fqName.name().empty()) {
// Given a package and version???
return nullptr;
@@ -589,7 +590,7 @@
}
// Rule 0: try resolve locally
-Type* AST::lookupTypeLocally(const FQName& fqName, Scope* scope) {
+Type* AST::lookupTypeLocally(const FQName& fqName, const Scope* scope) {
CHECK(fqName.package().empty() && fqName.version().empty()
&& !fqName.name().empty() && fqName.valueName().empty());
diff --git a/AST.h b/AST.h
index e9c1367..9026ebb 100644
--- a/AST.h
+++ b/AST.h
@@ -74,15 +74,16 @@
// Look up local identifier.
// It could be plain identifier or enum value as described by lookupEnumValue.
- LocalIdentifier* lookupLocalIdentifier(const Reference<LocalIdentifier>& ref, Scope* scope);
+ LocalIdentifier* lookupLocalIdentifier(const Reference<LocalIdentifier>& ref,
+ const Scope* scope);
// Look up an enum value by "FQName:valueName".
- EnumValue* lookupEnumValue(const FQName& fqName, std::string* errorMsg, Scope* scope);
+ EnumValue* lookupEnumValue(const FQName& fqName, std::string* errorMsg, const Scope* scope);
// Look up a type by FQName, "pure" names, i.e. those without package
// or version are first looked up in the current scope chain.
// After that lookup proceeds to imports.
- Type* lookupType(const FQName& fqName, Scope* scope);
+ Type* lookupType(const FQName& fqName, const Scope* scope);
void addImportedAST(AST *ast);
@@ -282,7 +283,7 @@
bool importFQName(const FQName& fqName);
// Helper functions for lookupType.
- Type* lookupTypeLocally(const FQName& fqName, Scope* scope);
+ Type* lookupTypeLocally(const FQName& fqName, const Scope* scope);
status_t lookupAutofilledType(const FQName &fqName, Type **returnedType);
Type *lookupTypeFromImports(const FQName &fqName);
diff --git a/CompoundType.cpp b/CompoundType.cpp
index 82ea179..04bfa77 100644
--- a/CompoundType.cpp
+++ b/CompoundType.cpp
@@ -1378,7 +1378,7 @@
<< discriminatorStorageType
<< " getDiscriminator() { return hidl_d; }\n\n";
- } else {
+ } else if (mStyle == STYLE_STRUCT) {
for (const auto& field : mFields) {
field->emitDocComment(out);
@@ -1387,6 +1387,8 @@
}
out << "\n";
+ } else {
+ LOG(FATAL) << "Java output doesn't support " << mStyle;
}
////////////////////////////////////////////////////////////////////////////
diff --git a/Scope.cpp b/Scope.cpp
index adc8f30..588ba95 100644
--- a/Scope.cpp
+++ b/Scope.cpp
@@ -227,10 +227,14 @@
bool Scope::deepIsJavaCompatible(std::unordered_set<const Type*>* visited) const {
for (const Type* type : mTypes) {
- if (!type->isJavaCompatible(visited)) {
+ // Java compatibility focuses on types that are actually used by interfaces.
+ // Declarations of java-incompatible types are simply omitted from
+ // corresponding Java libraries.
+ if (type->isInterface() && !type->isJavaCompatible(visited)) {
return false;
}
}
+
return Type::deepIsJavaCompatible(visited);
}
diff --git a/main.cpp b/main.cpp
index 038a2a9..3245758 100644
--- a/main.cpp
+++ b/main.cpp
@@ -293,21 +293,37 @@
const FileGenerator::GetFormatter& getFormatter) {
AST* ast;
std::string limitToType;
+ FQName typeName;
- // Required for legacy -Lmakefile files
+ // See appendPerTypeTargets.
+ // 'a.b.c@1.0::types.Foo' is used to compile 'Foo' for Java even though in
+ // the rest of the compiler, this type is simply called 'a.b.c@1.0::Foo'.
+ // However, here, we need to disambiguate an interface name and a type in
+ // types.hal in order to figure out what to parse, so this legacy behavior
+ // is kept.
if (fqName.name().find("types.") == 0) {
limitToType = fqName.name().substr(strlen("types."));
- FQName typesName = fqName.getTypesForPackage();
- ast = coordinator->parse(typesName);
+ ast = coordinator->parse(fqName.getTypesForPackage());
+
+ const auto& names = fqName.names();
+ CHECK(names.size() == 2 && names[0] == "types") << fqName.string();
+ typeName = FQName(fqName.package(), fqName.version(), names[1]);
} else {
ast = coordinator->parse(fqName);
+ typeName = fqName;
}
if (ast == nullptr) {
fprintf(stderr, "ERROR: Could not parse %s. Aborting.\n", fqName.string().c_str());
return UNKNOWN_ERROR;
}
+ Type* type = ast->lookupType(typeName, &ast->getRootScope());
+ CHECK(type != nullptr) << typeName.string();
+ if (!type->isJavaCompatible()) {
+ return OK;
+ }
+
Formatter out = getFormatter();
if (!out.isValid()) {
return UNKNOWN_ERROR;
diff --git a/test/java_partial_test/1.0/types.hal b/test/java_partial_test/1.0/types.hal
new file mode 100644
index 0000000..91a9018
--- /dev/null
+++ b/test/java_partial_test/1.0/types.hal
@@ -0,0 +1,39 @@
+/*
+ * Copyright (C) 2019 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package hidl_java_partial_test@1.0;
+
+struct ExistsInJava {
+ uint32_t happyHippo;
+ uint32_t hippoHappy;
+};
+
+struct DoesNotExistInJava {
+ memory shmemory;
+};
+
+struct DoesNotExistInJavaTransitive {
+ DoesNotExistInJava transative;
+};
+
+struct DoesNotExistInJavaVec {
+ vec<DoesNotExistInJava> avec;
+};
+
+union UnionDoesNotExistInJava {
+ uint32_t a;
+ uint64_t b;
+};
diff --git a/test/java_partial_test/Android.bp b/test/java_partial_test/Android.bp
new file mode 100644
index 0000000..3f856af
--- /dev/null
+++ b/test/java_partial_test/Android.bp
@@ -0,0 +1,24 @@
+java_genrule {
+ name: "hidl_partial_java_test_gen",
+ tools: [
+ "hidl-gen",
+ ],
+ srcs: [
+ "1.0/types.hal",
+ ],
+ cmd: "$(location hidl-gen) -o $(genDir) -Ljava " +
+ "-r hidl_java_partial_test:system/tools/hidl/test/java_partial_test " +
+ "hidl_java_partial_test@1.0 && " +
+ "[ \"$(genDir)/hidl_java_partial_test/V1_0/ExistsInJava.java\" == " +
+ "\"$$(find $(genDir) -type f)\" ]",
+ out: [
+ "hidl_java_partial_test/V1_0/ExistsInJava.java",
+ ],
+}
+
+java_library {
+ name: "hidl_partial_java_test",
+ srcs: [
+ ":hidl_partial_java_test_gen",
+ ],
+}