ART: Use clang-tidy to warn on RAII issue
Remove the macro hack to detect likely-incorrect usage of RAII wrappers.
Instead make the clang-tidy pattern bugprone-unused-raii fatal.
Test: mmma art
Change-Id: I9d0eb1c5c3f469b2907111af9d38d947b36c4878
diff --git a/build/Android.bp b/build/Android.bp
index 3a1d583..b7d2cbc 100644
--- a/build/Android.bp
+++ b/build/Android.bp
@@ -17,6 +17,27 @@
pluginFor: ["soong_build"],
}
+art_clang_tidy_errors = [
+ // Protect scoped things like MutexLock.
+ "bugprone-unused-raii",
+]
+// Should be: strings.Join(art_clang_tidy_errors, ",").
+art_clang_tidy_errors_str = "bugprone-unused-raii"
+
+art_clang_tidy_disabled = [
+ "-google-default-arguments",
+ // We have local stores that are only used for debug checks.
+ "-clang-analyzer-deadcode.DeadStores",
+ // We are OK with some static globals and that they can, in theory, throw.
+ "-cert-err58-cpp",
+ // We have lots of C-style variadic functions, and are OK with them. JNI ensures
+ // that working around this warning would be extra-painful.
+ "-cert-dcl50-cpp",
+ // No exceptions.
+ "-misc-noexcept-move-constructor",
+ "-performance-noexcept-move-constructor",
+]
+
art_global_defaults {
// Additional flags are computed by art.go
@@ -130,18 +151,7 @@
"external/vixl/src",
],
- tidy_checks: [
- "-google-default-arguments",
- // We have local stores that are only used for debug checks.
- "-clang-analyzer-deadcode.DeadStores",
- // We are OK with some static globals and that they can, in theory, throw.
- "-cert-err58-cpp",
- // We have lots of C-style variadic functions, and are OK with them. JNI ensures
- // that working around this warning would be extra-painful.
- "-cert-dcl50-cpp",
- // No exceptions.
- "-misc-noexcept-move-constructor",
- ],
+ tidy_checks: art_clang_tidy_errors + art_clang_tidy_disabled,
tidy_flags: [
// The static analyzer treats DCHECK as always enabled; we sometimes get
@@ -151,6 +161,8 @@
// void foo() { CHECK(kIsFooEnabled); /* do foo... */ }
// not being marked noreturn if kIsFooEnabled is false.
"-extra-arg=-Wno-missing-noreturn",
+ // Use art_clang_tidy_errors for build errors.
+ "-warnings-as-errors=" + art_clang_tidy_errors_str,
],
}
diff --git a/runtime/base/mutex-inl.h b/runtime/base/mutex-inl.h
index dfa14b9..51ca274 100644
--- a/runtime/base/mutex-inl.h
+++ b/runtime/base/mutex-inl.h
@@ -290,10 +290,6 @@
mu_.SharedUnlock(self_);
}
-// Catch bug where variable name is omitted. "ReaderMutexLock (lock);" instead of
-// "ReaderMutexLock mu(lock)".
-#define ReaderMutexLock(x) static_assert(0, "ReaderMutexLock declaration missing variable name")
-
} // namespace art
#endif // ART_RUNTIME_BASE_MUTEX_INL_H_
diff --git a/runtime/base/mutex.h b/runtime/base/mutex.h
index 602d183..ee47e7c 100644
--- a/runtime/base/mutex.h
+++ b/runtime/base/mutex.h
@@ -525,8 +525,6 @@
Mutex& mu_;
DISALLOW_COPY_AND_ASSIGN(MutexLock);
};
-// Catch bug where variable name is omitted. "MutexLock (lock);" instead of "MutexLock mu(lock)".
-#define MutexLock(x) static_assert(0, "MutexLock declaration missing variable name")
// Scoped locker/unlocker for a ReaderWriterMutex that acquires read access to mu upon
// construction and releases it upon destruction.
@@ -560,9 +558,6 @@
ReaderWriterMutex& mu_;
DISALLOW_COPY_AND_ASSIGN(WriterMutexLock);
};
-// Catch bug where variable name is omitted. "WriterMutexLock (lock);" instead of
-// "WriterMutexLock mu(lock)".
-#define WriterMutexLock(x) static_assert(0, "WriterMutexLock declaration missing variable name")
// For StartNoThreadSuspension and EndNoThreadSuspension.
class CAPABILITY("role") Role {
diff --git a/runtime/thread.cc b/runtime/thread.cc
index 3c5569f..1a078d5 100644
--- a/runtime/thread.cc
+++ b/runtime/thread.cc
@@ -1579,7 +1579,7 @@
VLOG(threads) << this << " self-suspending";
// Make thread appear suspended to other threads, release mutator_lock_.
// Transition to suspended and back to runnable, re-acquire share on mutator_lock_.
- ScopedThreadSuspension(this, kSuspended);
+ ScopedThreadSuspension(this, kSuspended); // NOLINT
VLOG(threads) << this << " self-reviving";
}
