[PATCH] D148131: Avoid unnecessarily aggressive line-breaking when using "LambdaBodyIndentation: OuterScope" with argument bin-packing.

2023-08-30 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks accepted this revision. HazardyKnusperkeks added a comment. This revision is now accepted and ready to land. But please wait for other opinions. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D148131/new/

[PATCH] D157963: [clang-format] Annotate constructor/destructor names

2023-08-28 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks added a subscriber: kadircet. HazardyKnusperkeks added a comment. @kadircet shouldn't you at least say why it got reverted? Even better state your problem and give a chance to fix before you revert? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D158945: [clang-format] Handle Template Arguments with AlignConsecutiveXX

2023-08-26 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks created this revision. Herald added projects: All, clang, clang-format. Herald added a subscriber: cfe-commits. Herald added reviewers: rymiel, owenpan, MyDeveloperDay. HazardyKnusperkeks requested review of this revision. This fixes

[PATCH] D148131: Avoid unnecessarily aggressive line-breaking when using "LambdaBodyIndentation: OuterScope" with argument bin-packing.

2023-08-25 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks added inline comments. Comment at: clang/docs/ReleaseNotes.rst:169 all arguments being able to fit on the same line. +- Avoid unnecessarily aggressive line-breaking when using + ``LambdaBodyIndentation: OuterScope`` with argument bin-packing.

[PATCH] D158805: [clang-format][NFC] Remove extraneous newlines at end of test cases

2023-08-25 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks added inline comments. Comment at: clang/unittests/Format/FormatTest.cpp:24187 // Newlines are important here. + verifyFormat("FOO(1+2 )", Style); This. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D158505: [clang-format] Fix weird handling of AfterColon

2023-08-24 Thread Björn Schäpers via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG6e77ba6c1e4d: [clang-format] Fix weird handling of AfterColon (authored by HazardyKnusperkeks). Changed prior to commit:

[PATCH] D158505: [clang-format] Fix weird handling of AfterColon

2023-08-23 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks marked 2 inline comments as done. HazardyKnusperkeks added a comment. Will be fixed on commit. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D158505/new/ https://reviews.llvm.org/D158505

[PATCH] D158363: [clang-format] Fix segmentation fault when formatting nested namespaces

2023-08-22 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks added a comment. In D158363#4606159 , @d0nc1h0t wrote: > In D158363#4604468 , > @HazardyKnusperkeks wrote: > >> Please upload the patch with the full context. > > I'm creating a patch via 'git

[PATCH] D158505: [clang-format] Fix weird handling of AfterColon

