Enable -Wextra-semi in release builds that don't set dcheck_always_on=true.

Most of these are macros that expand to nothing when dchecks are off, but
to a declaration when dchecks are on. They are meant to be used to declare
member variables.

Since clang-format gets very confused on class member declarations that
don't end in a semicolon and since we require clang-formatting,
we need some way to still permit a semicolon after these macros even
with dchecks off. To that end, add a dummy static_assert() at the end
of these macros.

Alternatively, we could surround all uses of these macros with explicit
DCHECK_IS_ON() checks.

Depends on https://skia-review.googlesource.com/c/skia/+/196421

TBR=dalecurtis

Bug: 926235,936856
Change-Id: I66ab08f383b2f27dc6a7617f67f33fa66ddfa00c
Reviewed-on: https://chromium-review.googlesource.com/c/1495041
Commit-Queue: Nico Weber <thakis@chromium.org>
Reviewed-by: Gabriel Charette <gab@chromium.org>
Cr-Commit-Position: refs/heads/master@{#636838}

base/sequence_checker.h

@@ -54,7 +54,7 @@
   DCHECK((name).CalledOnValidSequence())
 #define DETACH_FROM_SEQUENCE(name) (name).DetachFromSequence()
 #else  // DCHECK_IS_ON()
-#define SEQUENCE_CHECKER(name)
+#define SEQUENCE_CHECKER(name) static_assert(true, "")
 #define DCHECK_CALLED_ON_VALID_SEQUENCE(name) EAT_STREAM_PARAMETERS
 #define DETACH_FROM_SEQUENCE(name)
 #endif  // DCHECK_IS_ON()

base/threading/thread_checker.h

@@ -65,7 +65,7 @@
 #define DCHECK_CALLED_ON_VALID_THREAD(name) DCHECK((name).CalledOnValidThread())
 #define DETACH_FROM_THREAD(name) (name).DetachFromThread()
 #else  // DCHECK_IS_ON()
-#define THREAD_CHECKER(name)
+#define THREAD_CHECKER(name) static_assert(true, "")
 #define DCHECK_CALLED_ON_VALID_THREAD(name) EAT_STREAM_PARAMETERS
 #define DETACH_FROM_THREAD(name)
 #endif  // DCHECK_IS_ON()

base/threading/thread_restrictions.h

@@ -277,8 +277,13 @@ class Thread;
 #define EMPTY_BODY_IF_DCHECK_IS_OFF
 #else
 #define INLINE_IF_DCHECK_IS_OFF inline
+
+// The static_assert() eats follow-on semicolons. `= default` would work
+// too, but it makes clang realize that all the Scoped classes are no-ops in
+// non-dcheck builds and it starts emitting many -Wunused-variable warnings.
 #define EMPTY_BODY_IF_DCHECK_IS_OFF \
-  {}
+  {}                                \
+  static_assert(true, "")
 #endif
 
 namespace internal {
@@ -596,6 +601,9 @@ class BASE_EXPORT ThreadRestrictions {
   DISALLOW_IMPLICIT_CONSTRUCTORS(ThreadRestrictions);
 };
 
+#undef INLINE_IF_DCHECK_IS_OFF
+#undef EMPTY_BODY_IF_DCHECK_IS_OFF
+
 }  // namespace base
 
 #endif  // BASE_THREADING_THREAD_RESTRICTIONS_H_

build/config/compiler/BUILD.gn

@@ -1591,8 +1591,7 @@ config("chromium_code") {
     # TODO(thakis): Enable this for more platforms, https://crbug.com/926235
     # Fuchsia: https://crbug.com/935588
     # iOS: https://crbug.com/936211
-    has_dchecks = is_debug || dcheck_always_on
-    if (has_dchecks && !is_fuchsia && !is_ios) {
+    if (!is_fuchsia && !is_ios) {
       cflags += [ "-Wextra-semi" ]
     }
   }

media/base/reentrancy_checker.h

@@ -40,7 +40,7 @@
 #define REENTRANCY_CHECKER(name) ::base::Lock name
 #define NON_REENTRANT_SCOPE(name) ::media::NonReentrantScope name##scope(name)
 #else  // DCHECK_IS_ON()
-#define REENTRANCY_CHECKER(name)
+#define REENTRANCY_CHECKER(name) static_assert(true, "")
 #define NON_REENTRANT_SCOPE(name)
 #endif  // DCHECK_IS_ON()