This is an archive of the discontinued LLVM Phabricator instance.

Differential D150539

[clang-format] Handle <chrono> ud suffixes in IntegerLiteralSeparator
ClosedPublic

Authored by owenpan on May 14 2023, 11:09 PM.

Details

Reviewers
rymiel
HazardyKnusperkeks
MyDeveloperDay
Commits
rGa72b064acf95: [clang-format] Handle <chrono> ud suffixes in IntegerLiteralSeparator
Summary

Fixes https://github.com/llvm/llvm-project/issues/62679.

Diff Detail

Event Timeline

owenpan created this revision.May 14 2023, 11:09 PM
Herald added projects: Restricted Project, Restricted Project, Restricted Project. · View Herald TranscriptMay 14 2023, 11:09 PM
Herald added reviewers: rymiel, HazardyKnusperkeks, MyDeveloperDay. · View Herald Transcript
Herald added a subscriber: cfe-commits. · View Herald Transcript
owenpan requested review of this revision.May 14 2023, 11:09 PM
Harbormaster completed remote builds in B231908: Diff 522054.May 15 2023, 12:05 AM
HazardyKnusperkeks accepted this revision.May 15 2023, 1:14 PM
HazardyKnusperkeks added inline comments.
clang/lib/Format/IntegerLiteralSeparatorFixer.cpp
116–121

What about this?

This revision is now accepted and ready to land.May 15 2023, 1:14 PM
owenpan added inline comments.May 15 2023, 5:55 PM
clang/lib/Format/IntegerLiteralSeparatorFixer.cpp
116–121

Good idea!

owenpan updated this revision to Diff 522397.May 15 2023, 5:58 PM

Addressed the review comment.

owenpan requested review of this revision.May 15 2023, 5:58 PM
owenpan marked an inline comment as done.
Harbormaster completed remote builds in B232165: Diff 522397.May 15 2023, 8:19 PM
MyDeveloperDay accepted this revision.May 16 2023, 2:28 AM
This revision is now accepted and ready to land.May 16 2023, 2:28 AM
HazardyKnusperkeks accepted this revision.May 16 2023, 12:09 PM
Closed by commit rGa72b064acf95: [clang-format] Handle <chrono> ud suffixes in IntegerLiteralSeparator (authored by owenpan). · Explain WhyMay 16 2023, 1:18 PM
This revision was automatically updated to reflect the committed changes.
owenpan added a commit: rGa72b064acf95: [clang-format] Handle <chrono> ud suffixes in IntegerLiteralSeparator.

Revision Contents

PathSize
clang/
lib/
Format/
6 lines
unittests/
Format/
18 lines

Diff 522756

clang/lib/Format/IntegerLiteralSeparatorFixer.cpp

Show First 20 Lines • Show All 107 Lines • ▼ Show 20 Lines for (bool Skip = false; !Lex.LexFromRawLexer(Tok);) {
const bool IsBase2 = B == Base::Binary; const bool IsBase2 = B == Base::Binary;
const bool IsBase10 = B == Base::Decimal; const bool IsBase10 = B == Base::Decimal;
const bool IsBase16 = B == Base::Hex; const bool IsBase16 = B == Base::Hex;
if ((IsBase2 && SkipBinary) || (IsBase10 && SkipDecimal) || if ((IsBase2 && SkipBinary) || (IsBase10 && SkipDecimal) ||
(IsBase16 && SkipHex) || B == Base::Other) { (IsBase16 && SkipHex) || B == Base::Other) {
continue; continue;
} }
if (Style.isCpp()) { if (Style.isCpp()) {
if (const auto Pos = Text.find_first_of("_i"); Pos != StringRef::npos) { // Hex alpha digits a-f/A-F must be at the end of the string literal.
StringRef Suffixes = "_himnsuyd";
if (const auto Pos =
Text.find_first_of(IsBase16 ? Suffixes.drop_back() : Suffixes);
Pos != StringRef::npos) {
Text = Text.substr(0, Pos); Text = Text.substr(0, Pos);
HazardyKnusperkeksUnsubmitted
Done
ReplyInline Actions
if (Style.isCpp()) {
- // FIXME: This doesn't work for ud-suffix d from std::chrono::day.
- if (const auto Pos = Text.find_first_of("_himnsuy");
+ // Suffixes not applicable to hex numbers have to be at the end.
+ StringRef suffixes = "_himnsuyd";
+ if (IsBase16)
+ suffixes.drop_back();
+ if (const auto Pos = Text.find_first_of(suffixes);
Pos != StringRef::npos) {

What about this?

HazardyKnusperkeks: What about this?
owenpanAuthorUnsubmitted
Done
ReplyInline Actions

Good idea!

owenpan: Good idea!
Length = Pos; Length = Pos;
} }
} }
if ((IsBase10 && Text.find_last_of(".eEfFdDmM") != StringRef::npos) || if ((IsBase10 && Text.find_last_of(".eEfFdDmM") != StringRef::npos) ||
(IsBase16 && Text.find_last_of(".pP") != StringRef::npos)) { (IsBase16 && Text.find_last_of(".pP") != StringRef::npos)) {
continue; continue;
} }
const auto Start = Text[0] == '0' ? 2 : 0; const auto Start = Text[0] == '0' ? 2 : 0;
▲ Show 20 Lines • Show All 92 Lines • Show Last 20 Lines

clang/unittests/Format/IntegerLiteralSeparatorTest.cpp

Show First 20 Lines • Show All 54 Lines • ▼ Show 20 LinesTEST_F(IntegerLiteralSeparatorTest, SingleQuoteAsSeparator) {
verifyFormat("bi = 0b1'0000i;\n" verifyFormat("bi = 0b1'0000i;\n"
"dif = 1'234if;\n" "dif = 1'234if;\n"
"hil = 0xA'BCil;", "hil = 0xA'BCil;",
"bi = 0b10000i;\n" "bi = 0b10000i;\n"
"dif = 1234if;\n" "dif = 1234if;\n"
"hil = 0xABCil;", "hil = 0xABCil;",
Style); Style);
verifyFormat("bd = 0b1'0000d;\n"
"dh = 1'234h;\n"
"dmin = 1'234min;\n"
"dns = 1'234ns;\n"
"ds = 1'234s;\n"
"dus = 1'234us;\n"
"hy = 0xA'BCy;",
"bd = 0b10000d;\n"
"dh = 1234h;\n"
"dmin = 1234min;\n"
"dns = 1234ns;\n"
"ds = 1234s;\n"
"dus = 1234us;\n"
"hy = 0xABCy;",
Style);
verifyFormat("hd = 0xAB'Cd;", "hd = 0xABCd;", Style);
verifyFormat("d = 5'678_km;\n" verifyFormat("d = 5'678_km;\n"
"h = 0xD'EF_u16;", "h = 0xD'EF_u16;",
"d = 5678_km;\n" "d = 5678_km;\n"
"h = 0xDEF_u16;", "h = 0xDEF_u16;",
Style); Style);
} }
TEST_F(IntegerLiteralSeparatorTest, UnderscoreAsSeparator) { TEST_F(IntegerLiteralSeparatorTest, UnderscoreAsSeparator) {
▲ Show 20 Lines • Show All 158 Lines • Show Last 20 Lines