From d4a2d435b82a1ceffd4e68120acc1690cbad47f9 Mon Sep 17 00:00:00 2001 From: Chandler Carruth Date: Tue, 8 Dec 2020 14:43:37 -0800 Subject: [PATCH] Enable most relevant clang-tidy checks and fix uncovered issues. (#220) Most of these were fixed automatically (including things like adding `[[nodiscard]]` and such). A number of others required manual edits. I think all of them were pretty nice improvements. There were a few places where the issues really stem from external constraints and I've disabled the checks: GoogleTest macros or the specific LibFuzzer entry points. The only other places I disabled are the implicit conversions to a private `enum` in the classes wrapping those `enum`s. These implicit conversions are necessarily implicit to serve their only purpose: enabling their use in `switch` statements and `case` labels. When these were highlighted, it showed that one of these was actually converting to an *`int`*. I've switched that to use the private `enum` instead as doing so is important to enable warnings on non-covering `switch` statements over than `enum`. And indeed, there is a `switch` that was was implicitly relying on falling through in this way, so I've added the explicit documentation of the intentional pattern to address that warning. Sorry this is so large, all of this somewhat fell out of enabling the `clang-tidy` checks. If it is too difficult to review as lump, I can work on breaking it apart as needed. Just let me know. --- .clang-tidy | 8 +- diagnostics/diagnostic_emitter.h | 2 +- diagnostics/diagnostic_emitter_test.cpp | 3 +- lexer/token_kind.h | 39 +++++---- lexer/token_kind_test.cpp | 2 +- lexer/tokenized_buffer.cpp | 34 +++++--- lexer/tokenized_buffer.h | 106 ++++++++++++++---------- lexer/tokenized_buffer_fuzzer.cpp | 7 +- lexer/tokenized_buffer_test_helpers.h | 6 +- parser/parse_node_kind.h | 11 ++- parser/parse_test_helpers.h | 9 +- parser/parse_tree_fuzzer.cpp | 3 +- parser/parser_impl.cpp | 7 +- source/source_buffer.cpp | 7 +- source/source_buffer.h | 8 +- 15 files changed, 155 insertions(+), 97 deletions(-) diff --git a/.clang-tidy b/.clang-tidy index c4f0d253c927..b078ecc2d414 100644 --- a/.clang-tidy +++ b/.clang-tidy @@ -3,7 +3,13 @@ # SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception --- -Checks: '-*,readability-braces-around-statements,readability-identifier-naming' +Checks: + -*, bugprone-*, google-*, -google-readability-todo, + misc-definitions-in-headers, misc-misplaced-const, misc-redundant-expression, + misc-static-assert, misc-unconventional-assign-operator, + misc-uniqueptr-reset-release, misc-unused-*, modernize-*, + -modernize-avoid-c-arrays, performance-*, + readability-braces-around-statements, readability-identifier-naming WarningsAsErrors: true CheckOptions: - { key: readability-identifier-naming.ClassCase, value: CamelCase } diff --git a/diagnostics/diagnostic_emitter.h b/diagnostics/diagnostic_emitter.h index d5f7891e7235..7f04380cee0b 100644 --- a/diagnostics/diagnostic_emitter.h +++ b/diagnostics/diagnostic_emitter.h @@ -34,7 +34,7 @@ class DiagnosticEmitter { explicit DiagnosticEmitter(Callback callback) : callback_(std::move(callback)) {} - ~DiagnosticEmitter() {} + ~DiagnosticEmitter() = default; // Emits an error unconditionally. `F` is guaranteed to be called. template diff --git a/diagnostics/diagnostic_emitter_test.cpp b/diagnostics/diagnostic_emitter_test.cpp index 668546d514c5..0c230fca9727 100644 --- a/diagnostics/diagnostic_emitter_test.cpp +++ b/diagnostics/diagnostic_emitter_test.cpp @@ -12,7 +12,8 @@ namespace Carbon { namespace { -using namespace ::testing; +using ::testing::ElementsAre; +using ::testing::Eq; struct FakeDiagnostic { static constexpr llvm::StringLiteral ShortName = "fake-diagnostic"; diff --git a/lexer/token_kind.h b/lexer/token_kind.h index 69dfbe1af1c5..86070ce35c5c 100644 --- a/lexer/token_kind.h +++ b/lexer/token_kind.h @@ -5,8 +5,7 @@ #ifndef LEXER_TOKEN_KIND_H_ #define LEXER_TOKEN_KIND_H_ -#include - +#include #include #include "llvm/ADT/StringRef.h" @@ -20,8 +19,12 @@ class TokenKind { }; public: -#define CARBON_TOKEN(TokenName) \ - static constexpr auto TokenName()->TokenKind { return KindEnum::TokenName; } + // The formatting for this macro is weird due to a `clang-format` bug. See + // https://bugs.llvm.org/show_bug.cgi?id=48320 for details. +#define CARBON_TOKEN(TokenName) \ + static constexpr auto TokenName()->TokenKind { \ + return TokenKind(KindEnum::TokenName); \ + } #include "lexer/token_registry.def" // The default constructor is deleted as objects of this type should always be @@ -36,7 +39,7 @@ class TokenKind { } // Get a friendly name for the token for logging or debugging. - auto Name() const -> llvm::StringRef; + [[nodiscard]] auto Name() const -> llvm::StringRef; // Test whether this kind of token is a simple symbol sequence (punctuation, // not letters) that appears directly in the source text and can be @@ -44,41 +47,43 @@ class TokenKind { // inside of other tokens, outside of the contents of other tokens they // don't require any specific characters before or after to distinguish them // in the source. Returns false otherwise. - auto IsSymbol() const -> bool; + [[nodiscard]] auto IsSymbol() const -> bool; // Test whether this kind of token is a grouping symbol (part of an opening // and closing pair that must always be matched in the token stream). - auto IsGroupingSymbol() const -> bool; + [[nodiscard]] auto IsGroupingSymbol() const -> bool; // Test whether this kind of token is an opening symbol for a group. - auto IsOpeningSymbol() const -> bool; + [[nodiscard]] auto IsOpeningSymbol() const -> bool; // Returns the associated closing symbol for an opening symbol. // // The token kind must be an opening symbol. - auto GetClosingSymbol() const -> TokenKind; + [[nodiscard]] auto GetClosingSymbol() const -> TokenKind; // Test whether this kind of token is an closing symbol for a group. - auto IsClosingSymbol() const -> bool; + [[nodiscard]] auto IsClosingSymbol() const -> bool; // Returns the associated opening symbol for a closing symbol. // // The token kind must be an closing symbol. - auto GetOpeningSymbol() const -> TokenKind; + [[nodiscard]] auto GetOpeningSymbol() const -> TokenKind; // Test whether this kind of token is a keyword. - auto IsKeyword() const -> bool; + [[nodiscard]] auto IsKeyword() const -> bool; // If this token kind has a fixed spelling when in source code, returns it. // Otherwise returns an empty string. - auto GetFixedSpelling() const -> llvm::StringRef; + [[nodiscard]] auto GetFixedSpelling() const -> llvm::StringRef; - // Enable implicit conversion to an int, including in a `constexpr` context, - // to enable usage in `switch` and `case`. - constexpr operator int() const { return static_cast(kind_value); } + // Enable conversion to our private enum, including in a `constexpr` context, + // to enable usage in `switch` and `case`. The enum remains private and + // nothing else should be using this. + // NOLINTNEXTLINE(google-explicit-constructor) + constexpr operator KindEnum() const { return kind_value; } private: - constexpr TokenKind(KindEnum kind_value) : kind_value(kind_value) {} + constexpr explicit TokenKind(KindEnum kind_value) : kind_value(kind_value) {} KindEnum kind_value; }; diff --git a/lexer/token_kind_test.cpp b/lexer/token_kind_test.cpp index c803a0441326..a11b1b81f212 100644 --- a/lexer/token_kind_test.cpp +++ b/lexer/token_kind_test.cpp @@ -4,7 +4,7 @@ #include "lexer/token_kind.h" -#include +#include #include "gmock/gmock.h" #include "gtest/gtest.h" diff --git a/lexer/tokenized_buffer.cpp b/lexer/tokenized_buffer.cpp index 86e7241fb256..131eebf93f05 100644 --- a/lexer/tokenized_buffer.cpp +++ b/lexer/tokenized_buffer.cpp @@ -322,9 +322,7 @@ class TokenizedBuffer::Lexer { } switch (c) { case '_': - return false; case '\t': - return false; case '\n': return false; } @@ -487,16 +485,28 @@ auto TokenizedBuffer::PrintWidths::Widen(const PrintWidths& widths) -> void { indent = std::max(widths.indent, indent); } +// Compute the printed width of a number. When numbers are printed in decimal, +// the number of digits needed is is one more than the log-base-10 of the value. +// We handle a value of `zero` explicitly. +// +// This routine requires its argument to be *non-negative*. +static auto ComputeDecimalPrintedWidth(int number) -> int { + assert(number >= 0 && "Negative numbers are not supported."); + if (number == 0) { + return 1; + } + + return static_cast(std::log10(number)) + 1; +} + auto TokenizedBuffer::GetTokenPrintWidths(Token token) const -> PrintWidths { PrintWidths widths = {}; - // Compute the printed width of the various token information. When numbers - // here are printed in decimal, the number of digits needed is is one more - // than the log-base-10 of the value. - widths.index = std::log10(token_infos.size()) + 1; + widths.index = ComputeDecimalPrintedWidth(token_infos.size()); widths.kind = GetKind(token).Name().size(); - widths.line = std::log10(GetLineNumber(token)) + 1; - widths.column = std::log10(GetColumnNumber(token)) + 1; - widths.indent = std::log10(GetIndentColumnNumber(GetLine(token))) + 1; + widths.line = ComputeDecimalPrintedWidth(GetLineNumber(token)); + widths.column = ComputeDecimalPrintedWidth(GetColumnNumber(token)); + widths.indent = + ComputeDecimalPrintedWidth(GetIndentColumnNumber(GetLine(token))); return widths; } @@ -506,7 +516,7 @@ auto TokenizedBuffer::Print(llvm::raw_ostream& output_stream) const -> void { } PrintWidths widths = {}; - widths.index = std::log10(token_infos.size()) + 1; + widths.index = ComputeDecimalPrintedWidth((token_infos.size())); for (Token token : Tokens()) { widths.Widen(GetTokenPrintWidths(token)); } @@ -570,7 +580,7 @@ auto TokenizedBuffer::GetLineInfo(Line line) const -> const LineInfo& { auto TokenizedBuffer::AddLine(LineInfo info) -> Line { line_infos.push_back(info); - return Line(line_infos.size() - 1); + return Line(static_cast(line_infos.size()) - 1); } auto TokenizedBuffer::GetTokenInfo(Token token) -> TokenInfo& { @@ -583,7 +593,7 @@ auto TokenizedBuffer::GetTokenInfo(Token token) const -> const TokenInfo& { auto TokenizedBuffer::AddToken(TokenInfo info) -> Token { token_infos.push_back(info); - return Token(token_infos.size() - 1); + return Token(static_cast(token_infos.size()) - 1); } } // namespace Carbon diff --git a/lexer/tokenized_buffer.h b/lexer/tokenized_buffer.h index 3a7e9e0f9690..fa8c18521913 100644 --- a/lexer/tokenized_buffer.h +++ b/lexer/tokenized_buffer.h @@ -5,8 +5,7 @@ #ifndef LEXER_TOKENIZED_BUFFER_H_ #define LEXER_TOKENIZED_BUFFER_H_ -#include - +#include #include #include "diagnostics/diagnostic_emitter.h" @@ -48,12 +47,20 @@ class TokenizedBuffer { public: Token() = default; - bool operator==(const Token& rhs) const { return index == rhs.index; } - bool operator!=(const Token& rhs) const { return index != rhs.index; } - bool operator<(const Token& rhs) const { return index < rhs.index; } - bool operator<=(const Token& rhs) const { return index <= rhs.index; } - bool operator>(const Token& rhs) const { return index > rhs.index; } - bool operator>=(const Token& rhs) const { return index >= rhs.index; } + auto operator==(const Token& rhs) const -> bool { + return index == rhs.index; + } + auto operator!=(const Token& rhs) const -> bool { + return index != rhs.index; + } + auto operator<(const Token& rhs) const -> bool { return index < rhs.index; } + auto operator<=(const Token& rhs) const -> bool { + return index <= rhs.index; + } + auto operator>(const Token& rhs) const -> bool { return index > rhs.index; } + auto operator>=(const Token& rhs) const -> bool { + return index >= rhs.index; + } private: friend class TokenizedBuffer; @@ -78,12 +85,20 @@ class TokenizedBuffer { public: Line() = default; - bool operator==(const Line& rhs) const { return index == rhs.index; } - bool operator!=(const Line& rhs) const { return index != rhs.index; } - bool operator<(const Line& rhs) const { return index < rhs.index; } - bool operator<=(const Line& rhs) const { return index <= rhs.index; } - bool operator>(const Line& rhs) const { return index > rhs.index; } - bool operator>=(const Line& rhs) const { return index >= rhs.index; } + auto operator==(const Line& rhs) const -> bool { + return index == rhs.index; + } + auto operator!=(const Line& rhs) const -> bool { + return index != rhs.index; + } + auto operator<(const Line& rhs) const -> bool { return index < rhs.index; } + auto operator<=(const Line& rhs) const -> bool { + return index <= rhs.index; + } + auto operator>(const Line& rhs) const -> bool { return index > rhs.index; } + auto operator>=(const Line& rhs) const -> bool { + return index >= rhs.index; + } private: friend class TokenizedBuffer; @@ -110,8 +125,12 @@ class TokenizedBuffer { // Most normal APIs are provided by the `TokenizedBuffer`, we just support // basic comparison operations. - bool operator==(const Identifier& rhs) const { return index == rhs.index; } - bool operator!=(const Identifier& rhs) const { return index != rhs.index; } + auto operator==(const Identifier& rhs) const -> bool { + return index == rhs.index; + } + auto operator!=(const Identifier& rhs) const -> bool { + return index != rhs.index; + } private: friend class TokenizedBuffer; @@ -130,23 +149,25 @@ class TokenizedBuffer { explicit TokenIterator(Token token) : token(token) {} - bool operator==(const TokenIterator& rhs) const { + auto operator==(const TokenIterator& rhs) const -> bool { return token == rhs.token; } - bool operator<(const TokenIterator& rhs) const { return token < rhs.token; } + auto operator<(const TokenIterator& rhs) const -> bool { + return token < rhs.token; + } - const Token& operator*() const { return token; } - Token& operator*() { return token; } + auto operator*() const -> const Token& { return token; } + auto operator*() -> Token& { return token; } - int operator-(const TokenIterator& rhs) const { + auto operator-(const TokenIterator& rhs) const -> int { return token.index - rhs.token.index; } - TokenIterator& operator+=(int n) { + auto operator+=(int n) -> TokenIterator& { token.index += n; return *this; } - TokenIterator& operator-=(int n) { + auto operator-=(int n) -> TokenIterator& { token.index -= n; return *this; } @@ -164,33 +185,34 @@ class TokenizedBuffer { // // FIXME: Need to pass in some diagnostic machinery to report the details of // the error! Right now it prints to stderr. - static TokenizedBuffer Lex(SourceBuffer& source, DiagnosticEmitter& emitter); + static auto Lex(SourceBuffer& source, DiagnosticEmitter& emitter) + -> TokenizedBuffer; // Returns true if the buffer has errors that are detectable at lexing time. - auto HasErrors() const -> bool { return has_errors; } + [[nodiscard]] auto HasErrors() const -> bool { return has_errors; } - llvm::iterator_range Tokens() const { + [[nodiscard]] auto Tokens() const -> llvm::iterator_range { return llvm::make_range(TokenIterator(Token(0)), TokenIterator(Token(token_infos.size()))); } - auto Size() const -> int { return token_infos.size(); } + [[nodiscard]] auto Size() const -> int { return token_infos.size(); } - auto GetKind(Token token) const -> TokenKind; - auto GetLine(Token token) const -> Line; + [[nodiscard]] auto GetKind(Token token) const -> TokenKind; + [[nodiscard]] auto GetLine(Token token) const -> Line; // Returns the 1-based line number. - auto GetLineNumber(Token token) const -> int; + [[nodiscard]] auto GetLineNumber(Token token) const -> int; // Returns the 1-based column number. - auto GetColumnNumber(Token token) const -> int; + [[nodiscard]] auto GetColumnNumber(Token token) const -> int; // Returns the source text lexed into this token. - auto GetTokenText(Token token) const -> llvm::StringRef; + [[nodiscard]] auto GetTokenText(Token token) const -> llvm::StringRef; // Returns the identifier associated with this token. The token kind must be // an `Identifier`. - auto GetIdentifier(Token token) const -> Identifier; + [[nodiscard]] auto GetIdentifier(Token token) const -> Identifier; // Returns the value of an `IntegerLiteral()` token. auto GetIntegerLiteral(Token token) const -> llvm::APInt; @@ -198,26 +220,26 @@ class TokenizedBuffer { // Returns the closing token matched with the given opening token. // // The given token must be an opening token kind. - auto GetMatchedClosingToken(Token opening_token) const -> Token; + [[nodiscard]] auto GetMatchedClosingToken(Token opening_token) const -> Token; // Returns the opening token matched with the given closing token. // // The given token must be a closing token kind. - auto GetMatchedOpeningToken(Token closing_token) const -> Token; + [[nodiscard]] auto GetMatchedOpeningToken(Token closing_token) const -> Token; // Returns whether the token was created as part of an error recovery effort. // // For example, a closing paren inserted to match an unmatched paren. - auto IsRecoveryToken(Token token) const -> bool; + [[nodiscard]] auto IsRecoveryToken(Token token) const -> bool; // Returns the 1-based line number. - auto GetLineNumber(Line line) const -> int; + [[nodiscard]] auto GetLineNumber(Line line) const -> int; // Returns the 1-based indentation column number. - auto GetIndentColumnNumber(Line line) const -> int; + [[nodiscard]] auto GetIndentColumnNumber(Line line) const -> int; // Returns the text for an identifier. - auto GetIdentifierText(Identifier id) const -> llvm::StringRef; + [[nodiscard]] auto GetIdentifierText(Identifier id) const -> llvm::StringRef; // Prints a description of the tokenized stream to the provided `raw_ostream`. // @@ -314,12 +336,12 @@ class TokenizedBuffer { explicit TokenizedBuffer(SourceBuffer& source) : source(&source) {} auto GetLineInfo(Line line) -> LineInfo&; - auto GetLineInfo(Line line) const -> const LineInfo&; + [[nodiscard]] auto GetLineInfo(Line line) const -> const LineInfo&; auto AddLine(LineInfo info) -> Line; auto GetTokenInfo(Token token) -> TokenInfo&; - auto GetTokenInfo(Token token) const -> const TokenInfo&; + [[nodiscard]] auto GetTokenInfo(Token token) const -> const TokenInfo&; auto AddToken(TokenInfo info) -> Token; - auto GetTokenPrintWidths(Token token) const -> PrintWidths; + [[nodiscard]] auto GetTokenPrintWidths(Token token) const -> PrintWidths; auto PrintToken(llvm::raw_ostream& output_stream, Token token, PrintWidths widths) const -> void; diff --git a/lexer/tokenized_buffer_fuzzer.cpp b/lexer/tokenized_buffer_fuzzer.cpp index b90a048992e8..c530f4bd3edc 100644 --- a/lexer/tokenized_buffer_fuzzer.cpp +++ b/lexer/tokenized_buffer_fuzzer.cpp @@ -2,6 +2,7 @@ // Exceptions. See /LICENSE for license information. // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +#include #include #include "diagnostics/diagnostic_emitter.h" @@ -10,12 +11,14 @@ namespace Carbon { -extern "C" int LLVMFuzzerTestOneInput(const unsigned char* data, size_t size) { +// NOLINTNEXTLINE: Match the documented fuzzer entry point declaration style. +extern "C" int LLVMFuzzerTestOneInput(const unsigned char* data, + std::size_t size) { // We need two bytes of data to compute a file name length. if (size < 2) { return 0; } - unsigned short raw_filename_length; + uint16_t raw_filename_length; std::memcpy(&raw_filename_length, data, 2); data += 2; size -= 2; diff --git a/lexer/tokenized_buffer_test_helpers.h b/lexer/tokenized_buffer_test_helpers.h index 8ae9f208cd4c..ebc099472f13 100644 --- a/lexer/tokenized_buffer_test_helpers.h +++ b/lexer/tokenized_buffer_test_helpers.h @@ -31,8 +31,8 @@ struct ExpectedToken { bool recovery = false; llvm::StringRef text = ""; - friend std::ostream& operator<<(std::ostream& output, - const ExpectedToken& expected) { + friend auto operator<<(std::ostream& output, const ExpectedToken& expected) + -> std::ostream& { output << "\ntoken: { kind: '" << expected.kind.Name().str(); if (expected.line != -1) { output << "', line: " << expected.line; @@ -57,6 +57,7 @@ struct ExpectedToken { // TODO: Consider rewriting this into a `TokenEq` matcher which is used inside // `ElementsAre`. If that isn't easily done, potentially worth checking for size // mismatches first. +// NOLINTNEXTLINE: Expands from GoogleTest. MATCHER_P(HasTokens, raw_all_expected, "") { const TokenizedBuffer& buffer = arg; llvm::ArrayRef all_expected = raw_all_expected; @@ -133,6 +134,7 @@ MATCHER_P(HasTokens, raw_all_expected, "") { return matches; } +// NOLINTNEXTLINE: Expands from GoogleTest. MATCHER_P2(IsKeyValueScalars, key, value, "") { auto* kv_node = llvm::dyn_cast(arg); if (!kv_node) { diff --git a/parser/parse_node_kind.h b/parser/parse_node_kind.h index 7035275d07a0..debcd67421ad 100644 --- a/parser/parse_node_kind.h +++ b/parser/parse_node_kind.h @@ -27,8 +27,10 @@ class ParseNodeKind { public: // The formatting for this macro is weird due to a `clang-format` bug. See // https://bugs.llvm.org/show_bug.cgi?id=48320 for details. -#define CARBON_PARSE_NODE_KIND(Name) \ - static constexpr auto Name()->ParseNodeKind { return KindEnum::Name; } +#define CARBON_PARSE_NODE_KIND(Name) \ + static constexpr auto Name()->ParseNodeKind { \ + return ParseNodeKind(KindEnum::Name); \ + } #include "parser/parse_node_kind.def" // The default constructor is deleted as objects of this type should always be @@ -51,12 +53,13 @@ class ParseNodeKind { #include "parser/parse_node_kind.def" }; - constexpr ParseNodeKind(KindEnum k) : kind(k) {} + constexpr explicit ParseNodeKind(KindEnum k) : kind(k) {} // Enable conversion to our private enum, including in a `constexpr` context, // to enable usage in `switch` and `case`. The enum remains private and // nothing else should be using this. - explicit constexpr operator KindEnum() const { return kind; } + // NOLINTNEXTLINE(google-explicit-constructor) + constexpr operator KindEnum() const { return kind; } KindEnum kind; }; diff --git a/parser/parse_test_helpers.h b/parser/parse_test_helpers.h index 5c0907980efe..28366a2b0885 100644 --- a/parser/parse_test_helpers.h +++ b/parser/parse_test_helpers.h @@ -50,7 +50,8 @@ struct ExpectedNode { class ExpectedNodesMatcher : public ::testing::MatcherInterface { public: - ExpectedNodesMatcher(llvm::SmallVector expected_nodess) + explicit ExpectedNodesMatcher( + llvm::SmallVector expected_nodess) : expected_nodes(std::move(expected_nodess)) {} auto MatchAndExplain(const ParseTree& tree, @@ -60,7 +61,7 @@ class ExpectedNodesMatcher private: auto MatchExpectedNode(const ParseTree& tree, ParseTree::Node n, - int postorder_index, ExpectedNode expected_node, + int postorder_index, const ExpectedNode& expected_node, ::testing::MatchResultListener& output) const -> bool; llvm::SmallVector expected_nodes; @@ -221,8 +222,8 @@ inline auto ExpectedNodesMatcher::DescribeTo(std::ostream* output_ptr) const inline auto ExpectedNodesMatcher::MatchExpectedNode( const ParseTree& tree, ParseTree::Node n, int postorder_index, - ExpectedNode expected_node, ::testing::MatchResultListener& output) const - -> bool { + const ExpectedNode& expected_node, + ::testing::MatchResultListener& output) const -> bool { bool matches = true; ParseNodeKind kind = tree.GetNodeKind(n); diff --git a/parser/parse_tree_fuzzer.cpp b/parser/parse_tree_fuzzer.cpp index 8e3c53da6659..7828ddb5d41b 100644 --- a/parser/parse_tree_fuzzer.cpp +++ b/parser/parse_tree_fuzzer.cpp @@ -3,6 +3,7 @@ // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception #include +#include #include #include "diagnostics/diagnostic_emitter.h" @@ -19,7 +20,7 @@ extern "C" int LLVMFuzzerTestOneInput(const unsigned char* data, if (size < 2) { return 0; } - unsigned short raw_filename_length; + uint16_t raw_filename_length; std::memcpy(&raw_filename_length, data, 2); data += 2; size -= 2; diff --git a/parser/parser_impl.cpp b/parser/parser_impl.cpp index 825ae3d93ca3..01a3eccb3d88 100644 --- a/parser/parser_impl.cpp +++ b/parser/parser_impl.cpp @@ -52,7 +52,7 @@ auto ParseTree::Parser::ConsumeIf(TokenKind kind) auto ParseTree::Parser::AddLeafNode(ParseNodeKind kind, TokenizedBuffer::Token token) -> Node { Node n(tree.node_impls.size()); - tree.node_impls.push_back(NodeImpl(kind, token, /*SubtreeSize=*/1)); + tree.node_impls.push_back(NodeImpl(kind, token, /*subtree_size_arg=*/1)); return n; } @@ -94,7 +94,7 @@ auto ParseTree::Parser::AddNode(ParseNodeKind n_kind, TokenizedBuffer::Token t, SubtreeStart& start, bool has_error) -> Node { // The size of the subtree is the change in size from when we started this // subtree to now, but including the node we're about to add. - int tree_stop_size = tree.node_impls.size() + 1; + int tree_stop_size = static_cast(tree.node_impls.size()) + 1; int subtree_size = tree_stop_size - start.tree_size; Node n(tree.node_impls.size()); @@ -346,6 +346,9 @@ auto ParseTree::Parser::ParseDeclaration() -> llvm::Optional { return ParseFunctionDeclaration(); case TokenKind::Semi(): return ParseEmptyDeclaration(); + default: + // Errors are handled outside the switch. + break; } // We didn't recognize an introducer for a valid declaration. diff --git a/source/source_buffer.cpp b/source/source_buffer.cpp index 3de0087a44d3..eaff55633d89 100644 --- a/source/source_buffer.cpp +++ b/source/source_buffer.cpp @@ -4,13 +4,13 @@ #include "source/source_buffer.h" -#include #include -#include #include #include #include +#include +#include #include #include "llvm/ADT/ScopeExit.h" @@ -84,7 +84,8 @@ SourceBuffer::~SourceBuffer() { if (!text_.empty()) { errno = 0; int result = - munmap(const_cast((const void*)text_.data()), text_.size()); + munmap(const_cast(static_cast(text_.data())), + text_.size()); (void)result; assert(result != -1 && "Unmapping text failed!"); } diff --git a/source/source_buffer.h b/source/source_buffer.h index 710901885fc7..51ed1e8061a6 100644 --- a/source/source_buffer.h +++ b/source/source_buffer.h @@ -46,7 +46,7 @@ class SourceBuffer { // comment for details. SourceBuffer(const SourceBuffer& arg) = delete; - SourceBuffer(SourceBuffer&& arg) + SourceBuffer(SourceBuffer&& arg) noexcept : filename_(std::move(arg.filename_)), text_(arg.text_), is_string_rep_(arg.is_string_rep_) { @@ -66,9 +66,9 @@ class SourceBuffer { ~SourceBuffer(); - llvm::StringRef Filename() const { return filename_; } + [[nodiscard]] auto Filename() const -> llvm::StringRef { return filename_; } - llvm::StringRef Text() const { return text_; } + [[nodiscard]] auto Text() const -> llvm::StringRef { return text_; } private: std::string filename_; @@ -87,7 +87,7 @@ class SourceBuffer { explicit SourceBuffer(llvm::StringRef fake_filename, std::string buffer_text) : filename_(fake_filename.str()), is_string_rep_(true), - string_storage_(buffer_text) { + string_storage_(std::move(buffer_text)) { text_ = string_storage_; }