diff --git a/clang-tools-extra/clang-tidy/modernize/IntegralLiteralExpressionMatcher.cpp b/clang-tools-extra/clang-tidy/modernize/IntegralLiteralExpressionMatcher.cpp index 91a10f8956e6..ca0e99fc787a 100644 --- a/clang-tools-extra/clang-tidy/modernize/IntegralLiteralExpressionMatcher.cpp +++ b/clang-tools-extra/clang-tidy/modernize/IntegralLiteralExpressionMatcher.cpp @@ -8,6 +8,7 @@ #include "IntegralLiteralExpressionMatcher.h" +#include #include #include @@ -81,6 +82,50 @@ bool IntegralLiteralExpressionMatcher::unaryOperator() { return true; } +static LiteralSize literalTokenSize(const Token &Tok) { + unsigned int Length = Tok.getLength(); + if (Length <= 1) + return LiteralSize::Int; + + bool SeenUnsigned = false; + bool SeenLong = false; + bool SeenLongLong = false; + const char *Text = Tok.getLiteralData(); + for (unsigned int End = Length - 1; End > 0; --End) { + if (std::isdigit(Text[End])) + break; + + if (std::toupper(Text[End]) == 'U') + SeenUnsigned = true; + else if (std::toupper(Text[End]) == 'L') { + if (SeenLong) + SeenLongLong = true; + SeenLong = true; + } + } + + if (SeenLongLong) { + if (SeenUnsigned) + return LiteralSize::UnsignedLongLong; + + return LiteralSize::LongLong; + } + if (SeenLong) { + if (SeenUnsigned) + return LiteralSize::UnsignedLong; + + return LiteralSize::Long; + } + if (SeenUnsigned) + return LiteralSize::UnsignedInt; + + return LiteralSize::Int; +} + +static bool operator<(LiteralSize LHS, LiteralSize RHS) { + return static_cast(LHS) < static_cast(RHS); +} + bool IntegralLiteralExpressionMatcher::unaryExpr() { if (!unaryOperator()) return false; @@ -102,7 +147,10 @@ bool IntegralLiteralExpressionMatcher::unaryExpr() { !isIntegralConstant(*Current)) { return false; } + + LargestSize = std::max(LargestSize, literalTokenSize(*Current)); ++Current; + return true; } @@ -217,14 +265,24 @@ bool IntegralLiteralExpressionMatcher::conditionalExpr() { } bool IntegralLiteralExpressionMatcher::commaExpr() { - return nonTerminalChainedExpr( - &IntegralLiteralExpressionMatcher::conditionalExpr); + auto Pred = CommaAllowed + ? std::function( + [](Token Tok) { return Tok.is(tok::TokenKind::comma); }) + : std::function([](Token) { return false; }); + return nonTerminalChainedExpr( + &IntegralLiteralExpressionMatcher::conditionalExpr, Pred); } bool IntegralLiteralExpressionMatcher::expr() { return commaExpr(); } bool IntegralLiteralExpressionMatcher::match() { - return expr() && Current == End; + // Top-level allowed expression is conditionalExpr(), not expr(), because + // comma operators are only valid initializers when used inside parentheses. + return conditionalExpr() && Current == End; +} + +LiteralSize IntegralLiteralExpressionMatcher::largestLiteralSize() const { + return LargestSize; } } // namespace modernize diff --git a/clang-tools-extra/clang-tidy/modernize/IntegralLiteralExpressionMatcher.h b/clang-tools-extra/clang-tidy/modernize/IntegralLiteralExpressionMatcher.h index 9202cc1ed457..4499ef983ea4 100644 --- a/clang-tools-extra/clang-tidy/modernize/IntegralLiteralExpressionMatcher.h +++ b/clang-tools-extra/clang-tidy/modernize/IntegralLiteralExpressionMatcher.h @@ -16,15 +16,27 @@ namespace clang { namespace tidy { namespace modernize { +enum class LiteralSize { + Unknown = 0, + Int, + UnsignedInt, + Long, + UnsignedLong, + LongLong, + UnsignedLongLong +}; + // Parses an array of tokens and returns true if they conform to the rules of // C++ for whole expressions involving integral literals. Follows the operator -// precedence rules of C++. +// precedence rules of C++. Optionally exclude comma operator expressions. class IntegralLiteralExpressionMatcher { public: - IntegralLiteralExpressionMatcher(ArrayRef Tokens) - : Current(Tokens.begin()), End(Tokens.end()) {} + IntegralLiteralExpressionMatcher(ArrayRef Tokens, bool CommaAllowed) + : Current(Tokens.begin()), End(Tokens.end()), CommaAllowed(CommaAllowed) { + } bool match(); + LiteralSize largestLiteralSize() const; private: bool advance(); @@ -64,6 +76,8 @@ private: ArrayRef::iterator Current; ArrayRef::iterator End; + LiteralSize LargestSize{LiteralSize::Unknown}; + bool CommaAllowed; }; } // namespace modernize diff --git a/clang-tools-extra/clang-tidy/modernize/MacroToEnumCheck.cpp b/clang-tools-extra/clang-tidy/modernize/MacroToEnumCheck.cpp index 5d7daf837007..1c196a9a9274 100644 --- a/clang-tools-extra/clang-tidy/modernize/MacroToEnumCheck.cpp +++ b/clang-tools-extra/clang-tidy/modernize/MacroToEnumCheck.cpp @@ -321,8 +321,14 @@ void MacroToEnumCallbacks::FileChanged(SourceLocation Loc, bool MacroToEnumCallbacks::isInitializer(ArrayRef MacroTokens) { - IntegralLiteralExpressionMatcher Matcher(MacroTokens); - return Matcher.match(); + IntegralLiteralExpressionMatcher Matcher(MacroTokens, LangOpts.C99 == 0); + bool Matched = Matcher.match(); + bool isC = !LangOpts.CPlusPlus; + if (isC && (Matcher.largestLiteralSize() != LiteralSize::Int && + Matcher.largestLiteralSize() != LiteralSize::UnsignedInt)) + return false; + + return Matched; } diff --git a/clang-tools-extra/test/clang-tidy/checkers/modernize-macro-to-enum.c b/clang-tools-extra/test/clang-tidy/checkers/modernize-macro-to-enum.c new file mode 100644 index 000000000000..c15e04510460 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/modernize-macro-to-enum.c @@ -0,0 +1,16 @@ +// RUN: %check_clang_tidy %s modernize-macro-to-enum %t + +// C requires enum values to fit into an int. +#define TOO_BIG1 1L +#define TOO_BIG2 1UL +#define TOO_BIG3 1LL +#define TOO_BIG4 1ULL + +// C forbids comma operator in initializing expressions. +#define BAD_OP 1, 2 + +#define SIZE_OK1 1 +#define SIZE_OK2 1U +// CHECK-MESSAGES: :[[@LINE-2]]:1: warning: replace macro with enum [modernize-macro-to-enum] +// CHECK-MESSAGES: :[[@LINE-3]]:9: warning: macro 'SIZE_OK1' defines an integral constant; prefer an enum instead +// CHECK-MESSAGES: :[[@LINE-3]]:9: warning: macro 'SIZE_OK2' defines an integral constant; prefer an enum instead diff --git a/clang-tools-extra/test/clang-tidy/checkers/modernize-macro-to-enum.cpp b/clang-tools-extra/test/clang-tidy/checkers/modernize-macro-to-enum.cpp index 06ed2d3f5ca8..4a697930d0f0 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/modernize-macro-to-enum.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/modernize-macro-to-enum.cpp @@ -213,6 +213,13 @@ // CHECK-FIXES-NEXT: COMPLEX_PAREN6 = ((+1)) // CHECK-FIXES-NEXT: }; +#define GOOD_COMMA (1, 2) +// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: replace macro with enum +// CHECK-MESSAGES: :[[@LINE-2]]:9: warning: macro 'GOOD_COMMA' defines an integral constant; prefer an enum instead +// CHECK-FIXES: enum { +// CHECK-FIXES-NEXT: GOOD_COMMA = (1, 2) +// CHECK-FIXES-NEXT: }; + // Macros appearing in conditional expressions can't be replaced // by enums. #define USE_FOO 1 @@ -322,6 +329,9 @@ inline void used_ifndef() {} #define EPS2 1e5 #define EPS3 1. +// Ignore macros invoking comma operator unless they are inside parens. +#define BAD_COMMA 1, 2 + extern void draw(unsigned int Color); void f() diff --git a/clang-tools-extra/unittests/clang-tidy/ModernizeModuleTest.cpp b/clang-tools-extra/unittests/clang-tidy/ModernizeModuleTest.cpp index b772c0054de4..2461dacd5684 100644 --- a/clang-tools-extra/unittests/clang-tidy/ModernizeModuleTest.cpp +++ b/clang-tools-extra/unittests/clang-tidy/ModernizeModuleTest.cpp @@ -36,177 +36,241 @@ static std::vector tokenify(const char *Text) { return Tokens; } -static bool matchText(const char *Text) { +static bool matchText(const char *Text, bool AllowComma) { std::vector Tokens{tokenify(Text)}; - modernize::IntegralLiteralExpressionMatcher Matcher(Tokens); + modernize::IntegralLiteralExpressionMatcher Matcher(Tokens, AllowComma); return Matcher.match(); } +static modernize::LiteralSize sizeText(const char *Text) { + std::vector Tokens{tokenify(Text)}; + modernize::IntegralLiteralExpressionMatcher Matcher(Tokens, true); + if (Matcher.match()) + return Matcher.largestLiteralSize(); + return modernize::LiteralSize::Unknown; +} + +static const char *toString(modernize::LiteralSize Value) { + switch (Value) { + case modernize::LiteralSize::Int: + return "Int"; + case modernize::LiteralSize::UnsignedInt: + return "UnsignedInt"; + case modernize::LiteralSize::Long: + return "Long"; + case modernize::LiteralSize::UnsignedLong: + return "UnsignedLong"; + case modernize::LiteralSize::LongLong: + return "LongLong"; + case modernize::LiteralSize::UnsignedLongLong: + return "UnsignedLongLong"; + default: + return "Unknown"; + } +} + namespace { -struct Param { +struct MatchParam { + bool AllowComma; bool Matched; const char *Text; - friend std::ostream &operator<<(std::ostream &Str, const Param &Value) { - return Str << "Matched: " << std::boolalpha << Value.Matched << ", Text: '" - << Value.Text << "'"; + friend std::ostream &operator<<(std::ostream &Str, const MatchParam &Value) { + return Str << "Allow operator,: " << std::boolalpha << Value.AllowComma + << ", Matched: " << std::boolalpha << Value.Matched + << ", Text: '" << Value.Text << '\''; } }; -class MatcherTest : public ::testing::TestWithParam {}; +struct SizeParam { + modernize::LiteralSize Size; + const char *Text; + + friend std::ostream &operator<<(std::ostream &Str, const SizeParam &Value) { + return Str << "Size: " << toString(Value.Size) << ", Text: '" << Value.Text << '\''; + } +}; + +class MatcherTest : public ::testing::TestWithParam {}; + +class SizeTest : public ::testing::TestWithParam {}; } // namespace -static const Param Params[] = { +static const MatchParam MatchParams[] = { // Accept integral literals. - {true, "1"}, - {true, "0177"}, - {true, "0xdeadbeef"}, - {true, "0b1011"}, - {true, "'c'"}, + {true, true, "1"}, + {true, true, "0177"}, + {true, true, "0xdeadbeef"}, + {true, true, "0b1011"}, + {true, true, "'c'"}, // Reject non-integral literals. - {false, "1.23"}, - {false, "0x1p3"}, - {false, R"("string")"}, - {false, "1i"}, + {true, false, "1.23"}, + {true, false, "0x1p3"}, + {true, false, R"("string")"}, + {true, false, "1i"}, // Accept literals with these unary operators. - {true, "-1"}, - {true, "+1"}, - {true, "~1"}, - {true, "!1"}, + {true, true, "-1"}, + {true, true, "+1"}, + {true, true, "~1"}, + {true, true, "!1"}, // Reject invalid unary operators. - {false, "1-"}, - {false, "1+"}, - {false, "1~"}, - {false, "1!"}, + {true, false, "1-"}, + {true, false, "1+"}, + {true, false, "1~"}, + {true, false, "1!"}, // Accept valid binary operators. - {true, "1+1"}, - {true, "1-1"}, - {true, "1*1"}, - {true, "1/1"}, - {true, "1%2"}, - {true, "1<<1"}, - {true, "1>>1"}, - {true, "1<=>1"}, - {true, "1<1"}, - {true, "1>1"}, - {true, "1<=1"}, - {true, "1>=1"}, - {true, "1==1"}, - {true, "1!=1"}, - {true, "1&1"}, - {true, "1^1"}, - {true, "1|1"}, - {true, "1&&1"}, - {true, "1||1"}, - {true, "1+ +1"}, // A space is needed to avoid being tokenized as ++ or --. - {true, "1- -1"}, - {true, "1,1"}, + {true, true, "1+1"}, + {true, true, "1-1"}, + {true, true, "1*1"}, + {true, true, "1/1"}, + {true, true, "1%2"}, + {true, true, "1<<1"}, + {true, true, "1>>1"}, + {true, true, "1<=>1"}, + {true, true, "1<1"}, + {true, true, "1>1"}, + {true, true, "1<=1"}, + {true, true, "1>=1"}, + {true, true, "1==1"}, + {true, true, "1!=1"}, + {true, true, "1&1"}, + {true, true, "1^1"}, + {true, true, "1|1"}, + {true, true, "1&&1"}, + {true, true, "1||1"}, + {true, true, "1+ +1"}, // A space is needed to avoid being tokenized as ++ or --. + {true, true, "1- -1"}, + // Comma is only valid when inside parentheses. + {true, true, "(1,1)"}, // Reject invalid binary operators. - {false, "1+"}, - {false, "1-"}, - {false, "1*"}, - {false, "1/"}, - {false, "1%"}, - {false, "1<<"}, - {false, "1>>"}, - {false, "1<=>"}, - {false, "1<"}, - {false, "1>"}, - {false, "1<="}, - {false, "1>="}, - {false, "1=="}, - {false, "1!="}, - {false, "1&"}, - {false, "1^"}, - {false, "1|"}, - {false, "1&&"}, - {false, "1||"}, - {false, "1,"}, - {false, ",1"}, + {true, false, "1+"}, + {true, false, "1-"}, + {true, false, "1*"}, + {true, false, "1/"}, + {true, false, "1%"}, + {true, false, "1<<"}, + {true, false, "1>>"}, + {true, false, "1<=>"}, + {true, false, "1<"}, + {true, false, "1>"}, + {true, false, "1<="}, + {true, false, "1>="}, + {true, false, "1=="}, + {true, false, "1!="}, + {true, false, "1&"}, + {true, false, "1^"}, + {true, false, "1|"}, + {true, false, "1&&"}, + {true, false, "1||"}, + {true, false, "1,"}, + {true, false, ",1"}, + {true, false, "1,1"}, // Accept valid ternary operators. - {true, "1?1:1"}, - {true, "1?:1"}, // A gcc extension treats x ? : y as x ? x : y. + {true, true, "1?1:1"}, + {true, true, "1?:1"}, // A gcc extension treats x ? : y as x ? x : y. // Reject invalid ternary operators. - {false, "?"}, - {false, "?1"}, - {false, "?:"}, - {false, "?:1"}, - {false, "?1:"}, - {false, "?1:1"}, - {false, "1?"}, - {false, "1?1"}, - {false, "1?:"}, - {false, "1?1:"}, + {true, false, "?"}, + {true, false, "?1"}, + {true, false, "?:"}, + {true, false, "?:1"}, + {true, false, "?1:"}, + {true, false, "?1:1"}, + {true, false, "1?"}, + {true, false, "1?1"}, + {true, false, "1?:"}, + {true, false, "1?1:"}, // Accept parenthesized expressions. - {true, "(1)"}, - {true, "((+1))"}, - {true, "((+(1)))"}, - {true, "(-1)"}, - {true, "-(1)"}, - {true, "(+1)"}, - {true, "((+1))"}, - {true, "+(1)"}, - {true, "(~1)"}, - {true, "~(1)"}, - {true, "(!1)"}, - {true, "!(1)"}, - {true, "(1+1)"}, - {true, "(1-1)"}, - {true, "(1*1)"}, - {true, "(1/1)"}, - {true, "(1%2)"}, - {true, "(1<<1)"}, - {true, "(1>>1)"}, - {true, "(1<=>1)"}, - {true, "(1<1)"}, - {true, "(1>1)"}, - {true, "(1<=1)"}, - {true, "(1>=1)"}, - {true, "(1==1)"}, - {true, "(1!=1)"}, - {true, "(1&1)"}, - {true, "(1^1)"}, - {true, "(1|1)"}, - {true, "(1&&1)"}, - {true, "(1||1)"}, - {true, "(1?1:1)"}, + {true, true, "(1)"}, + {true, true, "((+1))"}, + {true, true, "((+(1)))"}, + {true, true, "(-1)"}, + {true, true, "-(1)"}, + {true, true, "(+1)"}, + {true, true, "((+1))"}, + {true, true, "+(1)"}, + {true, true, "(~1)"}, + {true, true, "~(1)"}, + {true, true, "(!1)"}, + {true, true, "!(1)"}, + {true, true, "(1+1)"}, + {true, true, "(1-1)"}, + {true, true, "(1*1)"}, + {true, true, "(1/1)"}, + {true, true, "(1%2)"}, + {true, true, "(1<<1)"}, + {true, true, "(1>>1)"}, + {true, true, "(1<=>1)"}, + {true, true, "(1<1)"}, + {true, true, "(1>1)"}, + {true, true, "(1<=1)"}, + {true, true, "(1>=1)"}, + {true, true, "(1==1)"}, + {true, true, "(1!=1)"}, + {true, true, "(1&1)"}, + {true, true, "(1^1)"}, + {true, true, "(1|1)"}, + {true, true, "(1&&1)"}, + {true, true, "(1||1)"}, + {true, true, "(1?1:1)"}, // Accept more complicated "chained" expressions. - {true, "1+1+1"}, - {true, "1+1+1+1"}, - {true, "1+1+1+1+1"}, - {true, "1*1*1"}, - {true, "1*1*1*1"}, - {true, "1*1*1*1*1"}, - {true, "1<<1<<1"}, - {true, "4U>>1>>1"}, - {true, "1<1<1"}, - {true, "1>1>1"}, - {true, "1<=1<=1"}, - {true, "1>=1>=1"}, - {true, "1==1==1"}, - {true, "1!=1!=1"}, - {true, "1&1&1"}, - {true, "1^1^1"}, - {true, "1|1|1"}, - {true, "1&&1&&1"}, - {true, "1||1||1"}, - {true, "1,1,1"}, + {true, true, "1+1+1"}, + {true, true, "1+1+1+1"}, + {true, true, "1+1+1+1+1"}, + {true, true, "1*1*1"}, + {true, true, "1*1*1*1"}, + {true, true, "1*1*1*1*1"}, + {true, true, "1<<1<<1"}, + {true, true, "4U>>1>>1"}, + {true, true, "1<1<1"}, + {true, true, "1>1>1"}, + {true, true, "1<=1<=1"}, + {true, true, "1>=1>=1"}, + {true, true, "1==1==1"}, + {true, true, "1!=1!=1"}, + {true, true, "1&1&1"}, + {true, true, "1^1^1"}, + {true, true, "1|1|1"}, + {true, true, "1&&1&&1"}, + {true, true, "1||1||1"}, + {true, true, "(1,1,1)"}, + + // Optionally reject comma operator + {false, false, "1,1"} }; TEST_P(MatcherTest, MatchResult) { - EXPECT_TRUE(matchText(GetParam().Text) == GetParam().Matched); + const MatchParam &Param = GetParam(); + + EXPECT_TRUE(matchText(Param.Text, Param.AllowComma) == Param.Matched); } -INSTANTIATE_TEST_SUITE_P(TokenExpressionParserTests, MatcherTest, - ::testing::ValuesIn(Params)); +INSTANTIATE_TEST_SUITE_P(IntegralLiteralExpressionMatcherTests, MatcherTest, + ::testing::ValuesIn(MatchParams)); + +static const SizeParam SizeParams[] = { + {modernize::LiteralSize::Int, "1"}, + {modernize::LiteralSize::UnsignedInt, "1U"}, + {modernize::LiteralSize::Long, "1L"}, + {modernize::LiteralSize::UnsignedLong, "1UL"}, + {modernize::LiteralSize::UnsignedLong, "1LU"}, + {modernize::LiteralSize::LongLong, "1LL"}, + {modernize::LiteralSize::UnsignedLongLong, "1ULL"}, + {modernize::LiteralSize::UnsignedLongLong, "1LLU"}}; + +TEST_P(SizeTest, TokenSize) { + EXPECT_EQ(sizeText(GetParam().Text), GetParam().Size); +}; + +INSTANTIATE_TEST_SUITE_P(IntegralLiteralExpressionMatcherTests, SizeTest, + ::testing::ValuesIn(SizeParams)); } // namespace test } // namespace tidy