Allow duplicate types from preprocessed
When compiling for SDK stub sources, we need to load documents which are
also a part of preprocessed framework.aidl, which results in inevitable
type duplication.
We allow duplicate types only if one of them is from preprocessed
document.
Bug: 192211616
Test: aidl_unittests
Change-Id: I21fc66d7305df824d2cc84308b84b9c9e9213d13
diff --git a/aidl_language.cpp b/aidl_language.cpp
index 83dfe74..7ea1bb8 100644
--- a/aidl_language.cpp
+++ b/aidl_language.cpp
@@ -906,7 +906,7 @@
AidlDefinedType::AidlDefinedType(const AidlLocation& location, const std::string& name,
const Comments& comments, const std::string& package,
std::vector<std::unique_ptr<AidlMember>>* members)
- : AidlAnnotatable(location, comments), name_(name), package_(package) {
+ : AidlAnnotatable(location, comments), AidlScope(this), name_(name), package_(package) {
// adjust name/package when name is fully qualified (for preprocessed files)
if (package_.empty() && name_.find('.') != std::string::npos) {
// Note that this logic is absolutely wrong. Given a parcelable
@@ -1030,6 +1030,25 @@
return GetEnclosingScope()->ResolveName(name);
}
+template <typename T>
+const T* AidlCast(const AidlNode& node) {
+ struct CastVisitor : AidlVisitor {
+ const T* cast = nullptr;
+ void Visit(const T& t) override { cast = &t; }
+ } visitor;
+ node.DispatchVisit(visitor);
+ return visitor.cast;
+}
+
+const AidlDocument& AidlDefinedType::GetDocument() const {
+ // TODO(b/182508839): resolve with nested types when we support nested types
+ auto scope = GetEnclosingScope();
+ AIDL_FATAL_IF(!scope, this) << "no scope defined.";
+ auto doc = AidlCast<AidlDocument>(scope->GetNode());
+ AIDL_FATAL_IF(!doc, this) << "scope is not a document.";
+ return *doc;
+}
+
AidlParcelable::AidlParcelable(const AidlLocation& location, const std::string& name,
const std::string& package, const Comments& comments,
const std::string& cpp_header, std::vector<std::string>* type_params,
@@ -1524,10 +1543,13 @@
AidlDocument::AidlDocument(const AidlLocation& location, const Comments& comments,
std::vector<std::unique_ptr<AidlImport>> imports,
- std::vector<std::unique_ptr<AidlDefinedType>> defined_types)
+ std::vector<std::unique_ptr<AidlDefinedType>> defined_types,
+ bool is_preprocessed)
: AidlCommentable(location, comments),
+ AidlScope(this),
imports_(std::move(imports)),
- defined_types_(std::move(defined_types)) {
+ defined_types_(std::move(defined_types)),
+ is_preprocessed_(is_preprocessed) {
for (const auto& t : defined_types_) {
t->SetEnclosingScope(this);
}
diff --git a/aidl_language.h b/aidl_language.h
index 4f19261..2dc1d91 100644
--- a/aidl_language.h
+++ b/aidl_language.h
@@ -123,6 +123,7 @@
class AidlScope {
public:
+ AidlScope(const AidlNode* self) : self_(self) {}
virtual ~AidlScope() = default;
virtual std::string ResolveName(const std::string& name) const = 0;
void SetEnclosingScope(const AidlScope* enclosing) {
@@ -130,8 +131,10 @@
enclosing_ = enclosing;
}
const AidlScope* GetEnclosingScope() const { return enclosing_; }
+ const AidlNode& GetNode() const { return *self_; }
private:
+ const AidlNode* self_;
const AidlScope* enclosing_ = nullptr;
};
@@ -928,6 +931,7 @@
if (package_.empty()) return std::vector<std::string>();
return android::base::Split(package_, ".");
}
+ const AidlDocument& GetDocument() const;
virtual std::string GetPreprocessDeclarationName() const = 0;
@@ -1222,7 +1226,7 @@
public:
AidlDocument(const AidlLocation& location, const Comments& comments,
std::vector<std::unique_ptr<AidlImport>> imports,
- std::vector<std::unique_ptr<AidlDefinedType>> defined_types);
+ std::vector<std::unique_ptr<AidlDefinedType>> defined_types, bool is_preprocessed);
~AidlDocument() = default;
// non-copyable, non-movable
@@ -1236,6 +1240,7 @@
const std::vector<std::unique_ptr<AidlDefinedType>>& DefinedTypes() const {
return defined_types_;
}
+ bool IsPreprocessed() const { return is_preprocessed_; }
void TraverseChildren(std::function<void(const AidlNode&)> traverse) const override {
for (const auto& i : Imports()) {
@@ -1250,6 +1255,7 @@
private:
const std::vector<std::unique_ptr<AidlImport>> imports_;
const std::vector<std::unique_ptr<AidlDefinedType>> defined_types_;
+ bool is_preprocessed_;
};
template <typename T>
diff --git a/aidl_language_y.yy b/aidl_language_y.yy
index 7dc1f94..aa82c4d 100644
--- a/aidl_language_y.yy
+++ b/aidl_language_y.yy
@@ -192,7 +192,7 @@
} else if (!$2->empty()) {
comments = $2->front()->GetComments();
}
- ps->SetDocument(std::make_unique<AidlDocument>(loc(@1), comments, std::move(*$2), std::move(*$3)));
+ ps->MakeDocument(loc(@1), comments, std::move(*$2), std::move(*$3));
delete $1;
delete $2;
delete $3;
diff --git a/aidl_typenames.cpp b/aidl_typenames.cpp
index 6c8c9a5..90a8393 100644
--- a/aidl_typenames.cpp
+++ b/aidl_typenames.cpp
@@ -117,53 +117,53 @@
return false;
}
-bool AidlTypenames::AddDocument(std::unique_ptr<AidlDocument> doc, bool is_preprocessed) {
+// Add a parsed document and populate type names in it.
+// Name conflict is an error unless one of them is from preprocessed.
+// For legacy, we populate unqualified names from preprocessed unstructured parcelable types
+// so that they can be referenced via a simple name.
+bool AidlTypenames::AddDocument(std::unique_ptr<AidlDocument> doc) {
+ bool is_preprocessed = doc->IsPreprocessed();
for (const auto& type : doc->DefinedTypes()) {
- // ParcelFileDescriptor is treated as a built-in type, but it's also in the framework.aidl.
- // So aidl should ignore built-in types in framework.aidl to prevent duplication.
- // (b/130899491)
- if (is_preprocessed && IsBuiltinTypename(type->GetName())) {
- continue;
+ if (IsBuiltinTypename(type->GetName())) {
+ // ParcelFileDescriptor is treated as a built-in type, but it's also in the framework.aidl.
+ // So aidl should ignore built-in types in framework.aidl to prevent duplication.
+ // (b/130899491)
+ if (is_preprocessed) {
+ continue;
+ }
+ // HasValidNameComponents handles name conflicts with built-in types
}
if (auto prev_definition = defined_types_.find(type->GetCanonicalName());
prev_definition != defined_types_.end()) {
- AIDL_ERROR(type) << "redefinition:" << type->GetCanonicalName() << " is defined "
- << prev_definition->second->GetLocation();
- return false;
+ // Skip duplicate type in preprocessed document
+ if (is_preprocessed) {
+ continue;
+ }
+ // Overwrite duplicate type which is already added via preprocessed with a new one
+ if (!prev_definition->second->GetDocument().IsPreprocessed()) {
+ AIDL_ERROR(type) << "redefinition: " << type->GetCanonicalName() << " is defined "
+ << prev_definition->second->GetLocation();
+ return false;
+ }
}
+
if (!HasValidNameComponents(*type)) {
return false;
}
+
+ // populate global 'type' namespace with fully-qualified names
+ defined_types_.emplace(type->GetCanonicalName(), type.get());
+ // preprocessed unstructured parcelable types can be referenced without qualification
+ if (is_preprocessed && type->AsUnstructuredParcelable()) {
+ defined_types_.emplace(type->GetName(), type.get());
+ }
}
// transfer ownership of document
documents_.push_back(std::move(doc));
- // populate global 'type' namespace with fully-qualified names
- for (const auto& type : documents_.back()->DefinedTypes()) {
- defined_types_.emplace(type->GetCanonicalName(), type.get());
- }
- if (is_preprocessed) {
- // preprocessed unstructured parcelable types can be referenced without qualification
- for (const auto& type : documents_.back()->DefinedTypes()) {
- if (type->AsUnstructuredParcelable()) {
- defined_types_.emplace(type->GetName(), type.get());
- }
- }
- }
return true;
}
-const AidlDocument* AidlTypenames::GetDocumentFor(const AidlDefinedType* type) const {
- for (const auto& doc : AllDocuments()) {
- for (const auto& defined_type : doc->DefinedTypes()) {
- if (defined_type.get() == type) {
- return doc.get();
- }
- }
- }
- return nullptr;
-}
-
const AidlDocument& AidlTypenames::MainDocument() const {
AIDL_FATAL_IF(documents_.size() == 0, AIDL_LOCATION_HERE) << "Main document doesn't exist";
return *(documents_[0]);
diff --git a/aidl_typenames.h b/aidl_typenames.h
index d33acb9..8956075 100644
--- a/aidl_typenames.h
+++ b/aidl_typenames.h
@@ -57,8 +57,7 @@
class AidlTypenames final {
public:
AidlTypenames() = default;
- bool AddDocument(std::unique_ptr<AidlDocument> doc, bool is_preprocessed);
- const AidlDocument* GetDocumentFor(const AidlDefinedType* type) const;
+ bool AddDocument(std::unique_ptr<AidlDocument> doc);
const std::vector<std::unique_ptr<AidlDocument>>& AllDocuments() const { return documents_; }
const AidlDocument& MainDocument() const;
static bool IsBuiltinTypename(const string& type_name);
diff --git a/aidl_unittest.cpp b/aidl_unittest.cpp
index 27bf194..5e887e2 100644
--- a/aidl_unittest.cpp
+++ b/aidl_unittest.cpp
@@ -819,6 +819,58 @@
EXPECT_THAT(code, testing::HasSubstr("public static final int y = 43;"));
}
+TEST_F(AidlTest, AllowMultipleUnstructuredNestedParcelablesInASingleDocument) {
+ io_delegate_.SetFileContents("p/IFoo.aidl",
+ "package p;\n"
+ "import x.Outer;\n"
+ "interface IFoo {\n"
+ " void foo(in Outer.Inner1 in1, in Outer.Inner2 in2);\n"
+ "}");
+ io_delegate_.SetFileContents("imported/x/Outer.aidl",
+ "package x;\n"
+ "parcelable Outer.Inner1;\n"
+ "parcelable Outer.Inner2;\n");
+ auto opt = Options::From("aidl -Iimported --lang=java p/IFoo.aidl");
+ CaptureStderr();
+ EXPECT_EQ(0, ::android::aidl::compile_aidl(opt, io_delegate_));
+ EXPECT_EQ("", GetCapturedStderr());
+}
+
+TEST_F(AidlTest,
+ StubsSourceIsGeneratedFromDuplicateDefinitionWithFrameworkAidl_FrameworkAidlLater) {
+ // Main doc(Foo.aidl) is loaded
+ // And then framework.aidl is loaded as preprocessed. (conflict)
+ io_delegate_.SetFileContents("sdk/framework.aidl", "parcelable x.Foo.Inner;\n");
+ io_delegate_.SetFileContents("x/Foo.aidl",
+ "package x;\n"
+ "parcelable Foo.Inner;\n");
+ auto opt = Options::From("aidl -psdk/framework.aidl -I. x/Foo.aidl");
+ CaptureStderr();
+ EXPECT_EQ(0, ::android::aidl::compile_aidl(opt, io_delegate_));
+ EXPECT_EQ("", GetCapturedStderr());
+}
+
+TEST_F(AidlTest,
+ StubsSourceIsGeneratedFromDuplicateDefinitionWithFrameworkAidl_FrameworkAidlFirst) {
+ // Main doc(IBar.aidl) is loaded first.
+ // Framework.aidl is loaded as preprocessed.
+ // And then import(Foo.aidl) is loaded. (conflict)
+ io_delegate_.SetFileContents("sdk/framework.aidl", "parcelable x.Foo.Inner;\n");
+ io_delegate_.SetFileContents("x/IBar.aidl",
+ "package x;\n"
+ "import x.Foo;\n"
+ "interface IBar {\n"
+ " void bar(in Foo.Inner inner);\n"
+ "}");
+ io_delegate_.SetFileContents("x/Foo.aidl",
+ "package x;\n"
+ "parcelable Foo.Inner;\n");
+ auto opt = Options::From("aidl -psdk/framework.aidl -I. x/IBar.aidl");
+ CaptureStderr();
+ EXPECT_EQ(0, ::android::aidl::compile_aidl(opt, io_delegate_));
+ EXPECT_EQ("", GetCapturedStderr());
+}
+
TEST_F(AidlTest, PreprocessedFileCantDeclarePackage) {
string simple_content = "package xxx; parcelable a.Foo;";
io_delegate_.SetFileContents("path", simple_content);
diff --git a/parser.cpp b/parser.cpp
index d2fe362..8d73799 100644
--- a/parser.cpp
+++ b/parser.cpp
@@ -54,7 +54,7 @@
// transfer ownership to AidlTypenames and return the raw pointer
const AidlDocument* result = parser.document_.get();
- if (!typenames.AddDocument(std::move(parser.document_), is_preprocessed)) {
+ if (!typenames.AddDocument(std::move(parser.document_))) {
return nullptr;
}
return result;
@@ -192,3 +192,11 @@
yy_delete_buffer(buffer_, scanner_);
yylex_destroy(scanner_);
}
+
+void Parser::MakeDocument(const AidlLocation& location, const Comments& comments,
+ std::vector<std::unique_ptr<AidlImport>> imports,
+ std::vector<std::unique_ptr<AidlDefinedType>> defined_types) {
+ AIDL_FATAL_IF(document_.get(), location);
+ document_ = std::make_unique<AidlDocument>(location, comments, std::move(imports),
+ std::move(defined_types), is_preprocessed_);
+}
\ No newline at end of file
diff --git a/parser.h b/parser.h
index f286de2..20599cc 100644
--- a/parser.h
+++ b/parser.h
@@ -91,7 +91,9 @@
void SetPackage(const AidlPackage& package);
const std::string& Package() const { return package_; }
- void SetDocument(std::unique_ptr<AidlDocument> document) { document_ = std::move(document); }
+ void MakeDocument(const AidlLocation& location, const Comments& comments,
+ std::vector<std::unique_ptr<AidlImport>> imports,
+ std::vector<std::unique_ptr<AidlDefinedType>> defined_types);
private:
explicit Parser(const std::string& filename, std::string& raw_buffer, bool is_preprocessed);
