diff --git a/.clang-tidy b/.clang-tidy index b078ecc2d414..2ee99f1ee966 100644 --- a/.clang-tidy +++ b/.clang-tidy @@ -4,11 +4,12 @@ --- 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-*, + -*, bugprone-*, -bugprone-narrowing-conversions, 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, + -modernize-use-default-member-init, performance-*, readability-braces-around-statements, readability-identifier-naming WarningsAsErrors: true CheckOptions: diff --git a/toolchain/lexer/character_set.h b/toolchain/lexer/character_set.h index 304d5e2766c3..101eb3029197 100644 --- a/toolchain/lexer/character_set.h +++ b/toolchain/lexer/character_set.h @@ -18,12 +18,12 @@ namespace Carbon { // // Alphabetical characters are permitted at the start of identifiers. This // currently includes 'A'..'Z' and 'a'..'z'. -inline bool IsAlpha(char c) { return llvm::isAlpha(c); } +inline auto IsAlpha(char c) -> bool { return llvm::isAlpha(c); } // Is this a decimal digit according to Carbon's lexical rules? // // This currently includes '0'..'9'. -inline bool IsDecimalDigit(char c) { return llvm::isDigit(c); } +inline auto IsDecimalDigit(char c) -> bool { return llvm::isDigit(c); } // Is this an alphanumeric character according to Carbon's lexical rules? // @@ -33,7 +33,7 @@ inline bool IsDecimalDigit(char c) { return llvm::isDigit(c); } // // Note that '_' is not considered alphanumeric, despite in most circumstances // being a valid continuation character of an identifier or numeric literal. -inline bool IsAlnum(char c) { return llvm::isAlnum(c); } +inline auto IsAlnum(char c) -> bool { return llvm::isAlnum(c); } // Is this a hexadecimal digit according to Carbon's lexical rules? // @@ -42,7 +42,7 @@ inline bool IsAlnum(char c) { return llvm::isAlnum(c); } // // Note that lowercase 'a'..'f' are currently not considered hexadecimal digits // in any context. -inline bool IsUpperHexDigit(char c) { +inline auto IsUpperHexDigit(char c) -> bool { return ('0' <= c && c <= '9') || ('A' <= c && c <= 'F'); } @@ -50,23 +50,25 @@ inline bool IsUpperHexDigit(char c) { // // Lowercase letters in numeric literals can be followed by `+` or `-` to // extend the literal. -inline bool IsLower(char c) { return 'a' <= c && c <= 'z'; } +inline auto IsLower(char c) -> bool { return 'a' <= c && c <= 'z'; } // Is this character considered to be horizontal whitespace? // // Such characters can appear in the indentation of a line. -inline bool IsHorizontalWhitespace(char c) { return c == ' ' || c == '\t'; } +inline auto IsHorizontalWhitespace(char c) -> bool { + return c == ' ' || c == '\t'; +} // Is this character considered to be vertical whitespace? // // Such characters are considered to terminate lines. -inline bool IsVerticalWhitespace(char c) { return c == '\n'; } +inline auto IsVerticalWhitespace(char c) -> bool { return c == '\n'; } // Is this character considered to be whitespace? // // Changes here will need matching changes in // `TokenizedBuffer::Lexer::SkipWhitespace`. -inline bool IsSpace(char c) { +inline auto IsSpace(char c) -> bool { return IsHorizontalWhitespace(c) || IsVerticalWhitespace(c); } diff --git a/toolchain/lexer/numeric_literal.h b/toolchain/lexer/numeric_literal.h index b33d5833f0e0..93443127dad5 100644 --- a/toolchain/lexer/numeric_literal.h +++ b/toolchain/lexer/numeric_literal.h @@ -19,7 +19,7 @@ namespace Carbon { class LexedNumericLiteral { public: // Get the text corresponding to this literal. - auto Text() const -> llvm::StringRef { return text; } + [[nodiscard]] auto Text() const -> llvm::StringRef { return text; } // Extract a numeric literal from the given text, if it has a suitable form. // @@ -52,7 +52,7 @@ class LexedNumericLiteral { auto ComputeValue(DiagnosticEmitter& emitter) const -> Value; private: - LexedNumericLiteral() {} + LexedNumericLiteral() = default; class Parser; diff --git a/toolchain/lexer/numeric_literal_test.cpp b/toolchain/lexer/numeric_literal_test.cpp index 758efa990bed..ee0fb48fe1e7 100644 --- a/toolchain/lexer/numeric_literal_test.cpp +++ b/toolchain/lexer/numeric_literal_test.cpp @@ -75,7 +75,7 @@ struct RealMatcher { }; // Matcher for a real literal value. -auto HasRealValue(RealMatcher real_matcher) +auto HasRealValue(const RealMatcher& real_matcher) -> Matcher { return VariantWith(AllOf( Field(&LexedNumericLiteral::RealValue::radix, real_matcher.radix), diff --git a/toolchain/lexer/string_literal.cpp b/toolchain/lexer/string_literal.cpp index b29b6d099c54..1d57ea844dfe 100644 --- a/toolchain/lexer/string_literal.cpp +++ b/toolchain/lexer/string_literal.cpp @@ -79,7 +79,7 @@ struct MismatchedIndentInString : SimpleDiagnostic { static auto TakeMultiLineStringLiteralPrefix(llvm::StringRef source_text) -> llvm::StringRef { llvm::StringRef remaining = source_text; - if (!remaining.consume_front("\"\"\"")) { + if (!remaining.consume_front(R"(""")")) { return llvm::StringRef(); } @@ -113,7 +113,7 @@ auto LexedStringLiteral::Lex(llvm::StringRef source_text) bool multi_line = !multi_line_prefix.empty(); if (multi_line) { source_text = source_text.drop_front(multi_line_prefix.size()); - terminator = "\"\"\""; + terminator = R"(""")"; } else if (!source_text.consume_front("\"")) { return llvm::None; } diff --git a/toolchain/lexer/string_literal.h b/toolchain/lexer/string_literal.h index 79715bcd73c9..8766f8918ea8 100644 --- a/toolchain/lexer/string_literal.h +++ b/toolchain/lexer/string_literal.h @@ -13,10 +13,10 @@ namespace Carbon { class LexedStringLiteral { public: // Get the text corresponding to this literal. - auto Text() const -> llvm::StringRef { return text; } + [[nodiscard]] auto Text() const -> llvm::StringRef { return text; } // Determine whether this is a multi-line string literal. - auto IsMultiLine() const -> bool { return multi_line; } + [[nodiscard]] auto IsMultiLine() const -> bool { return multi_line; } // Extract a string literal token from the given text, if it has a suitable // form. diff --git a/toolchain/lexer/test_helpers.h b/toolchain/lexer/test_helpers.h index 75ef4f709b78..623c6cdcbe7b 100644 --- a/toolchain/lexer/test_helpers.h +++ b/toolchain/lexer/test_helpers.h @@ -13,8 +13,7 @@ #include "llvm/Support/FormatVariadic.h" #include "toolchain/diagnostics/diagnostic_emitter.h" -namespace Carbon { -namespace Testing { +namespace Carbon::Testing { // A diagnostic translator for tests that lex a single token. Produces // locations such as "`12.5`:1:3" to refer to the third character in the token. @@ -23,7 +22,8 @@ class SingleTokenDiagnosticTranslator public: // Form a translator for a given token. The string provided here must refer // to the same character array that we are going to lex. - SingleTokenDiagnosticTranslator(llvm::StringRef token) : token(token) {} + explicit SingleTokenDiagnosticTranslator(llvm::StringRef token) + : token(token) {} auto GetLocation(const char* pos) -> Diagnostic::Location override { assert(llvm::is_sorted(std::array{token.begin(), pos, token.end()}) && @@ -47,14 +47,13 @@ class SingleTokenDiagnosticTranslator } private: - auto SynthesizeFilename() const -> std::string { + [[nodiscard]] auto SynthesizeFilename() const -> std::string { return llvm::formatv("`{0}`", token); } llvm::StringRef token; }; -} // namespace Testing -} // namespace Carbon +} // namespace Carbon::Testing #endif // TOOLCHAIN_LEXER_TOKENIZED_BUFFER_TEST_HELPERS_H_ diff --git a/toolchain/lexer/tokenized_buffer.cpp b/toolchain/lexer/tokenized_buffer.cpp index 64f2fdb2e0cb..282569d42760 100644 --- a/toolchain/lexer/tokenized_buffer.cpp +++ b/toolchain/lexer/tokenized_buffer.cpp @@ -123,11 +123,12 @@ class TokenizedBuffer::Lexer { public: // Consumes (and discard) a valid token to construct a result - // indicating a token has been produced. + // indicating a token has been produced. Relies on implicit conversions. + // NOLINTNEXTLINE(google-explicit-constructor) LexResult(Token) : LexResult(true) {} // Returns a result indicating no token was produced. - static LexResult NoMatch() { return LexResult(false); } + static auto NoMatch() -> LexResult { return LexResult(false); } // Tests whether a token was produced by the lexing routine, and // the lexer can continue forming tokens. diff --git a/toolchain/lexer/tokenized_buffer.h b/toolchain/lexer/tokenized_buffer.h index d5001eb5ba51..feda2e9d357d 100644 --- a/toolchain/lexer/tokenized_buffer.h +++ b/toolchain/lexer/tokenized_buffer.h @@ -215,16 +215,16 @@ class TokenizedBuffer { public: // The mantissa, represented as an unsigned integer. - auto Mantissa() const -> const llvm::APInt& { + [[nodiscard]] auto Mantissa() const -> const llvm::APInt& { return buffer->literal_int_storage[literal_index]; } // The exponent, represented as a signed integer. - auto Exponent() const -> const llvm::APInt& { + [[nodiscard]] auto Exponent() const -> const llvm::APInt& { return buffer->literal_int_storage[literal_index + 1]; } // If false, the value is mantissa * 2^exponent. // If true, the value is mantissa * 10^exponent. - auto IsDecimal() const -> bool { return is_decimal; } + [[nodiscard]] auto IsDecimal() const -> bool { return is_decimal; } private: friend class TokenizedBuffer; @@ -291,7 +291,7 @@ class TokenizedBuffer { [[nodiscard]] auto GetRealLiteral(Token token) const -> RealLiteralValue; // Returns the value of a `StringLiteral()` token. - auto GetStringLiteral(Token token) const -> llvm::StringRef; + [[nodiscard]] auto GetStringLiteral(Token token) const -> llvm::StringRef; // Returns the closing token matched with the given opening token. // diff --git a/toolchain/parser/parse_test_helpers.h b/toolchain/parser/parse_test_helpers.h index 5dd46b1ab4b7..c7e3dfe86eca 100644 --- a/toolchain/parser/parse_test_helpers.h +++ b/toolchain/parser/parse_test_helpers.h @@ -7,6 +7,7 @@ #include #include +#include #include #include "gmock/gmock.h" @@ -318,8 +319,9 @@ auto MatchNode(Args... args) -> ExpectedNode { #include "parse_node_kind.def" // Helper for matching a designator `lhs.rhs`. -auto MatchDesignator(ExpectedNode lhs, std::string rhs) -> ExpectedNode { - return MatchDesignatorExpression(std::move(lhs), MatchDesignatedName(rhs)); +inline auto MatchDesignator(ExpectedNode lhs, std::string rhs) -> ExpectedNode { + return MatchDesignatorExpression(std::move(lhs), + MatchDesignatedName(std::move(rhs))); } // Helper for matching a function parameter list. diff --git a/toolchain/parser/parser_impl.cpp b/toolchain/parser/parser_impl.cpp index 5e6564cc8c0d..d26e8256e996 100644 --- a/toolchain/parser/parser_impl.cpp +++ b/toolchain/parser/parser_impl.cpp @@ -650,7 +650,7 @@ auto ParseTree::Parser::ParseParenExpression() -> llvm::Optional { ParseCloseParen(open_paren, ParseNodeKind::ParenExpressionEnd()); return AddNode(ParseNodeKind::ParenExpression(), open_paren, start, - /*has_errors=*/!expr || !close_paren); + /*has_error=*/!expr || !close_paren); } auto ParseTree::Parser::ParsePrimaryExpression() -> llvm::Optional { @@ -971,7 +971,7 @@ auto ParseTree::Parser::ParseParenCondition(TokenKind introducer) ParseCloseParen(*open_paren, ParseNodeKind::ConditionEnd()); return AddNode(ParseNodeKind::Condition(), *open_paren, start, - /*has_errors=*/!expr || !close_paren); + /*has_error=*/!expr || !close_paren); } auto ParseTree::Parser::ParseIfStatement() -> llvm::Optional { @@ -985,7 +985,7 @@ auto ParseTree::Parser::ParseIfStatement() -> llvm::Optional { else_has_errors = !ParseStatement(); } return AddNode(ParseNodeKind::IfStatement(), if_token, start, - /*has_errors=*/!cond || !then_case || else_has_errors); + /*has_error=*/!cond || !then_case || else_has_errors); } auto ParseTree::Parser::ParseWhileStatement() -> llvm::Optional { @@ -994,7 +994,7 @@ auto ParseTree::Parser::ParseWhileStatement() -> llvm::Optional { auto cond = ParseParenCondition(TokenKind::WhileKeyword()); auto body = ParseStatement(); return AddNode(ParseNodeKind::WhileStatement(), while_token, start, - /*has_errors=*/!cond || !body); + /*has_error=*/!cond || !body); } auto ParseTree::Parser::ParseKeywordStatement(ParseNodeKind kind, @@ -1020,7 +1020,7 @@ auto ParseTree::Parser::ParseKeywordStatement(ParseNodeKind kind, {.preceding = keyword_kind}); // FIXME: Try to skip to a semicolon to recover. } - return AddNode(kind, keyword, start, /*has_errors=*/!semi || arg_error); + return AddNode(kind, keyword, start, /*has_error=*/!semi || arg_error); } auto ParseTree::Parser::ParseStatement() -> llvm::Optional { diff --git a/toolchain/parser/parser_impl.h b/toolchain/parser/parser_impl.h index cb3a676cfe87..7861fc82db09 100644 --- a/toolchain/parser/parser_impl.h +++ b/toolchain/parser/parser_impl.h @@ -34,16 +34,19 @@ class ParseTree::Parser { } // Gets the kind of the next token to be consumed. - auto NextTokenKind() const -> TokenKind { return tokens.GetKind(*position); } + [[nodiscard]] auto NextTokenKind() const -> TokenKind { + return tokens.GetKind(*position); + } // Tests whether the next token to be consumed is of the specified kind. - auto NextTokenIs(TokenKind kind) const -> bool { + [[nodiscard]] auto NextTokenIs(TokenKind kind) const -> bool { return NextTokenKind() == kind; } // Tests whether the next token to be consumed is of any of the specified // kinds. - auto NextTokenIsOneOf(std::initializer_list kinds) const -> bool { + [[nodiscard]] auto NextTokenIsOneOf( + std::initializer_list kinds) const -> bool { return NextTokenKind().IsOneOf(kinds); } diff --git a/toolchain/parser/precedence.cpp b/toolchain/parser/precedence.cpp index 5c6c5ebd181a..8ad701215cc0 100644 --- a/toolchain/parser/precedence.cpp +++ b/toolchain/parser/precedence.cpp @@ -44,7 +44,7 @@ constexpr int8_t NumPrecedenceLevels = Lowest + 1; // A precomputed lookup table determining the relative precedence of two // precedence groups. struct OperatorPriorityTable { - constexpr OperatorPriorityTable() : table{} { + constexpr OperatorPriorityTable() : table() { // Start with a list of , // relationships. MarkHigherThan({Highest}, {NumericPrefix, BitwisePrefix, LogicalPrefix, @@ -310,8 +310,8 @@ auto PrecedenceGroup::ForTrailing(TokenKind kind, bool infix) auto PrecedenceGroup::GetPriority(PrecedenceGroup left, PrecedenceGroup right) -> OperatorPriority { - static constexpr OperatorPriorityTable lookup; - return lookup.table[left.level][right.level]; + static constexpr OperatorPriorityTable Lookup; + return Lookup.table[left.level][right.level]; } } // namespace Carbon diff --git a/toolchain/parser/precedence.h b/toolchain/parser/precedence.h index 7f995b9bda50..883dcc0c480a 100644 --- a/toolchain/parser/precedence.h +++ b/toolchain/parser/precedence.h @@ -30,6 +30,9 @@ enum class Associativity : int8_t { // A precedence group associated with an operator or expression. class PrecedenceGroup { private: + // We rely on implicit conversions via `int8_t` for enumerators defined in the + // implementation. + // NOLINTNEXTLINE(google-explicit-constructor) PrecedenceGroup(int8_t level) : level(level) {} public: @@ -75,7 +78,7 @@ class PrecedenceGroup { -> OperatorPriority; // Get the associativity of this precedence group. - Associativity GetAssociativity() const { + [[nodiscard]] auto GetAssociativity() const -> Associativity { return static_cast(GetPriority(*this, *this)); } diff --git a/toolchain/parser/precedence_test.cpp b/toolchain/parser/precedence_test.cpp index ad1a172ba7f1..252bf2794da6 100644 --- a/toolchain/parser/precedence_test.cpp +++ b/toolchain/parser/precedence_test.cpp @@ -12,7 +12,6 @@ namespace Carbon { namespace { using ::testing::Eq; -using ::testing::Ne; TEST(PrecedenceTest, OperatorsAreRecognized) { EXPECT_TRUE(PrecedenceGroup::ForLeading(TokenKind::Minus()).hasValue()); diff --git a/toolchain/source/source_buffer.cpp b/toolchain/source/source_buffer.cpp index b6c2f3d2df43..0c269a2a9a06 100644 --- a/toolchain/source/source_buffer.cpp +++ b/toolchain/source/source_buffer.cpp @@ -61,6 +61,9 @@ auto SourceBuffer::CreateFromFile(llvm::StringRef filename) MAP_PRIVATE | MAP_POPULATE, #endif file_descriptor, /*offset=*/0); + // The `MAP_FAILED` macro may expand to a cast to pointer that `clang-tidy` + // complains about. + // NOLINTNEXTLINE(performance-no-int-to-ptr) if (mapped_text == MAP_FAILED) { return ErrnoToError(errno); }