Enable a couple of boring warnings. (#4018)

Just spotted these while looking at warnings that seem to fire on our
code are probably are things we'd fix if we saw them. None of these seem
important FWIW.

Also removes a redundant flag that is part of `-Wall`.

I have a follow-up for the high-value warning I spotted that motivated
me to look at all of this. But it's noisy so kept it as a separate PR.

bazel/cc_toolchains/clang_cc_toolchain_config.bzl

@@ -141,7 +141,8 @@ def _impl(ctx):
                             "-Wself-assign",
                             "-Wimplicit-fallthrough",
                             "-Wctad-maybe-unsupported",
-                            "-Wdelete-non-virtual-dtor",
+                            "-Wextra-semi",
+                            "-Wzero-as-null-pointer-constant",
                             # Don't warn on external code as we can't
                             # necessarily patch it easily. Note that these have
                             # to be initial directories in the `#include` line.

common/struct_reflection.h

@@ -72,7 +72,7 @@ constexpr auto CanListInitialize(...) -> bool {
 // 2) Add more AnyField<T>s until we can't initialize any more.
 template <typename T, bool AnyWorkedSoFar = false, typename... Fields>
 constexpr auto CountFields() -> int {
-  if constexpr (CanListInitialize<T, Fields...>(0)) {
+  if constexpr (CanListInitialize<T, Fields...>(nullptr)) {
     return CountFields<T, true, Fields..., AnyField<T>>();
   } else if constexpr (AnyWorkedSoFar) {
     constexpr int NumFields = sizeof...(Fields) - 1;

explorer/ast/bindings.cpp

@@ -63,7 +63,7 @@ void Bindings::Print(llvm::raw_ostream& out) const {
     out << sep << "`" << *binding << "`: `" << *value << "`";
   }
   out << "]";
-};
+}
 
 auto Bindings::None() -> Nonnull<const Bindings*> {
   static Nonnull<const Bindings*> bindings = new Bindings;

explorer/ast/statement.h

@@ -264,7 +264,7 @@ class VariableDefinition : public Statement {
     return expression_category_;
   }
 
-  auto is_returned() const -> bool { return def_type_ == Returned; };
+  auto is_returned() const -> bool { return def_type_ == Returned; }
 
  private:
   Nonnull<Pattern*> pattern_;

explorer/file_test.cpp

@@ -108,6 +108,6 @@ class ExplorerFileTest : public FileTestBase {
 
 }  // namespace
 
-CARBON_FILE_TEST_FACTORY(ExplorerFileTest);
+CARBON_FILE_TEST_FACTORY(ExplorerFileTest)
 
 }  // namespace Carbon::Testing

explorer/interpreter/heap.h

@@ -30,7 +30,7 @@ class Heap : public HeapAllocationInterface, public Printable<Heap> {
 
   // Constructs an empty Heap.
   explicit Heap(Nonnull<TraceStream*> trace_stream, Nonnull<Arena*> arena)
-      : arena_(arena), trace_stream_(trace_stream){};
+      : arena_(arena), trace_stream_(trace_stream) {}
 
   Heap(const Heap&) = delete;
   auto operator=(const Heap&) -> Heap& = delete;

explorer/syntax/BUILD

@@ -88,6 +88,7 @@ cc_library(
         "-Wno-unused-but-set-variable",
         "-Wno-unused-function",
         "-Wno-writable-strings",
+        "-Wno-zero-as-null-pointer-constant",
     ],
     # Running clang-tidy is slow, and explorer is currently feature frozen, so
     # don't spend time linting it.

explorer/syntax/lex_scan_helper.h

@@ -24,11 +24,11 @@ class StringLexHelper {
   // EOF.
   auto Advance() -> bool;
   // Returns the last scanned char.
-  auto last_char() -> char { return str_.back(); };
+  auto last_char() -> char { return str_.back(); }
   // Returns the scanned string.
-  auto str() -> const std::string& { return str_; };
+  auto str() -> const std::string& { return str_; }
 
-  auto is_eof() -> bool { return is_eof_; };
+  auto is_eof() -> bool { return is_eof_; }
 
  private:
   std::string str_;

language_server/language_server.cpp

@@ -35,7 +35,7 @@ void LanguageServer::OnInitialize(
 
   llvm::json::Object reply{{"capabilities", std::move(capabilities)}};
   cb(reply);
-};
+}
 
 auto LanguageServer::onNotify(llvm::StringRef method, llvm::json::Value value)
     -> bool {

testing/file_test/file_test_base_test.cpp

@@ -162,6 +162,6 @@ class FileTestBaseTest : public FileTestBase {
 
 }  // namespace
 
-CARBON_FILE_TEST_FACTORY(FileTestBaseTest);
+CARBON_FILE_TEST_FACTORY(FileTestBaseTest)
 
 }  // namespace Carbon::Testing

toolchain/check/context.cpp

@@ -821,7 +821,7 @@ class TypeCompleter {
     CARBON_CHECK(value_rep.kind != SemIR::ValueRepr::Unknown)
         << "Complete type should have a value representation";
     return value_rep;
-  };
+  }
 
   auto BuildBuiltinValueRepr(SemIR::TypeId type_id,
                              SemIR::Builtin builtin) const -> SemIR::ValueRepr {

toolchain/install/install_paths.h

@@ -91,7 +91,7 @@ class InstallPaths {
   // in the `StringRef` for inclusion in any user report.
   [[nodiscard]] auto error() const -> std::optional<llvm::StringRef> {
     return error_;
-  };
+  }
 
   // The computed installation prefix. This should correspond to the
   // `prefix_root` directory in Bazel's output, or to some prefix the toolchain

toolchain/lex/lex.cpp

@@ -600,7 +600,7 @@ static constexpr auto MakeDispatchTable() -> DispatchTableT {
   table['\n'] = &DispatchLexVerticalWhitespace;
 
   return table;
-};
+}
 
 static constexpr DispatchTableT DispatchTable = MakeDispatchTable();
 

toolchain/lex/numeric_literal.cpp

@@ -379,7 +379,7 @@ auto NumericLiteral::Parser::CheckDigitSeparatorPlacement(
   if (remaining_digit_separators) {
     diagnose_irregular_digit_separators();
   }
-};
+}
 
 // Check that we don't have a '0' prefix on a non-zero decimal integer.
 auto NumericLiteral::Parser::CheckLeadingZero() -> bool {

toolchain/lex/token_kind.h

@@ -71,23 +71,23 @@ class TokenKind : public CARBON_ENUM_BASE(TokenKind) {
 
   // Test whether this kind of token is a one-character symbol whose character
   // is not part of any other symbol.
-  auto is_one_char_symbol() const -> bool { return IsOneCharSymbol[AsInt()]; };
+  auto is_one_char_symbol() const -> bool { return IsOneCharSymbol[AsInt()]; }
 
   // Test whether this kind of token is a keyword.
-  auto is_keyword() const -> bool { return IsKeyword[AsInt()]; };
+  auto is_keyword() const -> bool { return IsKeyword[AsInt()]; }
 
   // Test whether this kind of token is a sized type literal.
   auto is_sized_type_literal() const -> bool {
     return *this == TokenKind::IntTypeLiteral ||
            *this == TokenKind::UnsignedIntTypeLiteral ||
            *this == TokenKind::FloatTypeLiteral;
-  };
+  }
 
   // If this token kind has a fixed spelling when in source code, returns it.
   // Otherwise returns an empty string.
   auto fixed_spelling() const -> llvm::StringLiteral {
     return FixedSpelling[AsInt()];
-  };
+  }
 
   // Get the expected number of parse tree nodes that will be created for this
   // token.

toolchain/parse/context.h

@@ -56,7 +56,7 @@ class Context {
     auto Print(llvm::raw_ostream& output) const -> void {
       output << state << " @" << token << " subtree_start=" << subtree_start
              << " has_error=" << has_error;
-    };
+    }
 
     // The state.
     State state;

toolchain/testing/file_test.cpp

@@ -161,7 +161,7 @@ class ToolchainFileTest : public FileTestBase {
 
 }  // namespace
 
-CARBON_FILE_TEST_FACTORY(ToolchainFileTest);
+CARBON_FILE_TEST_FACTORY(ToolchainFileTest)
 
 }  // namespace Carbon::Testing