Don't call virtual methods in TypeNamespace constructors
While probably safe in this particular context because of this specific
class structure, it is bad practice to call virtual methods in constructors.
Bug: None
Test: unittests continue to pass
Change-Id: I3c9a435efd89a2c2bd7ce576aed585b8ab766bda
diff --git a/aidl.cpp b/aidl.cpp
index 3cff7e0..a9b2d6e 100644
--- a/aidl.cpp
+++ b/aidl.cpp
@@ -561,6 +561,7 @@
unique_ptr<AidlInterface> interface;
std::vector<std::unique_ptr<AidlImport>> imports;
unique_ptr<cpp::TypeNamespace> types(new cpp::TypeNamespace());
+ types->Init();
int err = internals::load_and_validate_aidl(
std::vector<std::string>{}, // no preprocessed files
options.ImportPaths(),
@@ -583,6 +584,7 @@
unique_ptr<AidlInterface> interface;
std::vector<std::unique_ptr<AidlImport>> imports;
unique_ptr<java::JavaTypeNamespace> types(new java::JavaTypeNamespace());
+ types->Init();
int err = internals::load_and_validate_aidl(
options.preprocessed_files_,
options.import_paths_,
diff --git a/aidl_unittest.cpp b/aidl_unittest.cpp
index 7515f6f..05ca02c 100644
--- a/aidl_unittest.cpp
+++ b/aidl_unittest.cpp
@@ -36,6 +36,11 @@
class AidlTest : public ::testing::Test {
protected:
+ void SetUp() override {
+ java_types_.Init();
+ cpp_types_.Init();
+ }
+
unique_ptr<AidlInterface> Parse(const string& path,
const string& contents,
TypeNamespace* types) {
@@ -55,11 +60,12 @@
FakeIoDelegate io_delegate_;
vector<string> import_paths_;
+ java::JavaTypeNamespace java_types_;
+ cpp::TypeNamespace cpp_types_;
};
TEST_F(AidlTest, JavaAcceptsMissingPackage) {
- java::JavaTypeNamespace types;
- EXPECT_NE(nullptr, Parse("IFoo.aidl", "interface IFoo { }", &types));
+ EXPECT_NE(nullptr, Parse("IFoo.aidl", "interface IFoo { }", &java_types_));
}
TEST_F(AidlTest, RejectsArraysOfBinders) {
@@ -70,50 +76,41 @@
string contents = "package foo;\n"
"import bar.IBar;\n"
"interface IFoo { void f(in IBar[] input); }";
- java::JavaTypeNamespace java_types;
- EXPECT_EQ(nullptr, Parse(path, contents, &java_types));
- cpp::TypeNamespace cpp_types;
- EXPECT_EQ(nullptr, Parse(path, contents, &cpp_types));
+ EXPECT_EQ(nullptr, Parse(path, contents, &java_types_));
+ EXPECT_EQ(nullptr, Parse(path, contents, &cpp_types_));
}
TEST_F(AidlTest, CppRejectsMissingPackage) {
- cpp::TypeNamespace types;
- EXPECT_EQ(nullptr, Parse("IFoo.aidl", "interface IFoo { }", &types));
+ EXPECT_EQ(nullptr, Parse("IFoo.aidl", "interface IFoo { }", &cpp_types_));
EXPECT_NE(nullptr,
- Parse("a/IFoo.aidl", "package a; interface IFoo { }", &types));
+ Parse("a/IFoo.aidl", "package a; interface IFoo { }", &cpp_types_));
}
TEST_F(AidlTest, RejectsOnewayOutParameters) {
- cpp::TypeNamespace cpp_types;
- java::JavaTypeNamespace java_types;
string oneway_interface =
"package a; oneway interface IFoo { void f(out int bar); }";
string oneway_method =
"package a; interface IBar { oneway void f(out int bar); }";
- EXPECT_EQ(nullptr, Parse("a/IFoo.aidl", oneway_interface, &cpp_types));
- EXPECT_EQ(nullptr, Parse("a/IFoo.aidl", oneway_interface, &java_types));
- EXPECT_EQ(nullptr, Parse("a/IBar.aidl", oneway_method, &cpp_types));
- EXPECT_EQ(nullptr, Parse("a/IBar.aidl", oneway_method, &java_types));
+ EXPECT_EQ(nullptr, Parse("a/IFoo.aidl", oneway_interface, &cpp_types_));
+ EXPECT_EQ(nullptr, Parse("a/IFoo.aidl", oneway_interface, &java_types_));
+ EXPECT_EQ(nullptr, Parse("a/IBar.aidl", oneway_method, &cpp_types_));
+ EXPECT_EQ(nullptr, Parse("a/IBar.aidl", oneway_method, &java_types_));
}
TEST_F(AidlTest, RejectsOnewayNonVoidReturn) {
- cpp::TypeNamespace cpp_types;
- java::JavaTypeNamespace java_types;
string oneway_method = "package a; interface IFoo { oneway int f(); }";
- EXPECT_EQ(nullptr, Parse("a/IFoo.aidl", oneway_method, &cpp_types));
- EXPECT_EQ(nullptr, Parse("a/IFoo.aidl", oneway_method, &java_types));
+ EXPECT_EQ(nullptr, Parse("a/IFoo.aidl", oneway_method, &cpp_types_));
+ EXPECT_EQ(nullptr, Parse("a/IFoo.aidl", oneway_method, &java_types_));
}
TEST_F(AidlTest, AcceptsOneway) {
- cpp::TypeNamespace cpp_types;
- java::JavaTypeNamespace java_types;
string oneway_method = "package a; interface IFoo { oneway void f(int a); }";
string oneway_interface =
"package a; oneway interface IBar { void f(int a); }";
- EXPECT_NE(nullptr, Parse("a/IFoo.aidl", oneway_method, &cpp_types));
- EXPECT_NE(nullptr, Parse("a/IFoo.aidl", oneway_method, &java_types));
- EXPECT_NE(nullptr, Parse("a/IBar.aidl", oneway_interface, &cpp_types));
- EXPECT_NE(nullptr, Parse("a/IBar.aidl", oneway_interface, &java_types));
+ EXPECT_NE(nullptr, Parse("a/IFoo.aidl", oneway_method, &cpp_types_));
+ EXPECT_NE(nullptr, Parse("a/IFoo.aidl", oneway_method, &java_types_));
+ EXPECT_NE(nullptr, Parse("a/IBar.aidl", oneway_interface, &cpp_types_));
+ EXPECT_NE(nullptr, Parse("a/IBar.aidl", oneway_interface, &java_types_));
}
} // namespace aidl
} // namespace android
diff --git a/ast_java_unittest.cpp b/ast_java_unittest.cpp
index 7c09684..3981363 100644
--- a/ast_java_unittest.cpp
+++ b/ast_java_unittest.cpp
@@ -38,6 +38,7 @@
TEST(AstJavaTests, GeneratesClass) {
JavaTypeNamespace types;
+ types.Init();
Type class_type(&types, "TestClass", ValidatableType::KIND_GENERATED,
false, false);
Type extend_type(&types, "SuperClass", ValidatableType::KIND_BUILT_IN,
diff --git a/generate_cpp_unittest.cpp b/generate_cpp_unittest.cpp
index 93336ef..f5518f5 100644
--- a/generate_cpp_unittest.cpp
+++ b/generate_cpp_unittest.cpp
@@ -364,7 +364,9 @@
protected:
ASTTest(string file_path, string file_contents)
: file_path_(file_path),
- file_contents_(file_contents) {}
+ file_contents_(file_contents) {
+ types_.Init();
+ }
unique_ptr<AidlInterface> Parse() {
io_delegate_.SetFileContents(file_path_, file_contents_);
diff --git a/type_cpp.cpp b/type_cpp.cpp
index 85e15e4..0cf4767 100644
--- a/type_cpp.cpp
+++ b/type_cpp.cpp
@@ -168,7 +168,7 @@
}
}
-TypeNamespace::TypeNamespace() {
+void TypeNamespace::Init() {
Add(new PrimitiveType(
ValidatableType::KIND_BUILT_IN, kNoPackage, "byte",
"cstdint", "int8_t", "readByte", "writeByte",
diff --git a/type_cpp.h b/type_cpp.h
index 214252b..98eb211 100644
--- a/type_cpp.h
+++ b/type_cpp.h
@@ -87,9 +87,10 @@
class TypeNamespace : public ::android::aidl::LanguageTypeNamespace<Type> {
public:
- TypeNamespace();
+ TypeNamespace() = default;
virtual ~TypeNamespace() = default;
+ void Init() override;
bool AddParcelableType(const AidlParcelable* p,
const std::string& filename) override;
bool AddBinderType(const AidlInterface* b,
diff --git a/type_cpp_unittest.cpp b/type_cpp_unittest.cpp
index a2e8f74..d4232a0 100644
--- a/type_cpp_unittest.cpp
+++ b/type_cpp_unittest.cpp
@@ -26,6 +26,9 @@
class CppTypeNamespaceTest : public ::testing::Test {
protected:
+ void SetUp() override {
+ types_.Init();
+ }
TypeNamespace types_;
};
diff --git a/type_java.cpp b/type_java.cpp
index e922f9f..ff05440 100644
--- a/type_java.cpp
+++ b/type_java.cpp
@@ -705,7 +705,7 @@
// ================================================================
-JavaTypeNamespace::JavaTypeNamespace() {
+void JavaTypeNamespace::Init() {
Add(new BasicType(this, "void", "XXX", "XXX", "XXX", "XXX", "XXX"));
m_bool_type = new BooleanType(this);
diff --git a/type_java.h b/type_java.h
index bf348c4..a5e0e0a 100644
--- a/type_java.h
+++ b/type_java.h
@@ -365,9 +365,10 @@
class JavaTypeNamespace : public LanguageTypeNamespace<Type> {
public:
- JavaTypeNamespace();
+ JavaTypeNamespace() = default;
virtual ~JavaTypeNamespace() = default;
+ void Init() override;
bool AddParcelableType(const AidlParcelable* p,
const string& filename) override;
bool AddBinderType(const AidlInterface* b,
diff --git a/type_java_unittest.cpp b/type_java_unittest.cpp
index 6be2b1a..e435480 100644
--- a/type_java_unittest.cpp
+++ b/type_java_unittest.cpp
@@ -29,6 +29,9 @@
class JavaTypeNamespaceTest : public ::testing::Test {
protected:
+ void SetUp() override {
+ types_.Init();
+ }
JavaTypeNamespace types_;
};
diff --git a/type_namespace.h b/type_namespace.h
index b5719b0..7aaba7b 100644
--- a/type_namespace.h
+++ b/type_namespace.h
@@ -67,6 +67,10 @@
class TypeNamespace {
public:
+ // Load the TypeNamespace with built in types. Don't do work in the
+ // constructor because many of the useful methods are virtual.
+ virtual void Init() = 0;
+
// Load this TypeNamespace with user defined types.
virtual bool AddParcelableType(const AidlParcelable* p,
const std::string& filename) = 0;
