Change C++ output to match make expectations
- generate a single C++ output file
- put headers into an output directory
- create the output directory structure using an IoDelegate
Bug: 23599697
Test: Unittests pass
Change-Id: I0f7b821678cd5adc00b2fdba06b52b235c724467
diff --git a/aidl.cpp b/aidl.cpp
index 464cd52..c586883 100644
--- a/aidl.cpp
+++ b/aidl.cpp
@@ -608,7 +608,7 @@
// TODO(wiley) b/23600457 generate a dependency file if requested with -b
- return (cpp::GenerateCpp(options, *types, *interface)) ? 0 : 1;
+ return (cpp::GenerateCpp(options, *types, *interface, io_delegate)) ? 0 : 1;
}
int compile_aidl_to_java(const JavaOptions& options,
diff --git a/generate_cpp.cpp b/generate_cpp.cpp
index d2180dc..3c571b5 100644
--- a/generate_cpp.cpp
+++ b/generate_cpp.cpp
@@ -30,9 +30,11 @@
#include "ast_cpp.h"
#include "code_writer.h"
#include "logging.h"
+#include "os.h"
using android::base::StringPrintf;
using android::base::Join;
+using android::base::Split;
using std::string;
using std::unique_ptr;
using std::vector;
@@ -172,6 +174,24 @@
return c_name;
}
+string HeaderFile(const AidlInterface& interface,
+ ClassNames class_type,
+ bool use_os_sep = true) {
+ string file_path = interface.GetPackage();
+ for (char& c: file_path) {
+ if (c == '.') {
+ c = (use_os_sep) ? OS_PATH_SEPARATOR : '/';
+ }
+ }
+ if (!file_path.empty()) {
+ file_path += (use_os_sep) ? OS_PATH_SEPARATOR : '/';
+ }
+ file_path += ClassName(interface, class_type);
+ file_path += ".h";
+
+ return file_path;
+}
+
string BuildHeaderGuard(const AidlInterface& interface,
ClassNames header_type) {
string class_name = ClassName(interface, header_type);
@@ -260,14 +280,16 @@
unique_ptr<Document> BuildClientSource(const TypeNamespace& types,
const AidlInterface& interface) {
- const string bp_name = ClassName(interface, ClassNames::CLIENT);
- vector<string> include_list = { bp_name + ".h", kParcelHeader };
+ vector<string> include_list = {
+ HeaderFile(interface, ClassNames::CLIENT, false),
+ kParcelHeader
+ };
vector<unique_ptr<Declaration>> file_decls;
// The constructor just passes the IBinder instance up to the super
// class.
file_decls.push_back(unique_ptr<Declaration>{new ConstructorImpl{
- bp_name,
+ ClassName(interface, ClassNames::CLIENT),
ArgList{"const android::sp<android::IBinder>& impl"},
{ "BpInterface<IPingResponder>(impl)" }}});
@@ -348,9 +370,12 @@
} // namespace
unique_ptr<Document> BuildServerSource(const TypeNamespace& types,
- const AidlInterface& parsed_doc) {
- const string bn_name = ClassName(parsed_doc, ClassNames::SERVER);
- vector<string> include_list{bn_name + ".h", kParcelHeader};
+ const AidlInterface& interface) {
+ const string bn_name = ClassName(interface, ClassNames::SERVER);
+ vector<string> include_list{
+ HeaderFile(interface, ClassNames::SERVER, false),
+ kParcelHeader
+ };
unique_ptr<MethodImpl> on_transact{new MethodImpl{
kAndroidStatusLiteral, bn_name, "onTransact",
ArgList{{"uint32_t code",
@@ -368,7 +393,7 @@
on_transact->GetStatementBlock()->AddStatement(s);
// The switch statement has a case statement for each transaction code.
- for (const auto& method : parsed_doc.GetMethods()) {
+ for (const auto& method : interface.GetMethods()) {
StatementBlock* b = s->AddCase("Call::" + UpperCase(method->GetName()));
if (!b) { return nullptr; }
@@ -390,19 +415,20 @@
}
unique_ptr<Document> BuildInterfaceSource(const TypeNamespace& /* types */,
- const AidlInterface& parsed_doc) {
- const string i_name = ClassName(parsed_doc, ClassNames::INTERFACE);
- const string bp_name = ClassName(parsed_doc, ClassNames::CLIENT);
- vector<string> include_list{i_name + ".h", bp_name + ".h"};
+ const AidlInterface& interface) {
+ vector<string> include_list{
+ HeaderFile(interface, ClassNames::INTERFACE, false),
+ HeaderFile(interface, ClassNames::CLIENT, false),
+ };
- string fq_name = i_name;
- if (!parsed_doc.GetPackage().empty()) {
- fq_name = StringPrintf("%s.%s", parsed_doc.GetPackage().c_str(), i_name.c_str());
+ string fq_name = ClassName(interface, ClassNames::INTERFACE);
+ if (!interface.GetPackage().empty()) {
+ fq_name = interface.GetPackage() + "." + fq_name;
}
unique_ptr<ConstructorDecl> meta_if{new ConstructorDecl{
"IMPLEMENT_META_INTERFACE",
- ArgList{vector<string>{ClassName(parsed_doc, ClassNames::BASE),
+ ArgList{vector<string>{ClassName(interface, ClassNames::BASE),
'"' + fq_name + '"'}}}};
return unique_ptr<Document>{new CppSource{
@@ -445,7 +471,7 @@
{kIBinderHeader,
kIInterfaceHeader,
"utils/Errors.h",
- i_name + ".h"},
+ HeaderFile(interface, ClassNames::INTERFACE, false)},
NestInNamespaces(std::move(bp_class))}};
}
@@ -476,7 +502,7 @@
return unique_ptr<Document>{new CppHeader{
BuildHeaderGuard(interface, ClassNames::SERVER),
{"binder/IInterface.h",
- i_name + ".h"},
+ HeaderFile(interface, ClassNames::INTERFACE, false)},
NestInNamespaces(std::move(bn_class))}};
}
@@ -522,12 +548,35 @@
NestInNamespaces(std::move(if_class))}};
}
-bool GenerateCppForFile(const std::string& name, unique_ptr<Document> doc) {
- if (!doc) {
+bool WriteHeader(const CppOptions& options,
+ const TypeNamespace& types,
+ const AidlInterface& interface,
+ const IoDelegate& io_delegate,
+ ClassNames header_type) {
+ unique_ptr<Document> header;
+ switch (header_type) {
+ case ClassNames::INTERFACE:
+ header = BuildInterfaceHeader(types, interface);
+ break;
+ case ClassNames::CLIENT:
+ header = BuildClientHeader(types, interface);
+ break;
+ case ClassNames::SERVER:
+ header = BuildServerHeader(types, interface);
+ break;
+ default:
+ LOG(FATAL) << "aidl internal error";
+ }
+ if (!header) {
+ LOG(ERROR) << "aidl internal error: Failed to generate header.";
return false;
}
- unique_ptr<CodeWriter> writer = GetFileWriter(name);
- doc->Write(writer.get());
+
+ // TODO(wiley): b/25026025 error checking for file I/O
+ header->Write(io_delegate.GetCodeWriter(
+ options.OutputHeaderDir() + OS_PATH_SEPARATOR +
+ HeaderFile(interface, header_type)).get());
+
return true;
}
@@ -537,23 +586,40 @@
bool GenerateCpp(const CppOptions& options,
const TypeNamespace& types,
- const AidlInterface& parsed_doc) {
- bool success = true;
+ const AidlInterface& interface,
+ const IoDelegate& io_delegate) {
+ auto interface_src = BuildInterfaceSource(types, interface);
+ auto client_src = BuildClientSource(types, interface);
+ auto server_src = BuildServerSource(types, interface);
- success &= GenerateCppForFile(options.ClientCppFileName(),
- BuildClientSource(types, parsed_doc));
- success &= GenerateCppForFile(options.ClientHeaderFileName(),
- BuildClientHeader(types, parsed_doc));
- success &= GenerateCppForFile(options.ServerCppFileName(),
- BuildServerSource(types, parsed_doc));
- success &= GenerateCppForFile(options.ServerHeaderFileName(),
- BuildServerHeader(types, parsed_doc));
- success &= GenerateCppForFile(options.InterfaceCppFileName(),
- BuildInterfaceSource(types, parsed_doc));
- success &= GenerateCppForFile(options.InterfaceHeaderFileName(),
- BuildInterfaceHeader(types, parsed_doc));
+ if (!interface_src || !client_src || !server_src) {
+ return false;
+ }
- return success;
+ if (!io_delegate.CreatedNestedDirs(options.OutputHeaderDir(),
+ Split(interface.GetPackage(), "."))) {
+ LOG(ERROR) << "Failed to create directory structure for headers.";
+ return false;
+ }
+
+ if (!WriteHeader(options, types, interface, io_delegate,
+ ClassNames::INTERFACE) ||
+ !WriteHeader(options, types, interface, io_delegate,
+ ClassNames::CLIENT) ||
+ !WriteHeader(options, types, interface, io_delegate,
+ ClassNames::SERVER)) {
+ return false;
+ }
+
+ // TODO(wiley): b/25026025 error checking for file I/O.
+ // If it fails, we should remove all the partial results.
+ unique_ptr<CodeWriter> writer = io_delegate.GetCodeWriter(
+ options.OutputCppFilePath());
+ interface_src->Write(writer.get());
+ client_src->Write(writer.get());
+ server_src->Write(writer.get());
+
+ return true;
}
} // namespace cpp
diff --git a/generate_cpp.h b/generate_cpp.h
index 86aea87..474bdde 100644
--- a/generate_cpp.h
+++ b/generate_cpp.h
@@ -28,7 +28,8 @@
bool GenerateCpp(const CppOptions& options,
const cpp::TypeNamespace& types,
- const AidlInterface& parsed_doc);
+ const AidlInterface& parsed_doc,
+ const IoDelegate& io_delegate);
namespace internals {
std::unique_ptr<Document> BuildClientSource(const TypeNamespace& types,
diff --git a/generate_cpp_unittest.cpp b/generate_cpp_unittest.cpp
index 0ba8f7a..8ef3b4c 100644
--- a/generate_cpp_unittest.cpp
+++ b/generate_cpp_unittest.cpp
@@ -36,12 +36,15 @@
namespace {
const char kTrivialInterfaceAIDL[] =
-R"(interface IPingResponder {
+R"(
+package android.os;
+
+interface IPingResponder {
int Ping(String token);
})";
const char kExpectedTrivialClientSourceOutput[] =
-R"(#include <BpPingResponder.h>
+R"(#include <android/os/BpPingResponder.h>
#include <binder/Parcel.h>
namespace android {
@@ -71,11 +74,11 @@
)";
const char kExpectedTrivialServerHeaderOutput[] =
-R"(#ifndef AIDL_GENERATED__BN_PING_RESPONDER_H_
-#define AIDL_GENERATED__BN_PING_RESPONDER_H_
+R"(#ifndef AIDL_GENERATED_ANDROID_OS_BN_PING_RESPONDER_H_
+#define AIDL_GENERATED_ANDROID_OS_BN_PING_RESPONDER_H_
#include <binder/IInterface.h>
-#include <IPingResponder.h>
+#include <android/os/IPingResponder.h>
namespace android {
@@ -90,10 +93,10 @@
} // namespace android
-#endif // AIDL_GENERATED__BN_PING_RESPONDER_H_)";
+#endif // AIDL_GENERATED_ANDROID_OS_BN_PING_RESPONDER_H_)";
const char kExpectedTrivialServerSourceOutput[] =
-R"(#include <BnPingResponder.h>
+R"(#include <android/os/BnPingResponder.h>
#include <binder/Parcel.h>
namespace android {
@@ -130,13 +133,13 @@
)";
const char kExpectedTrivialClientHeaderOutput[] =
-R"(#ifndef AIDL_GENERATED__BP_PING_RESPONDER_H_
-#define AIDL_GENERATED__BP_PING_RESPONDER_H_
+R"(#ifndef AIDL_GENERATED_ANDROID_OS_BP_PING_RESPONDER_H_
+#define AIDL_GENERATED_ANDROID_OS_BP_PING_RESPONDER_H_
#include <binder/IBinder.h>
#include <binder/IInterface.h>
#include <utils/Errors.h>
-#include <IPingResponder.h>
+#include <android/os/IPingResponder.h>
namespace android {
@@ -153,11 +156,11 @@
} // namespace android
-#endif // AIDL_GENERATED__BP_PING_RESPONDER_H_)";
+#endif // AIDL_GENERATED_ANDROID_OS_BP_PING_RESPONDER_H_)";
const char kExpectedTrivialInterfaceHeaderOutput[] =
-R"(#ifndef AIDL_GENERATED__I_PING_RESPONDER_H_
-#define AIDL_GENERATED__I_PING_RESPONDER_H_
+R"(#ifndef AIDL_GENERATED_ANDROID_OS_I_PING_RESPONDER_H_
+#define AIDL_GENERATED_ANDROID_OS_I_PING_RESPONDER_H_
#include <binder/IBinder.h>
#include <binder/IInterface.h>
@@ -181,17 +184,17 @@
} // namespace android
-#endif // AIDL_GENERATED__I_PING_RESPONDER_H_)";
+#endif // AIDL_GENERATED_ANDROID_OS_I_PING_RESPONDER_H_)";
const char kExpectedTrivialInterfaceSourceOutput[] =
-R"(#include <IPingResponder.h>
-#include <BpPingResponder.h>
+R"(#include <android/os/IPingResponder.h>
+#include <android/os/BpPingResponder.h>
namespace android {
namespace generated {
-IMPLEMENT_META_INTERFACE(PingResponder, "IPingResponder");
+IMPLEMENT_META_INTERFACE(PingResponder, "android.os.IPingResponder");
} // namespace generated
@@ -205,7 +208,7 @@
AidlInterface* Parse() {
FakeIoDelegate io_delegate;
- io_delegate.SetFileContents("IPingResponder.aidl", kTrivialInterfaceAIDL);
+ io_delegate.SetFileContents("android/os/IPingResponder.aidl", kTrivialInterfaceAIDL);
cpp::TypeNamespace types;
AidlInterface* ret = nullptr;
@@ -213,7 +216,7 @@
int err = ::android::aidl::internals::load_and_validate_aidl(
{}, // no preprocessed files
{}, // no import paths
- "IPingResponder.aidl",
+ "android/os/IPingResponder.aidl",
io_delegate,
&types,
&ret,
diff --git a/io_delegate.cpp b/io_delegate.cpp
index 5703e4c..ecaaae4 100644
--- a/io_delegate.cpp
+++ b/io_delegate.cpp
@@ -16,10 +16,22 @@
#include "io_delegate.h"
+#include <cstring>
#include <fstream>
+#include <vector>
+
+#ifdef _WIN32
+#include <direct.h>
+#else
+#include <sys/stat.h>
+#endif
+
+#include "logging.h"
+#include "os.h"
using std::string;
using std::unique_ptr;
+using std::vector;
namespace android {
namespace aidl {
@@ -55,5 +67,38 @@
return (0 == access(path.c_str(), R_OK));
#endif
}
+
+bool IoDelegate::CreatedNestedDirs(
+ const string& caller_base_dir,
+ const vector<string>& nested_subdirs) const {
+ string base_dir = caller_base_dir;
+ if (base_dir.empty()) {
+ base_dir = ".";
+ }
+ for (const string& subdir : nested_subdirs) {
+ if (base_dir[base_dir.size() - 1] != OS_PATH_SEPARATOR) {
+ base_dir += OS_PATH_SEPARATOR;
+ }
+ base_dir += subdir;
+ bool success;
+#ifdef _WIN32
+ success = _mkdir(base_dir.c_str()) == 0;
+#else
+ success = mkdir(base_dir.c_str(),
+ S_IRWXU | S_IRWXG | S_IROTH | S_IXOTH) == 0;
+#endif
+ if (!success && errno != EEXIST) {
+ LOG(ERROR) << "Error while creating directories: " << strerror(errno);
+ return false;
+ }
+ }
+ return true;
+}
+
+unique_ptr<CodeWriter> IoDelegate::GetCodeWriter(
+ const string& file_path) const {
+ return GetFileWriter(file_path);
+}
+
} // namespace android
} // namespace aidl
diff --git a/io_delegate.h b/io_delegate.h
index 9be713c..6f51013 100644
--- a/io_delegate.h
+++ b/io_delegate.h
@@ -21,6 +21,9 @@
#include <memory>
#include <string>
+#include <vector>
+
+#include "code_writer.h"
namespace android {
namespace aidl {
@@ -38,6 +41,13 @@
virtual bool FileIsReadable(const std::string& path) const;
+ virtual bool CreatedNestedDirs(
+ const std::string& base_dir,
+ const std::vector<std::string>& nested_subdirs) const;
+
+ virtual std::unique_ptr<CodeWriter> GetCodeWriter(
+ const std::string& file_path) const;
+
private:
DISALLOW_COPY_AND_ASSIGN(IoDelegate);
}; // class IoDelegate
diff --git a/options.cpp b/options.cpp
index d966999..21b743f 100644
--- a/options.cpp
+++ b/options.cpp
@@ -16,6 +16,7 @@
#include "options.h"
+#include <cstring>
#include <iostream>
#include <stdio.h>
@@ -173,7 +174,7 @@
namespace {
unique_ptr<CppOptions> cpp_usage() {
- cerr << "usage: aidl-cpp INPUT_FILE OUTPUT_DIR" << endl
+ cerr << "usage: aidl-cpp INPUT_FILE HEADER_DIR OUTPUT_FILE" << endl
<< endl
<< "OPTIONS:" << endl
<< " -I<DIR> search path for import statements" << endl
@@ -181,8 +182,10 @@
<< endl
<< "INPUT_FILE:" << endl
<< " an aidl interface file" << endl
- << "OUTPUT_DIR:" << endl
- << " directory to put generated code" << endl;
+ << "HEADER_DIR:" << endl
+ << " empty directory to put generated headers" << endl
+ << "OUTPUT_FILE:" << endl
+ << " path to write generated .cpp code" << endl;
return unique_ptr<CppOptions>(nullptr);
}
@@ -214,86 +217,25 @@
}
}
- // There are exactly two positional arguments.
+ // There are exactly three positional arguments.
const int remaining_args = argc - i;
- if (remaining_args != 2) {
- cerr << "Expected 2 positional arguments but got " << remaining_args << "." << endl;
+ if (remaining_args != 3) {
+ cerr << "Expected 3 positional arguments but got " << remaining_args << "." << endl;
return cpp_usage();
}
+
options->input_file_name_ = argv[i];
+ options->output_header_dir_ = argv[i + 1];
+ options->output_file_name_ = argv[i + 2];
+
if (!EndsWith(options->input_file_name_, ".aidl")) {
- cerr << "Expected .aidl file for input but got "
- << options->input_file_name_ << endl;
+ cerr << "Expected .aidl file for input but got " << options->input_file_name_ << endl;
return cpp_usage();
}
- options->output_base_folder_ = argv[i + 1];
-
- // C++ generation drops 6 files with very similar names based on the name
- // of the input .aidl file. If this file is called foo/Bar.aidl, extract
- // the substring "Bar" and store it in output_base_name_.
- string base_name = options->input_file_name_;
- if (!ReplaceSuffix(".aidl", "", &base_name)) {
- LOG(FATAL) << "Internal aidl error.";
- return cpp_usage();
- }
- auto pos = base_name.rfind(OS_PATH_SEPARATOR);
- if (pos != string::npos) {
- base_name = base_name.substr(pos + 1);
- }
- // If the .aidl file is named something like ITopic.aidl, strip off
- // the 'I' so that we can generate BpTopic and BnTopic.
- if (base_name.length() > 2 &&
- isupper(base_name[0]) &&
- isupper(base_name[1])) {
- base_name = base_name.substr(1);
- }
- options->output_base_name_ = base_name;
-
return options;
}
-string CppOptions::InputFileName() const {
- return input_file_name_;
-}
-
-vector<string> CppOptions::ImportPaths() const {
- return import_paths_;
-}
-
-string CppOptions::ClientCppFileName() const {
- return MakeOutputName("Bp", ".cpp");
-}
-
-string CppOptions::ClientHeaderFileName() const {
- return MakeOutputName("Bp", ".h");
-}
-
-string CppOptions::ServerCppFileName() const {
- return MakeOutputName("Bn", ".cpp");
-}
-
-string CppOptions::ServerHeaderFileName() const {
- return MakeOutputName("Bn", ".h");
-}
-
-string CppOptions::InterfaceCppFileName() const {
- // Note that here we're putting back the 'I' we stripped off earlier.
- return MakeOutputName("I", ".cpp");
-}
-
-string CppOptions::InterfaceHeaderFileName() const {
- return MakeOutputName("I", ".h");
-}
-
-string CppOptions::MakeOutputName(const std::string& prefix,
- const std::string& suffix) const {
- if (output_base_folder_ == "-")
- return "-";
- return output_base_folder_ + OS_PATH_SEPARATOR +
- prefix + output_base_name_ + suffix;
-}
-
bool EndsWith(const string& str, const string& suffix) {
if (str.length() < suffix.length()) {
return false;
diff --git a/options.h b/options.h
index 056ec1d..2783f9e 100644
--- a/options.h
+++ b/options.h
@@ -73,29 +73,20 @@
// Prints the usage statement on failure.
static std::unique_ptr<CppOptions> Parse(int argc, const char* const* argv);
- std::string InputFileName() const;
- std::vector<std::string> ImportPaths() const;
+ std::string InputFileName() const { return input_file_name_; }
+ std::string OutputHeaderDir() const { return output_header_dir_; }
+ std::string OutputCppFilePath() const { return output_file_name_; }
- std::string ClientCppFileName() const;
- std::string ClientHeaderFileName() const;
-
- std::string ServerCppFileName() const;
- std::string ServerHeaderFileName() const;
-
- std::string InterfaceCppFileName() const;
- std::string InterfaceHeaderFileName() const;
-
- // TODO(wiley) Introduce other getters as necessary.
+ std::vector<std::string> ImportPaths() const { return import_paths_; }
+ std::string DependencyFilepath() const { return dep_file_name_; }
private:
CppOptions() = default;
- std::string MakeOutputName(const std::string& prefix,
- const std::string& suffix) const;
std::string input_file_name_;
std::vector<std::string> import_paths_;
- std::string output_base_folder_;
- std::string output_base_name_;
+ std::string output_header_dir_;
+ std::string output_file_name_;
std::string dep_file_name_;
FRIEND_TEST(CppOptionsTests, ParsesCompileCpp);
diff --git a/options_unittest.cpp b/options_unittest.cpp
index 727bff3..568765b 100644
--- a/options_unittest.cpp
+++ b/options_unittest.cpp
@@ -57,24 +57,18 @@
const char kCompileCommandJavaOutput[] = "directory/ITool.java";
const char kCompileDepFile[] = "-doutput.deps";
-const char kCompileCommandOutputDir[] = "output/dir";
+const char kCompileCommandHeaderDir[] = "output/dir";
+const char kCompileCommandCppOutput[] = "some/file.cpp";
const char* kCompileCppCommand[] = {
"aidl-cpp",
kCompileCommandIncludePath,
kCompileDepFile,
kCompileCommandInput,
- kCompileCommandOutputDir,
+ kCompileCommandHeaderDir,
+ kCompileCommandCppOutput,
nullptr,
};
-const char kClientCppPath[] = "output/dir/BpTool.cpp";
-const char kClientHeaderPath[] = "output/dir/BpTool.h";
-const char kServerCppPath[] = "output/dir/BnTool.cpp";
-const char kServerHeaderPath[] = "output/dir/BnTool.h";
-const char kInterfaceCppPath[] = "output/dir/ITool.cpp";
-const char kInterfaceHeaderPath[] = "output/dir/ITool.h";
-
-
template <typename T>
unique_ptr<T> GetOptions(const char* command[]) {
int argc = 0;
@@ -128,13 +122,8 @@
options->import_paths_[0]);
EXPECT_EQ(string{kCompileDepFile}.substr(2), options->dep_file_name_);
EXPECT_EQ(kCompileCommandInput, options->InputFileName());
-
- EXPECT_EQ(kClientCppPath, options->ClientCppFileName());
- EXPECT_EQ(kClientHeaderPath, options->ClientHeaderFileName());
- EXPECT_EQ(kServerCppPath, options->ServerCppFileName());
- EXPECT_EQ(kServerHeaderPath, options->ServerHeaderFileName());
- EXPECT_EQ(kInterfaceCppPath, options->InterfaceCppFileName());
- EXPECT_EQ(kInterfaceHeaderPath, options->InterfaceHeaderFileName());
+ EXPECT_EQ(kCompileCommandHeaderDir, options->OutputHeaderDir());
+ EXPECT_EQ(kCompileCommandCppOutput, options->OutputCppFilePath());
}
TEST(OptionsTests, EndsWith) {