[PATCH] D140543: [clang-format] Add an option to format integer literal separators

Owen Pan via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Dec 29 03:43:31 PST 2022


owenpan requested review of this revision.
owenpan added a comment.

In D140543#4016184 <https://reviews.llvm.org/D140543#4016184>, @thakis wrote:

> In D140543#4016181 <https://reviews.llvm.org/D140543#4016181>, @owenpan wrote:
>
>> In D140543#4016156 <https://reviews.llvm.org/D140543#4016156>, @thakis wrote:
>>
>>> This seems to break tests everywhere, eg http://45.33.8.238/linux/95289/step_7.txt
>>>
>>> Please take a look and revert for now if it takes a while to fix.
>>
>> I had run FormatTests on Windows and macOS without any problems and don't understand why the build bots failed. I will disable the `FixRanges` test as a workaround.
>
> FWIW it also fails on Windows and macOS on my bots: http://45.33.8.238/macm1/51645/step_7.txt http://45.33.8.238/win/72399/step_7.txt
>
> If the test is failing, why not revert the commit for now instead of disabling the test? That's what we usually do.

I couldn't reproduce the failure until I ran the unit tests in the //Release// build. You are right though that I should have just reverted the commit.



================
Comment at: clang/lib/Format/IntegerLiteralSeparatorFixer.cpp:81-82
+
+  llvm::SpecificBumpPtrAllocator<Token> Allocator;
+  auto Tok = new (Allocator.Allocate()) Token;
+  Lex->LexFromRawLexer(*Tok);
----------------
I should allocate memory for the `Token` object as shown but instead had `Token Tok;` before.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D140543/new/

https://reviews.llvm.org/D140543



More information about the cfe-commits mailing list