2023-08-22 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks added inline comments. Comment at: clang/unittests/Format/FormatTest.cpp:7995 verifyFormat("Constructor(aa ,\n" - "aa ) :\n" - "

[PATCH] D158505: [clang-format] Fix weird handling of AfterColon

2023-08-22 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks created this revision. Herald added projects: All, clang, clang-format. Herald added a subscriber: cfe-commits. Herald added reviewers: rymiel, owenpan, MyDeveloperDay. HazardyKnusperkeks requested review of this revision. This fixes

[PATCH] D158363: [clang-format] Fix segmentation fault when formatting nested namespaces

2023-08-21 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks added a comment. Please upload the patch with the full context. And add a test case. (From your issue.) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D158363/new/ https://reviews.llvm.org/D158363

[PATCH] D157963: [clang-format] Annotate constructor/destructor names

2023-08-15 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks accepted this revision. HazardyKnusperkeks added inline comments. This revision is now accepted and ready to land. Comment at: clang/lib/Format/TokenAnnotator.cpp:3103 + if (!Next) +return Next; + if (Next->is(tok::comment)) Is a bit

[PATCH] D157568: [clang-format] Handle NamespaceMacro string arg for FixNamespaceComments

2023-08-13 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks accepted this revision. HazardyKnusperkeks added inline comments. This revision is now accepted and ready to land. Comment at: clang/lib/Format/NamespaceEndCommentsFixer.cpp:177 llvm::Regex("^/[/*] *(end (of )?)? *(anonymous|unnamed)? *" -

[PATCH] D156259: [clang-format] Fix a bug that erroneously placed function arguments on a new line despite all arguments being able to fit on the same line.

2023-08-03 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks added a comment. In D156259#4558570 , @jp4a50 wrote: > @HazardyKnusperkeks could you merge this for me assuming the build is green > please? I don't have merge rights. Thanks. That will have to wait, but if no one else stepped in,

[PATCH] D138263: [clang-format] Supress aligning of trailing namespace comments

2023-08-02 Thread Björn Schäpers via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. HazardyKnusperkeks marked an inline comment as done. Closed by commit rG77a38f43b168: [clang-format] Supress aligning of trailing namespace comments (authored by

[PATCH] D156228: [clang-format][NFCish] Obey debug settings

2023-08-02 Thread Björn Schäpers via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG2c3b12b5418f: [clang-format][NFCish] Obey debug settings (authored by HazardyKnusperkeks). Repository: rG LLVM Github Monorepo CHANGES SINCE

[PATCH] D156705: [clang-format] Fix braced initializer formatting with templated base class initializer

2023-07-31 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks added inline comments. Comment at: clang/unittests/Format/FormatTest.cpp:13461 " A() : a{} {}\n" + " A() : Base{} {}\n" " A(int b) : b(b) {}\n" Please also add nested templates. CHANGES SINCE LAST ACTION

[PATCH] D156259: Fix a bug that erroneously placed function arguments on a new line despite all arguments being able to fit on the same line.

2023-07-31 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks added inline comments. Comment at: clang/lib/Format/ContinuationIndenter.cpp:657-666 + bool DisallowLineBreaksOnThisLine = [ = this->Style, ] { +// Deal with lambda arguments in C++. The aim here is to ensure that we +// don't over-indent lambda

[PATCH] D138263: [clang-format] Supress aligning of trailing namespace comments

2023-07-28 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks marked an inline comment as done. HazardyKnusperkeks added inline comments. Comment at: clang/lib/Format/UnwrappedLineParser.cpp:2964-2965 if (FormatTok->is(tok::l_brace)) { +FormatToken *LBrace = FormatTok; +

[PATCH] D156259: Fix a bug that erroneously placed function arguments on a new line despite all arguments being able to fit on the same line.

2023-07-27 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks added inline comments. Comment at: clang/lib/Format/ContinuationIndenter.cpp:664-666 +if ((Style.Language == FormatStyle::LK_Cpp || + Style.Language == FormatStyle::LK_ObjC) && +!Current.is(tok::comment) && PrevNonComment &&

[PATCH] D156370: [clang-format] Fix bug with parsing of function/variable names.

2023-07-27 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks added a comment. In D156370#4537033 , @gedare wrote: > In D156370#4536793 , > @HazardyKnusperkeks wrote: > >> Does this result in a different annotation? Could you add a test for that? > > If

[PATCH] D156360: [clang-format] Support function and overloaded operator SpacesInParensOption

2023-07-27 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks added inline comments. Comment at: clang/unittests/Format/FormatTest.cpp:16786 verifyFormat("SomeType *__attribute__( ( attr ) ) *a = NULL;", Spaces); - verifyFormat("void __attribute__( ( naked ) ) foo( int bar )", Spaces); + verifyFormat("void

[PATCH] D156370: [clang-format] Fix bug with parsing of function/variable names.

2023-07-26 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks added a comment. Does this result in a different annotation? Could you add a test for that? Comment at: clang/lib/Format/TokenAnnotator.cpp:2211-2212 if (PreviousNotConst->is(tok::r_paren) && -PreviousNotConst->is(TT_TypeDeclarationParen)) { +

[PATCH] D156360: [clang-format] Support function and overloaded operator SpacesInParensOption

2023-07-26 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks added a comment. You need some tests to show that the new options really apply like intended. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D156360/new/ https://reviews.llvm.org/D156360

[PATCH] D138263: [clang-format] Supress aligning of trailing namespace comments

2023-07-26 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks added a comment. In D138263#4535269 , @MyDeveloperDay wrote: > In D138263#3936536 , > @HazardyKnusperkeks wrote: > >> In D138263#3936007 , @owenpan

[PATCH] D156259: Fix a bug that erroneously placed function arguments on a new line despite all arguments being able to fit on the same line.

2023-07-26 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks added inline comments. Comment at: clang/lib/Format/ContinuationIndenter.cpp:263 State.StartOfStringLiteral = 0; + State.NoLineBreak = false; State.StartOfLineLevel = 0; jp4a50 wrote: > HazardyKnusperkeks wrote: > > Was this

[PATCH] D156259: Fix a bug that erroneously placed function arguments on a new line despite all arguments being able to fit on the same line.

2023-07-25 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks added inline comments. Comment at: clang/lib/Format/ContinuationIndenter.cpp:263 State.StartOfStringLiteral = 0; + State.NoLineBreak = false; State.StartOfLineLevel = 0; Was this uninitialized? Comment at:

[PATCH] D156228: [clang-format][NFCish] Obey debug settings

2023-07-25 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks created this revision. HazardyKnusperkeks added a reviewer: klimek. HazardyKnusperkeks added a project: clang-format. Herald added projects: All, clang. Herald added a subscriber: cfe-commits. Herald added reviewers: rymiel, owenpan, MyDeveloperDay. HazardyKnusperkeks requested

[PATCH] D138263: [clang-format] Supress aligning of trailing namespace comments

2023-07-25 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks updated this revision to Diff 543918. HazardyKnusperkeks marked 8 inline comments as done. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D138263/new/ https://reviews.llvm.org/D138263 Files: clang/lib/Format/FormatToken.h clang/lib/Format/UnwrappedLineParser.cpp

[PATCH] D151761: clang-format: Add AlignConsecutiveShortCaseStatements

2023-07-24 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks accepted this revision. HazardyKnusperkeks added a comment. All of those variants are fine by me, but I'd stick with the current version, because it is linked to `AllowShortCaseLabelsOnASingleLine`. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D151761/new/

[PATCH] D156065: [clang-format] Insert namespace comments with leading spaces

2023-07-24 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks added inline comments. Comment at: clang/unittests/Format/NamespaceEndCommentsFixerTest.cpp:29 +FormatStyle S = Style; +S.SpacesBeforeTrailingComments = 0; tooling::Replacements Replaces = I'd rather fix all the tests.

[PATCH] D154093: [clang-format] Break long strings in Verilog

2023-07-21 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks added a comment. For the replacement part, I refer to @owenpan and/or @MyDeveloperDay hoping they have more insight. The rest look good to me. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D154093/new/

[PATCH] D155529: [clang-format] Add SpaceInParensOption for __attribute__ keyword

2023-07-20 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks accepted this revision. HazardyKnusperkeks added inline comments. This revision is now accepted and ready to land. Comment at: clang/unittests/Format/ConfigParseTest.cpp:220 CHECK_PARSE_NESTED_BOOL(SpaceBeforeParensOptions, BeforeNonEmptyParentheses); +

[PATCH] D155239: [clang-format] Add SpacesInParens with SpacesInParensOptions

2023-07-20 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks accepted this revision. HazardyKnusperkeks added a comment. This revision is now accepted and ready to land. Nice work. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D155239/new/ https://reviews.llvm.org/D155239

[PATCH] D155239: [clang-format] Add SpacesInParens with SpacesInParensOptions

2023-07-20 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks added a comment. In D155239#4520081 , @gedare wrote: > In D155239#4520067 , > @HazardyKnusperkeks wrote: > >> Everything is fine, I just need to know how the attribute stuff is formatted >>

[PATCH] D155239: [clang-format] Add SpacesInParens with SpacesInParensOptions

2023-07-20 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks added a comment. Everything is fine, I just need to know how the attribute stuff is formatted with plain LLVM style. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D155239/new/ https://reviews.llvm.org/D155239

[PATCH] D154093: [clang-format] Break long strings in Verilog

2023-07-20 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks added a comment. Apart from that I'm okay. Comment at: clang/lib/Format/WhitespaceManager.cpp:1428 + // case, it is allowed if there is a replacement for the empty range + // between 2 tokens and another non-empty range at the start of the second

[PATCH] D155783: [clang-format] Fix variable lacks of blank when previous operator is star

2023-07-20 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks added a comment. Please provide a diff with the full context (`-U99` will do the trick). And another context is needed, just `x * y` will never be (meaningful) be formatted. At least a `;` is missing, and I'd argue if your example is just ... something ... x * y;

[PATCH] D155239: [clang-format] Add SpacesInParens with SpacesInParensOptions

2023-07-19 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks requested changes to this revision. HazardyKnusperkeks added a comment. This revision now requires changes to proceed. D155529#inline-1505573 But what does `clang-format` without this patch format? I just want to make sure,

[PATCH] D138263: [clang-format] Supress aligning of trailing namespace comments

2023-07-19 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks updated this revision to Diff 541959. HazardyKnusperkeks marked 3 inline comments as done. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D138263/new/ https://reviews.llvm.org/D138263 Files: clang/lib/Format/FormatToken.h clang/lib/Format/UnwrappedLineParser.cpp

[PATCH] D155529: [clang-format] Add SpaceInParensOption for __attribute__ keyword

2023-07-18 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks added inline comments. Comment at: clang/include/clang/Format/Format.h:4208-4213 +/// Put a space in parentheses inside attribute specifier lists. +/// \code +///true: false: +///__attribute__(( noreturn

[PATCH] D155239: [clang-format] Add SpacesInParens with SpacesInParensOptions

2023-07-18 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks added a comment. If you limit it to `Never` I don't see any value in the differentiation. You could just always use `Custom` (by dropping the custom and only having the nested options). But I think having at least the `Always` option would be nice. If you want **always** to

[PATCH] D155239: [clang-format] Add SpacesInParens with SpacesInParensOptions

2023-07-15 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks added inline comments. Comment at: clang/include/clang/Format/Format.h:4220 /// \version 3.7 bool SpacesInParentheses; The deprecated options should be removed from the struct, see `AllowAllConstructorInitializersOnNextLine` for an

[PATCH] D154093: [clang-format] Break long strings in Verilog

2023-07-15 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks added inline comments. Comment at: clang/lib/Format/ContinuationIndenter.cpp:2247 bool DryRun, bool Strict) { - std::unique_ptr Token = + std::unique_ptr Token = createBreakableToken(Current, State,

[PATCH] D153589: [NFC] Initialize pointer fields and remove a needless null check.

2023-07-12 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks added a comment. Can you mark this change as closed? If you'd put "Differential Revision: https://reviews.llvm.org/D153589; in your commit message this would have happened automatically. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D153589/new/

[PATCH] D138373: [clang-format] Don't eat two semicolons after namespace

2023-07-12 Thread Björn Schäpers via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. HazardyKnusperkeks marked an inline comment as done. Closed by commit rGce7356f08194: [clang-format] Dont eat two semicolons after namespace (authored by

[PATCH] D138402: [clang-format] Correctly count a tab's width in a comment

2023-07-12 Thread Björn Schäpers via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG1116ed2beb1c: [clang-format] Correctly count a tabs width in a comment (authored by HazardyKnusperkeks). Changed prior to commit:

[PATCH] D138373: [clang-format] Don't eat two semicolons after namespace

2023-07-12 Thread Björn Schäpers via Phabricator via cfe-commits
Herald added a comment. NOTE: Clang-Format Team Automated Review Comment It looks like your clang-format review does not contain any unit tests, please try to ensure all code changes have a unit test (unless this is an `NFC` or refactoring, adding documentation etc..) Add your unit tests in

[PATCH] D154484: [clang-format] Add an option to remove redundant parentheses

2023-07-10 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks accepted this revision. HazardyKnusperkeks added a comment. In D154484#4483659 , @sstwcw wrote: > Did you forget about GNU statement expressions? > > if (({ > foo(); > bar(); > })) { > } I wouldn't haven

[PATCH] D151761: clang-format: Add AlignConsecutiveShortCaseStatements

2023-07-08 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks accepted this revision. HazardyKnusperkeks added a comment. This revision is now accepted and ready to land. Thanks for the patience, I'm really looking forward to use this. But please wait for other opinions. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D151761/new/

[PATCH] D151761: clang-format: Add AlignConsecutiveShortCaseStatements

2023-07-07 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks added inline comments. Comment at: clang/include/clang/Format/Format.h:380 +} +bool operator!=(const ShortCaseStatementsAlignmentStyle ) const { + return !(*this == R); I'd drop that. We don't have it for any other struct. And

[PATCH] D154484: [clang-format] Add an option to remove redundant parentheses

2023-07-07 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks added inline comments. Comment at: clang/unittests/Format/FormatTest.cpp:25808 +TEST_F(FormatTest, RemoveParentheses) { + FormatStyle Style = getLLVMStyle(); owenpan wrote: > HazardyKnusperkeks wrote: > > Should check for `__attribute((what

[PATCH] D154550: [clang-format] Allow empty loops on a single line.

2023-07-07 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks accepted this revision. HazardyKnusperkeks added a comment. This revision is now accepted and ready to land. Please wait for other opinions. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D154550/new/

[PATCH] D154550: [clang-format] Allow empty loops on a single line.

2023-07-06 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks added inline comments. Comment at: clang/lib/Format/Format.cpp:595-599 +IO.enumCase(Value, "Never", FormatStyle::SWFLS_Never); +IO.enumCase(Value, "false", FormatStyle::SWFLS_Never); +IO.enumCase(Value, "NonEmpty", FormatStyle::SWFLS_NonEmpty); +

[PATCH] D151761: clang-format: Add AlignConsecutiveShortCaseStatements

2023-07-06 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks added a comment. In D151761#4474136 , @galenelias wrote: > I re-wrote the alignment to stop using AlignTokens so that I can now handle > all the edge cases that came up. Specifically: > > - Allowing empty case labels (implicit fall

[PATCH] D154484: [clang-format] Add an option to remove redundant parentheses

2023-07-06 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks added inline comments. Comment at: clang/unittests/Format/FormatTest.cpp:25808 +TEST_F(FormatTest, RemoveParentheses) { + FormatStyle Style = getLLVMStyle(); Should check for `__attribute((what ever))__`. Repository: rG LLVM Github

[PATCH] D154093: [clang-format] Break long strings in Verilog

2023-07-06 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks added a comment. In D154093#4466597 , @sstwcw wrote: > In D154093#4466246 , > @HazardyKnusperkeks wrote: > >> I'd really prefer you put what you need to modify `mutable` instead of >>

[PATCH] D89918: Fix issue: clang-format result is not consistent if "// clang-format off" is followed by macro definition.

2023-07-01 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks added a comment. Give the author some time is okay, but I think we can and should commandeer at some point and abandon. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D89918/new/ https://reviews.llvm.org/D89918

[PATCH] D154093: [clang-format] Break long strings in Verilog

2023-07-01 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks added a comment. I'd really prefer you put what you need to modify `mutable` instead of removing the `const` from everything else. But that's just my opinion. Comment at: clang/include/clang/Format/Format.h:1905 + /// + /// In C: /// \code

[PATCH] D154091: [clang-format] Recognize escape sequences when breaking strings

2023-07-01 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks added inline comments. Comment at: clang/lib/Format/BreakableToken.cpp:223 - if (SpaceOffset != 0) -return BreakableToken::Split(SpaceOffset + 1, 0); + if (NewLine != 0) +return BreakableToken::Split(NewLine, 0); What if the new

[PATCH] D89918: Fix issue: clang-format result is not consistent if "// clang-format off" is followed by macro definition.

2023-06-28 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks added a comment. Is this still an issue? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D89918/new/ https://reviews.llvm.org/D89918 ___ cfe-commits mailing list

[PATCH] D153798: [clang-format] Correctly annotate operator free function call

2023-06-27 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks accepted this revision. HazardyKnusperkeks added inline comments. Comment at: clang/lib/Format/TokenAnnotator.cpp:316 + // line can't be a declaration. bool OperatorCalledAsMemberFunction = + (Prev->Previous && Nuu

[PATCH] D153641: [clang-format] Preserve AmpAmpTokenType in nested parentheses

2023-06-24 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks accepted this revision. HazardyKnusperkeks added a comment. This revision is now accepted and ready to land. In D153641#465 , @rymiel wrote: > @HazardyKnusperkeks I'm not sure why it didn't recurse already, given that > you even

[PATCH] D153579: [clang-format] Align consecutive function pointers and references

2023-06-22 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks added a comment. Could you please split this into two reviews? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D153579/new/ https://reviews.llvm.org/D153579 ___ cfe-commits mailing list

[PATCH] D153228: [clang-format] Fixed bad performance with enabled qualifier fixer.

2023-06-22 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks added inline comments. Comment at: clang/lib/Format/Format.cpp:3476-3477 + + // Regarding the 16: Note that multiple passes are added in + // addQualifierAlignmentFixerPasses(). + SmallVector Passes; owenpan wrote: > This comment is

[PATCH] D153205: [clang-format] Add new block type ListInit

2023-06-21 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks added a comment. In D153205#4437966 , @MyDeveloperDay wrote: > My only concern is that this changes behaviour without someone making the > conscious decision to make that change, now as much as I can't understand why > someone would

[PATCH] D153228: [clang-format] Fixed bad performance with enabled qualifier fixer.

2023-06-19 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks accepted this revision. HazardyKnusperkeks added a comment. This revision is now accepted and ready to land. Please wait for at least @MyDeveloperDay to say something, as he is the original author of the work. Comment at: clang/lib/Format/Format.cpp:3475

[PATCH] D153208: [clang-format] Add InsertNewlineAtEOF to .clang-format files

2023-06-19 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks added a comment. In D153208#4431697 , @owenpan wrote: > In D153208#4431239 , > @HazardyKnusperkeks wrote: > >> We also could add a clang-format style, the you wouldn't have to to touch >> all

[PATCH] D153228: [clang-format] Fixed bad performance with enabled qualifier fixer.

2023-06-18 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks added a comment. I'm shocked, astonished and in awe how small this patch is. Great work and explanation! Comment at: clang/lib/Format/Format.cpp:3475 AnalyzerPass; SmallVector Passes; Just increase here, and add a comment that

[PATCH] D153208: [clang-format] Add InsertNewlineAtEOF to .clang-format files

2023-06-18 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks accepted this revision. HazardyKnusperkeks added a comment. This revision is now accepted and ready to land. We also could add a clang-format style, the you wouldn't have to to touch all .clang-format files. ;) CHANGES SINCE LAST ACTION

[PATCH] D153205: [clang-format] Add new block type ListInit

2023-06-18 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks added a comment. In D153205#4430528 , @owenpan wrote: > It seems to me that there has been a proliferation of new options being > proposed and/or accepted recently. I'd like to remind everyone of the > long-standing policy >

[PATCH] D152975: [clang-format] Allow break after return keyword

2023-06-18 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks added a comment. In D152975#4430504 , @gedare wrote: > In D152975#4425932 , > @HazardyKnusperkeks wrote: > >> And please add a remark in the changelog. > > Is there a separate changelog file?

[PATCH] D152975: [clang-format] Allow break after return keyword

2023-06-15 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks added a comment. I'd like more tests (and examples how it is before the change), some operations (+, *, etc.), your stated string literal with and without many spaces, call chains. I have nothing against the introduction, I'm just not sure about the default value and the

[PATCH] D151761: clang-format: Add AlignConsecutiveShortCaseStatements

2023-06-13 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks added a comment. In D151761#4416224 , @galenelias wrote: > Well, I guess I didn't put quite enough thought into the `AlignCaseColons` > option. While this solves the empty case label problem, it will also match > in non-short case

[PATCH] D152443: Add operator style options to clang-format

2023-06-12 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks added inline comments. Comment at: clang/lib/Format/TokenAnnotator.cpp:4203 return true; - -if (Left.is(tok::kw_operator)) - return Right.is(tok::coloncolon); +// Operator overloads, calls and address-of expressions +if

[PATCH] D151761: clang-format: Add AlignConsecutiveShortCaseStatements

2023-06-11 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks added a comment. After I came across some of my code, where I'd really like a feature like this, I'm back at wishing to align the colons. Same as in a bit field. And when you would do that I think there would be no open issue with empty statements, right? So for my code I'd

[PATCH] D152443: Add SpaceAfterOperatorKeyword & SpaceAfterOperatorKeywordInCall style options for clang-format

2023-06-11 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks added a comment. In D152443#4412069 , @KitsuneAlex wrote: > This revision of the diff changes the names of the new style options to > **SpaceAfterOperatorOverload** and **SpaceAfterOperatorCall**, which > clarifies their scope a lot

[PATCH] D152623: [clang-format] Indent Verilog struct literal on new line

2023-06-10 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks added a comment. So I assume your `'` is a 'DictLiteral`? Does it have to be one? I don't know what else maybe a `DictLiteral` in what language and am not so comfortable with this change. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D151761: clang-format: Add AlignConsecutiveShortCaseStatements

2023-06-10 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks added a comment. In D151761#4410158 , @galenelias wrote: > Yah, I think leaving it open would be my preference at this point. Not sure > how to properly document that though? Just be explicit in the tests? > Mention it in

[PATCH] D152443: Add SpaceAfterOperatorKeyword & SpaceAfterOperatorKeywordInCall style options for clang-format

2023-06-09 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks added a comment. In D152443#4407331 , @KitsuneAlex wrote: > Applied all suggested changes and added a suiting option for aformentioned > edge-case for call expressions. > Also added the missing release notes to the apropriate

[PATCH] D152443: Add SpaceAfterOperatorKeyword style option for clang-format

2023-06-08 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks added a comment. Can you add a note to the release notes also please? Comment at: clang/include/clang/Format/Format.h:3709 + /// \version 17 + bool SpaceAfterOperatorKeyword; + O before T, please resort. Comment at:

[PATCH] D151047: [clang-format] Fix indentation for selective formatting.

2023-06-08 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks accepted this revision. HazardyKnusperkeks added inline comments. This revision is now accepted and ready to land. Comment at: clang/lib/Format/UnwrappedLineFormatter.cpp:80 + // directive, since these do not terminate a code block. + if

[PATCH] D151761: clang-format: Add AlignConsecutiveShortCaseStatements

2023-06-07 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks added a comment. In D151761#4403828 , @galenelias wrote: > In D151761#4400693 , > @HazardyKnusperkeks wrote: > >> I'd say: align it. > > If it was straightforward, I would definitely agree to

[PATCH] D151047: [clang-format] Fix indentation for selective formatting.

2023-06-07 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks added inline comments. Comment at: clang/lib/Format/UnwrappedLineFormatter.cpp:80 + // directive, since these do not terminate a code block. + if (!Line.InPPDirective && Line.Level < IndentForLevel.size()) +IndentForLevel.resize(Line.Level +

[PATCH] D151761: clang-format: Add AlignConsecutiveShortCaseStatements

2023-06-06 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks requested changes to this revision. HazardyKnusperkeks added a comment. This revision now requires changes to proceed. In D151761#4400056 , @galenelias wrote: > In D151761#4394758 , >

[PATCH] D151761: clang-format: Add AlignConsecutiveShortCaseStatements

2023-06-03 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks accepted this revision. HazardyKnusperkeks added a comment. This revision is now accepted and ready to land. Please wait for some other approval (or comments) for a few days. Comment at: clang/unittests/Format/FormatTest.cpp:19218 + // Verify comments and

[PATCH] D152051: libclang-cpp: Add external visibility attribute to all classes

2023-06-03 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks added inline comments. Comment at: clang/include/clang/Format/Format.h:4532 /// Returns ``true`` if the Style has been set. -bool getPredefinedStyle(StringRef Name, FormatStyle::LanguageKind Language, +LLVM_EXTERNAL_VISIBILITY bool

[PATCH] D151761: clang-format: Add AlignConsecutiveShortCaseStatements

2023-06-03 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks added inline comments. Comment at: clang/unittests/Format/FormatTest.cpp:19217 + + // Verify comments and empty lines break the alignment + verifyFormat("switch (level) {\n" Comment at:

[PATCH] D151761: clang-format: Add AlignConsecutiveShortCaseLabels

2023-06-01 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks added a comment. In D151761#4385571 , @galenelias wrote: > In D151761#4385163 , > @HazardyKnusperkeks wrote: > >> When I read the title I thought "Yay, some one is doing what I want and >>

[PATCH] D151761: clang-format: Add AlignConsecutiveShortCaseLabels

2023-05-31 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks added a comment. When I read the title I thought "Yay, some one is doing what I want and don't find the time.", but (for me) sadly you're not. I'd like to align the colon (and thus the statement behind that). Do you think you can add that option too?

[PATCH] D151145: Add disabled unittest reproducing TextProto formatting issue.

2023-05-25 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks added a comment. > In D151145#4365580 , > @HazardyKnusperkeks wrote: > >> As stated on Github I don't think we have a bug. > > Are you sure, I'm pretty sure this is a bug? Can you elaborate on the > discussion on github. I wouldn't

[PATCH] D151047: [clang-format] Fix indentation for selective formatting.

2023-05-25 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks added a comment. In D151047#4369742 , @Sedeniono wrote: > To create a new fix, do I understand the guide > correctly that > I basically execute `arc diff --verbatim` and

[PATCH] D151047: [clang-format] Fix indentation for selective formatting.

2023-05-24 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks added a comment. In D151047#4369503 , @MyDeveloperDay wrote: > I think we need to extract the context of the test from RenameTests to ensure > we have it covered here. I don't personally normally run the entire LLVM > suite. Dito,

[PATCH] D151047: [clang-format] Fix indentation for selective formatting.

2023-05-23 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks added a comment. In D151047#4363127 , @owenpan wrote: > In D151047#4361205 , @Sedeniono > wrote: > >> Also, should it be merged into the LLVM 16.x branch? > > +1. @MyDeveloperDay and

[PATCH] D151145: Add disabled unittest reproducing TextProto formatting issue.

2023-05-23 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks added reviewers: MyDeveloperDay, owenpan, rymiel, HazardyKnusperkeks. HazardyKnusperkeks added a comment. As stated on Github I don't think we have a bug. Comment at: clang/unittests/Format/FormatTestRawStrings.cpp:79 +)format"; +llvm::MemoryBufferRef

[PATCH] D151047: [clang-format] Fix indentation for selective formatting.

2023-05-22 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks accepted this revision. HazardyKnusperkeks added a comment. Just a side note, please link to commits, or even better the reviews here when talking about old commits. I found it through the github issue, but I think the other way around is better. I was interested in the

[PATCH] D150614: [clang-format] Ignore first token when finding MustBreak

2023-05-17 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks accepted this revision. HazardyKnusperkeks added inline comments. Comment at: clang/unittests/Format/FormatTest.cpp:20136 "#ifdef _DEBUG\n" - "void bar()\n" - " {\n" - " }\n" + "void

[PATCH] D150403: [clang-format] Adjust braced list detection (try 2)

2023-05-16 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks added a comment. In D150403#4343708 , @galenelias wrote: > Thanks @HazardyKnusperkeks! I don't have commit access, so will need someone > to land this for me. We'll wait a bit, if someone might have a comment. And (at least I) need

[PATCH] D150614: [clang-format] Ignore first token when finding MustBreak

2023-05-16 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks added inline comments. Comment at: clang/unittests/Format/FormatTest.cpp:20136 "#ifdef _DEBUG\n" - "void bar()\n" - " {\n" - " }\n" + "void bar() {}\n" "#else\n"

[PATCH] D150629: [clang-format] Don't allow template to be preceded by closing brace

2023-05-16 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks added inline comments. Comment at: clang/unittests/Format/FormatTest.cpp:10682 verifyFormat("a < 0 ? b : a > 0 ? c : d;"); + verifyFormat("ratio{-1, 2} < ratio{-1, 3} == -1 / 3 > -1 / 2;"); verifyFormat("void f() {\n" owenpan wrote: >

  1   2   3   4   5   6   7   8   9   10   >