From dd0890619aff10f1d59fc9dacfe2ef4405e963d3 Mon Sep 17 00:00:00 2001 From: Chandler Carruth Date: Mon, 3 Jun 2024 07:02:26 +0200 Subject: [PATCH] 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 | 3 ++- common/struct_reflection.h | 2 +- explorer/ast/bindings.cpp | 2 +- explorer/ast/statement.h | 2 +- explorer/file_test.cpp | 2 +- explorer/interpreter/heap.h | 2 +- explorer/syntax/BUILD | 1 + explorer/syntax/lex_scan_helper.h | 6 +++--- language_server/language_server.cpp | 2 +- testing/file_test/file_test_base_test.cpp | 2 +- toolchain/check/context.cpp | 2 +- toolchain/install/install_paths.h | 2 +- toolchain/lex/lex.cpp | 2 +- toolchain/lex/numeric_literal.cpp | 2 +- toolchain/lex/token_kind.h | 8 ++++---- toolchain/parse/context.h | 2 +- toolchain/testing/file_test.cpp | 2 +- 17 files changed, 23 insertions(+), 21 deletions(-) diff --git a/bazel/cc_toolchains/clang_cc_toolchain_config.bzl b/bazel/cc_toolchains/clang_cc_toolchain_config.bzl index ec7e2220206f..e971a24d6f63 100644 --- a/bazel/cc_toolchains/clang_cc_toolchain_config.bzl +++ b/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. diff --git a/common/struct_reflection.h b/common/struct_reflection.h index 9bb113819817..9221a939dc2d 100644 --- a/common/struct_reflection.h +++ b/common/struct_reflection.h @@ -72,7 +72,7 @@ constexpr auto CanListInitialize(...) -> bool { // 2) Add more AnyFields until we can't initialize any more. template constexpr auto CountFields() -> int { - if constexpr (CanListInitialize(0)) { + if constexpr (CanListInitialize(nullptr)) { return CountFields>(); } else if constexpr (AnyWorkedSoFar) { constexpr int NumFields = sizeof...(Fields) - 1; diff --git a/explorer/ast/bindings.cpp b/explorer/ast/bindings.cpp index 27ce42a9915e..827b7a3e1223 100644 --- a/explorer/ast/bindings.cpp +++ b/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 { static Nonnull bindings = new Bindings; diff --git a/explorer/ast/statement.h b/explorer/ast/statement.h index 2b7782ab3d26..e90504380205 100644 --- a/explorer/ast/statement.h +++ b/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_; diff --git a/explorer/file_test.cpp b/explorer/file_test.cpp index 026ab47b5d65..89f60d63c03a 100644 --- a/explorer/file_test.cpp +++ b/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 diff --git a/explorer/interpreter/heap.h b/explorer/interpreter/heap.h index d1e25c540d70..15aee93ca97f 100644 --- a/explorer/interpreter/heap.h +++ b/explorer/interpreter/heap.h @@ -30,7 +30,7 @@ class Heap : public HeapAllocationInterface, public Printable { // Constructs an empty Heap. explicit Heap(Nonnull trace_stream, Nonnull arena) - : arena_(arena), trace_stream_(trace_stream){}; + : arena_(arena), trace_stream_(trace_stream) {} Heap(const Heap&) = delete; auto operator=(const Heap&) -> Heap& = delete; diff --git a/explorer/syntax/BUILD b/explorer/syntax/BUILD index f71c69654cea..226d0356d257 100644 --- a/explorer/syntax/BUILD +++ b/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. diff --git a/explorer/syntax/lex_scan_helper.h b/explorer/syntax/lex_scan_helper.h index 8c7b4f597089..3f707cd6d2f4 100644 --- a/explorer/syntax/lex_scan_helper.h +++ b/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_; diff --git a/language_server/language_server.cpp b/language_server/language_server.cpp index fb35ef5dc332..9afc6f745a59 100644 --- a/language_server/language_server.cpp +++ b/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 { diff --git a/testing/file_test/file_test_base_test.cpp b/testing/file_test/file_test_base_test.cpp index e395713bf784..b80ac8e6ca8c 100644 --- a/testing/file_test/file_test_base_test.cpp +++ b/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 diff --git a/toolchain/check/context.cpp b/toolchain/check/context.cpp index 784184bbbd45..7eca3f2e05e8 100644 --- a/toolchain/check/context.cpp +++ b/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 { diff --git a/toolchain/install/install_paths.h b/toolchain/install/install_paths.h index 3816f65a7806..9e3ade36e21f 100644 --- a/toolchain/install/install_paths.h +++ b/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 { return error_; - }; + } // The computed installation prefix. This should correspond to the // `prefix_root` directory in Bazel's output, or to some prefix the toolchain diff --git a/toolchain/lex/lex.cpp b/toolchain/lex/lex.cpp index 231bc32d43c6..ef7722a8877b 100644 --- a/toolchain/lex/lex.cpp +++ b/toolchain/lex/lex.cpp @@ -600,7 +600,7 @@ static constexpr auto MakeDispatchTable() -> DispatchTableT { table['\n'] = &DispatchLexVerticalWhitespace; return table; -}; +} static constexpr DispatchTableT DispatchTable = MakeDispatchTable(); diff --git a/toolchain/lex/numeric_literal.cpp b/toolchain/lex/numeric_literal.cpp index 1be9ab820a6f..fd5777a4053d 100644 --- a/toolchain/lex/numeric_literal.cpp +++ b/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 { diff --git a/toolchain/lex/token_kind.h b/toolchain/lex/token_kind.h index 6db40b0a34a8..3b41fc2a408a 100644 --- a/toolchain/lex/token_kind.h +++ b/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. diff --git a/toolchain/parse/context.h b/toolchain/parse/context.h index fa666ba1bcc7..09a147b0ff38 100644 --- a/toolchain/parse/context.h +++ b/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; diff --git a/toolchain/testing/file_test.cpp b/toolchain/testing/file_test.cpp index 0b5daa53a6d6..fc2f119a01d9 100644 --- a/toolchain/testing/file_test.cpp +++ b/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