Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -25,10 +25,11 @@ struct IntegerLiteralCheck {
static constexpr llvm::StringLiteral Name = llvm::StringLiteral("integer");
// What should be skipped before looking for the Suffixes? (Nothing here.)
static constexpr llvm::StringLiteral SkipFirst = llvm::StringLiteral("");
// Suffix can only consist of 'u' and 'l' chars, and can be a complex number
// ('i', 'j'). In MS compatibility mode, suffixes like i32 are supported.
// Suffix can only consist of 'u', 'l', and 'z' chars, can be a bit-precise
// integer (wb), and can be a complex number ('i', 'j'). In MS compatibility
// mode, suffixes like i32 are supported.
static constexpr llvm::StringLiteral Suffixes =
llvm::StringLiteral("uUlLiIjJ");
llvm::StringLiteral("uUlLzZwWbBiIjJ");
};
constexpr llvm::StringLiteral IntegerLiteralCheck::Name;
constexpr llvm::StringLiteral IntegerLiteralCheck::SkipFirst;
Expand All @@ -45,10 +46,10 @@ struct FloatingLiteralCheck {
// Since the exponent ('p'/'P') is mandatory for hexadecimal floating-point
// literals, we first skip everything before the exponent.
static constexpr llvm::StringLiteral SkipFirst = llvm::StringLiteral("pP");
// Suffix can only consist of 'f', 'l', "f16", 'h', 'q' chars,
// and can be a complex number ('i', 'j').
// Suffix can only consist of 'f', 'l', "f16", "bf16", "df", "dd", "dl",
// 'h', 'q' chars, and can be a complex number ('i', 'j').
static constexpr llvm::StringLiteral Suffixes =
llvm::StringLiteral("fFlLhHqQiIjJ");
llvm::StringLiteral("fFlLbBdDhHqQiIjJ");
};
constexpr llvm::StringLiteral FloatingLiteralCheck::Name;
constexpr llvm::StringLiteral FloatingLiteralCheck::SkipFirst;
Expand Down
4 changes: 4 additions & 0 deletions clang-tools-extra/docs/ReleaseNotes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -354,6 +354,10 @@ Changes in existing checks
<clang-tidy/checks/readability/redundant-smartptr-get>` check by fixing
some false positives involving smart pointers to arrays.

- Improved :doc:`readability-uppercase-literal-suffix
<clang-tidy/checks/readability/uppercase-literal-suffix>` check to recognize
literal suffixes added in C++23 and C23.

Removed checks
^^^^^^^^^^^^^^

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,162 @@
// TODO: When Clang adds support for decimal floating point types, enable these tests by:
// 1. Removing all the #if 0 + #endif guards.
// 2. Removing all occurrences of the string "DISABLED-" in this file.
// 3. Deleting this message.

// RUN: %check_clang_tidy %s readability-uppercase-literal-suffix %t -- -- -std=c23
// RUN: grep -Ev "// *[A-Z-]+:" %s > %t.c
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we need grep? What if build env doesn't have it, e.g. Windows

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Our win tests may have grep but downstream users may not

// RUN: clang-tidy %t.c -checks='-*,readability-uppercase-literal-suffix' -fix -- -std=c23
// RUN: clang-tidy %t.c -checks='-*,readability-uppercase-literal-suffix' -warnings-as-errors='-*,readability-uppercase-literal-suffix' -- -std=c23

void bit_precise_literal_suffix() {
// _BitInt()

static constexpr auto v1 = 1wb;
// CHECK-MESSAGES: :[[@LINE-1]]:30: warning: integer literal has suffix 'wb', which is not uppercase
// CHECK-MESSAGES-NEXT: static constexpr auto v1 = 1wb;
// CHECK-MESSAGES-NEXT: ^~~
// CHECK-MESSAGES-NEXT: WB{{$}}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need these messages? I haven't seen them in other tests and they look obsolete (considering we have CHECK-FIXES).

Copy link
Contributor Author

@localspook localspook Aug 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm parroting the existing uppercase-literal-suffix-* tests here. I agree that they seem unnecessary; should I go and remove them from the existing tests too? (ditto for grep, it looks like it could just be a cp)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think yes, remove unnecessary CHECK-FIXES-NEXT.
But i still don't understand what we need cp for. Could we just make two separate test files instead of creating one on the fly?

Copy link
Contributor Author

@localspook localspook Aug 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The idea is that we copy the test, run the check and apply fixes, then run the check again to make sure there are no more warnings. (I’m not sure how much value this adds on top of the usual CHECK-FIXES. I wouldn’t be against removing it too.) But making a separate file would cause duplication: we would need to check in both the original and the fixed file, right?

Copy link
Contributor

@vbvictor vbvictor Aug 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for description! A separate file won't do much, and I don't see any value in running check multiple times over corrected file, so I vote to remove it. If this thing is ever needed, I think it should go to check_clang_tidy.py as an opt-in for tests.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The only benefit I see is to don't write GOOD cases. So the workflow would be like:

  1. Write only "bad" cases in test file
  2. First run corrects that they are all become "good"
  3. Second run checks that none of "good" are not flagged as FP.

So since we write all good cases explicitly (I hope so) then we don't need double-run.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this thing is ever needed, I think it should go to check_clang_tidy.py as an opt-in for tests.

+1

I’ll go and remove it from this and existing tests

// CHECK-FIXES: static constexpr auto v1 = 1WB;
static_assert(v1 == 1WB);

static constexpr auto v2 = 1WB; // OK.
static_assert(v2 == 1WB);

// _BitInt() Unsigned

static constexpr auto v3 = 1wbu;
// CHECK-MESSAGES: :[[@LINE-1]]:30: warning: integer literal has suffix 'wbu', which is not uppercase
// CHECK-MESSAGES-NEXT: static constexpr auto v3 = 1wbu;
// CHECK-MESSAGES-NEXT: ^~~~
// CHECK-MESSAGES-NEXT: WBU{{$}}
// CHECK-FIXES: static constexpr auto v3 = 1WBU;
static_assert(v3 == 1WBU);

static constexpr auto v4 = 1WBu;
// CHECK-MESSAGES: :[[@LINE-1]]:30: warning: integer literal has suffix 'WBu', which is not uppercase
// CHECK-MESSAGES-NEXT: static constexpr auto v4 = 1WBu;
// CHECK-MESSAGES-NEXT: ^~~~
// CHECK-MESSAGES-NEXT: WBU{{$}}
// CHECK-FIXES: static constexpr auto v4 = 1WBU;
static_assert(v4 == 1WBU);

static constexpr auto v5 = 1wbU;
// CHECK-MESSAGES: :[[@LINE-1]]:30: warning: integer literal has suffix 'wbU', which is not uppercase
// CHECK-MESSAGES-NEXT: static constexpr auto v5 = 1wbU;
// CHECK-MESSAGES-NEXT: ^~~~
// CHECK-MESSAGES-NEXT: WBU{{$}}
// CHECK-FIXES: static constexpr auto v5 = 1WBU;
static_assert(v5 == 1WBU);

static constexpr auto v6 = 1WBU; // OK.
static_assert(v6 == 1WBU);

// Unsigned _BitInt()

static constexpr auto v7 = 1uwb;
// CHECK-MESSAGES: :[[@LINE-1]]:30: warning: integer literal has suffix 'uwb', which is not uppercase
// CHECK-MESSAGES-NEXT: static constexpr auto v7 = 1uwb;
// CHECK-MESSAGES-NEXT: ^~~~
// CHECK-MESSAGES-NEXT: UWB{{$}}
// CHECK-FIXES: static constexpr auto v7 = 1UWB;
static_assert(v7 == 1UWB);

static constexpr auto v8 = 1uWB;
// CHECK-MESSAGES: :[[@LINE-1]]:30: warning: integer literal has suffix 'uWB', which is not uppercase
// CHECK-MESSAGES-NEXT: static constexpr auto v8 = 1uWB;
// CHECK-MESSAGES-NEXT: ^~~~
// CHECK-MESSAGES-NEXT: UWB{{$}}
// CHECK-FIXES: static constexpr auto v8 = 1UWB;
static_assert(v8 == 1UWB);

static constexpr auto v9 = 1Uwb;
// CHECK-MESSAGES: :[[@LINE-1]]:30: warning: integer literal has suffix 'Uwb', which is not uppercase
// CHECK-MESSAGES-NEXT: static constexpr auto v9 = 1Uwb;
// CHECK-MESSAGES-NEXT: ^~~~
// CHECK-MESSAGES-NEXT: UWB{{$}}
// CHECK-FIXES: static constexpr auto v9 = 1UWB;
static_assert(v9 == 1UWB);

static constexpr auto v10 = 1UWB; // OK.
static_assert(v10 == 1UWB);
}

void decimal_floating_point_suffix() {
// _Decimal32

#if 0
static constexpr auto v1 = 1.df;
// DISABLED-CHECK-MESSAGES: :[[@LINE-1]]:30: warning: floating point literal has suffix 'df', which is not uppercase
// DISABLED-CHECK-MESSAGES-NEXT: static constexpr auto v1 = 1.df;
// DISABLED-CHECK-MESSAGES-NEXT: ^ ~
// DISABLED-CHECK-MESSAGES-NEXT: DF{{$}}
// DISABLED-CHECK-FIXES: static constexpr auto v1 = 1.DF;
static_assert(v1 == 1.DF);

static constexpr auto v2 = 1.e0df;
// DISABLED-CHECK-MESSAGES: :[[@LINE-1]]:30: warning: floating point literal has suffix 'df', which is not uppercase
// DISABLED-CHECK-MESSAGES-NEXT: static constexpr auto v2 = 1.e0df;
// DISABLED-CHECK-MESSAGES-NEXT: ^ ~
// DISABLED-CHECK-MESSAGES-NEXT: DF{{$}}
// DISABLED-CHECK-FIXES: static constexpr auto v2 = 1.e0DF;
static_assert(v2 == 1.DF);

static constexpr auto v3 = 1.DF; // OK.
static_assert(v3 == 1.DF);

static constexpr auto v4 = 1.e0DF; // OK.
static_assert(v4 == 1.DF);
#endif

// _Decimal64

#if 0
static constexpr auto v5 = 1.dd;
// DISABLED-CHECK-MESSAGES: :[[@LINE-1]]:30: warning: floating point literal has suffix 'dd', which is not uppercase
// DISABLED-CHECK-MESSAGES-NEXT: static constexpr auto v5 = 1.dd;
// DISABLED-CHECK-MESSAGES-NEXT: ^ ~
// DISABLED-CHECK-MESSAGES-NEXT: DD{{$}}
// DISABLED-CHECK-FIXES: static constexpr auto v5 = 1.DD;
static_assert(v5 == 1.DD);

static constexpr auto v6 = 1.e0dd;
// DISABLED-CHECK-MESSAGES: :[[@LINE-1]]:30: warning: floating point literal has suffix 'dd', which is not uppercase
// DISABLED-CHECK-MESSAGES-NEXT: static constexpr auto v6 = 1.e0dd;
// DISABLED-CHECK-MESSAGES-NEXT: ^ ~
// DISABLED-CHECK-MESSAGES-NEXT: DD{{$}}
// DISABLED-CHECK-FIXES: static constexpr auto v6 = 1.e0DD;
static_assert(v6 == 1.DD);

static constexpr auto v7 = 1.DD; // OK.
static_assert(v7 == 1.DD);

static constexpr auto v8 = 1.e0DD; // OK.
static_assert(v8 == 1.DD);
#endif

// _Decimal128

#if 0
static constexpr auto v9 = 1.dl;
// DISABLED-CHECK-MESSAGES: :[[@LINE-1]]:30: warning: floating point literal has suffix 'dl', which is not uppercase
// DISABLED-CHECK-MESSAGES-NEXT: static constexpr auto v9 = 1.dl;
// DISABLED-CHECK-MESSAGES-NEXT: ^ ~
// DISABLED-CHECK-MESSAGES-NEXT: DL{{$}}
// DISABLED-CHECK-FIXES: static constexpr auto v9 = 1.DL;
static_assert(v9 == 1.DL);

static constexpr auto v10 = 1.e0dl;
// DISABLED-CHECK-MESSAGES: :[[@LINE-1]]:31: warning: floating point literal has suffix 'dl', which is not uppercase
// DISABLED-CHECK-MESSAGES-NEXT: static constexpr auto v10 = 1.e0dl;
// DISABLED-CHECK-MESSAGES-NEXT: ^ ~
// DISABLED-CHECK-MESSAGES-NEXT: DL{{$}}
// DISABLED-CHECK-FIXES: static constexpr auto v10 = 1.e0DL;
static_assert(v10 == 1.DL);

static constexpr auto v11 = 1.DL; // OK.
static_assert(v11 == 1.DL);

static constexpr auto v12 = 1.e0DL; // OK.
static_assert(v12 == 1.DL);
#endif
}
Loading
Loading