Enable clang_tidy for AIDL-generated C++ libraries
tidy is enabled for the cpp and ndk backends. The AIDL compiler is
modified to emit code that doesn't break the tidy checks.
Bug: 162909698
Test: m checkbuild
Test: run aidl_unittests
Change-Id: I0df916e5df15dba85b75603a544d4666594526ea
diff --git a/aidl_to_cpp_common.cpp b/aidl_to_cpp_common.cpp
index 5e0ef1f..2d69a4a 100644
--- a/aidl_to_cpp_common.cpp
+++ b/aidl_to_cpp_common.cpp
@@ -648,7 +648,8 @@
{name}& operator=(const {name}&) = default;
{name}& operator=({name}&&) = default;
-template <typename _Tp, std::enable_if_t<_not_self<_Tp>, int> = 0>
+template <typename _Tp, typename = std::enable_if_t<_not_self<_Tp>>>
+// NOLINTNEXTLINE(google-explicit-constructor)
constexpr {name}(_Tp&& _arg)
: _value(std::forward<_Tp>(_arg)) {{}}
@@ -714,8 +715,21 @@
for (const auto& variable : decl.GetFields()) {
out << fmt::format("case {}: {{\n", variable->GetName());
out.Indent();
- read_var(value, variable->GetType());
+ const auto& type = variable->GetType();
+ read_var(value, type);
+ out << fmt::format("if constexpr (std::is_trivially_copyable_v<{}>) {{\n",
+ name_of(type, typenames));
+ out.Indent();
+ out << fmt::format("set<{}>({});\n", variable->GetName(), value);
+ out.Dedent();
+ out << "} else {\n";
+ out.Indent();
+ // Even when the `if constexpr` is false, the compiler runs the tidy check for the
+ // next line, which doesn't make sense. Silence the check for the unreachable code.
+ out << "// NOLINTNEXTLINE(performance-move-const-arg)\n";
out << fmt::format("set<{}>(std::move({}));\n", variable->GetName(), value);
+ out.Dedent();
+ out << "}\n";
out << fmt::format("return {}; }}\n", ctx.status_ok);
out.Dedent();
}
diff --git a/aidl_unittest.cpp b/aidl_unittest.cpp
index c51f655..3eb9eb9 100644
--- a/aidl_unittest.cpp
+++ b/aidl_unittest.cpp
@@ -3037,7 +3037,8 @@
Foo& operator=(const Foo&) = default;
Foo& operator=(Foo&&) = default;
- template <typename _Tp, std::enable_if_t<_not_self<_Tp>, int> = 0>
+ template <typename _Tp, typename = std::enable_if_t<_not_self<_Tp>>>
+ // NOLINTNEXTLINE(google-explicit-constructor)
constexpr Foo(_Tp&& _arg)
: _value(std::forward<_Tp>(_arg)) {}
@@ -3121,17 +3122,32 @@
case ns: {
::std::vector<int32_t> _aidl_value;
if ((_aidl_ret_status = _aidl_parcel->readInt32Vector(&_aidl_value)) != ::android::OK) return _aidl_ret_status;
- set<ns>(std::move(_aidl_value));
+ if constexpr (std::is_trivially_copyable_v<::std::vector<int32_t>>) {
+ set<ns>(_aidl_value);
+ } else {
+ // NOLINTNEXTLINE(performance-move-const-arg)
+ set<ns>(std::move(_aidl_value));
+ }
return ::android::OK; }
case e: {
::a::ByteEnum _aidl_value;
if ((_aidl_ret_status = _aidl_parcel->readByte(reinterpret_cast<int8_t *>(&_aidl_value))) != ::android::OK) return _aidl_ret_status;
- set<e>(std::move(_aidl_value));
+ if constexpr (std::is_trivially_copyable_v<::a::ByteEnum>) {
+ set<e>(_aidl_value);
+ } else {
+ // NOLINTNEXTLINE(performance-move-const-arg)
+ set<e>(std::move(_aidl_value));
+ }
return ::android::OK; }
case pfd: {
::android::os::ParcelFileDescriptor _aidl_value;
if ((_aidl_ret_status = _aidl_parcel->readParcelable(&_aidl_value)) != ::android::OK) return _aidl_ret_status;
- set<pfd>(std::move(_aidl_value));
+ if constexpr (std::is_trivially_copyable_v<::android::os::ParcelFileDescriptor>) {
+ set<pfd>(_aidl_value);
+ } else {
+ // NOLINTNEXTLINE(performance-move-const-arg)
+ set<pfd>(std::move(_aidl_value));
+ }
return ::android::OK; }
}
return ::android::BAD_VALUE;
@@ -3192,7 +3208,8 @@
Foo& operator=(const Foo&) = default;
Foo& operator=(Foo&&) = default;
- template <typename _Tp, std::enable_if_t<_not_self<_Tp>, int> = 0>
+ template <typename _Tp, typename = std::enable_if_t<_not_self<_Tp>>>
+ // NOLINTNEXTLINE(google-explicit-constructor)
constexpr Foo(_Tp&& _arg)
: _value(std::forward<_Tp>(_arg)) {}
@@ -3277,17 +3294,32 @@
case ns: {
std::vector<int32_t> _aidl_value;
if ((_aidl_ret_status = ::ndk::AParcel_readVector(_parcel, &_aidl_value)) != STATUS_OK) return _aidl_ret_status;
- set<ns>(std::move(_aidl_value));
+ if constexpr (std::is_trivially_copyable_v<std::vector<int32_t>>) {
+ set<ns>(_aidl_value);
+ } else {
+ // NOLINTNEXTLINE(performance-move-const-arg)
+ set<ns>(std::move(_aidl_value));
+ }
return STATUS_OK; }
case e: {
::aidl::a::ByteEnum _aidl_value;
if ((_aidl_ret_status = AParcel_readByte(_parcel, reinterpret_cast<int8_t*>(&_aidl_value))) != STATUS_OK) return _aidl_ret_status;
- set<e>(std::move(_aidl_value));
+ if constexpr (std::is_trivially_copyable_v<::aidl::a::ByteEnum>) {
+ set<e>(_aidl_value);
+ } else {
+ // NOLINTNEXTLINE(performance-move-const-arg)
+ set<e>(std::move(_aidl_value));
+ }
return STATUS_OK; }
case pfd: {
::ndk::ScopedFileDescriptor _aidl_value;
if ((_aidl_ret_status = ::ndk::AParcel_readRequiredParcelFileDescriptor(_parcel, &_aidl_value)) != STATUS_OK) return _aidl_ret_status;
- set<pfd>(std::move(_aidl_value));
+ if constexpr (std::is_trivially_copyable_v<::ndk::ScopedFileDescriptor>) {
+ set<pfd>(_aidl_value);
+ } else {
+ // NOLINTNEXTLINE(performance-move-const-arg)
+ set<pfd>(std::move(_aidl_value));
+ }
return STATUS_OK; }
}
return STATUS_BAD_VALUE;
diff --git a/build/aidl_interface.go b/build/aidl_interface.go
index 4aaeae7..a3f8f57 100644
--- a/build/aidl_interface.go
+++ b/build/aidl_interface.go
@@ -1615,6 +1615,10 @@
Min_sdk_version: minSdkVersion,
UseApexNameMacro: true,
Target: targetProperties{Darwin: perTargetProperties{Enabled: proptools.BoolPtr(false)}},
+ Tidy: proptools.BoolPtr(true),
+ // Do the tidy check only for the generated headers
+ Tidy_flags: []string{"--header-filter=" + android.PathForOutput(mctx).String() + ".*"},
+ Tidy_checks_as_errors: []string{"*"},
}, &i.properties.VndkProperties, &commonProperties.VndkProperties, &overrideVndkProperties)
return cppModuleGen
diff --git a/build/properties.go b/build/properties.go
index 69e773e..0b61b63 100644
--- a/build/properties.go
+++ b/build/properties.go
@@ -60,6 +60,9 @@
Min_sdk_version *string
UseApexNameMacro bool
Target targetProperties
+ Tidy *bool
+ Tidy_flags []string
+ Tidy_checks_as_errors []string
}
type javaProperties struct {
diff --git a/generate_ndk.cpp b/generate_ndk.cpp
index 0fb8b53..7cbdb5b 100644
--- a/generate_ndk.cpp
+++ b/generate_ndk.cpp
@@ -569,7 +569,7 @@
out << "class ScopedTrace {\n";
out.Indent();
out << "public:\n"
- << "inline ScopedTrace(const char* name) {\n"
+ << "inline explicit ScopedTrace(const char* name) {\n"
<< "ATrace_beginSection(name);\n"
<< "}\n"
<< "inline ~ScopedTrace() {\n"
@@ -722,7 +722,7 @@
out << "}\n";
// defintion for static member setDefaultImpl
- out << "bool " << clazz << "::setDefaultImpl(std::shared_ptr<" << clazz << "> impl) {\n";
+ out << "bool " << clazz << "::setDefaultImpl(const std::shared_ptr<" << clazz << ">& impl) {\n";
out.Indent();
out << "// Only one user of this interface can use this function\n";
out << "// at a time. This is a heuristic to detect if two different\n";
@@ -819,7 +819,7 @@
<< ClassName(defined_type, ClassNames::INTERFACE) << "> {\n";
out << "public:\n";
out.Indent();
- out << clazz << "(const ::ndk::SpAIBinder& binder);\n";
+ out << "explicit " << clazz << "(const ::ndk::SpAIBinder& binder);\n";
out << "virtual ~" << clazz << "();\n";
out << "\n";
for (const auto& method : defined_type.GetMethods()) {
@@ -927,7 +927,7 @@
out << "static binder_status_t readFromParcel(const AParcel* parcel, std::shared_ptr<" << clazz
<< ">* instance);";
out << "\n";
- out << "static bool setDefaultImpl(std::shared_ptr<" << clazz << "> impl);";
+ out << "static bool setDefaultImpl(const std::shared_ptr<" << clazz << ">& impl);";
out << "\n";
out << "static const std::shared_ptr<" << clazz << ">& getDefaultImpl();";
out << "\n";