[PATCH] D94955: [clang-format] Treat ForEachMacros as loops

2022-01-14 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius added inline comments. Comment at: clang/unittests/Format/FormatTest.cpp:2209 + + FormatStyle ShortBlocks = getLLVMStyle(); + ShortBlocks.AllowShortBlocksOnASingleLine = FormatStyle::SBS_Always; If wanted, I might commit the new tests without ForEachMa

[PATCH] D117197: [clang-format] Add new option to support adding a space between Javascript Union and Intersection types

2022-01-14 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius accepted this revision. curdeius added a comment. This revision is now accepted and ready to land. Looks ok from my side. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D117197/new/ https://reviews.llvm.org/D117197 ___ cfe-commits mail

[PATCH] D99031: [clang-format] Fix CompactNamespaces corner case when AllowShortLambdasOnASingleLine/BraceWrapping.BeforeLambdaBody are set

2022-01-14 Thread Marek Kurdej via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG6ea3d9efc536: [clang-format] Fix CompactNamespaces corner case when… (authored by curdeius). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D99031/new/ https:

[PATCH] D117142: [clang-format] Fix short functions being considered as inline inside an indented namespace.

2022-01-14 Thread Marek Kurdej via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG7af11989be21: [clang-format] Fix short functions being considered as inline inside an… (authored by curdeius). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D

[PATCH] D117414: [clang-format] Add return code to git-clang-format

2022-01-16 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius accepted this revision. curdeius added a comment. This revision is now accepted and ready to land. LGTM! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D117414/new/ https://reviews.llvm.org/D117414 __

[PATCH] D117416: [clang-format] Handle C variables with name that matches c++ access specifier

2022-01-16 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius added a comment. Does it fix https://github.com/llvm/llvm-project/issues/46915? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D117416/new/ https://reviews.llvm.org/D117416 ___ cfe-commits mailing

[PATCH] D117416: [clang-format] Handle C variables with name that matches c++ access specifier

2022-01-16 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius requested changes to this revision. curdeius added a comment. Thanks for having a try on this. However, I don't like this approach too much. You add many changes and a single test. That's not sufficient. Also, handling C++ keywords in all cases (e.g. `delete` as a function name) *may* n

[PATCH] D117421: [clang-format] Fix incorrect alignment of operator= overloads.

2022-01-16 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius added a comment. Thanks for working on this! I'll have a closer look tomorrow or on Tuesday. Comment at: clang/lib/Format/WhitespaceManager.cpp:735 +// Do not align operator= overloads. +if (C.Tok->Previous && C.Tok->Previous->is(tok::kw_operator)) { +

[PATCH] D115060: [clang-format][NFC] Code Tidies in UnwrappedLineFormatter

2022-01-17 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius accepted this revision. curdeius added a comment. LGTM. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D115060/new/ https://reviews.llvm.org/D115060 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin

[PATCH] D117421: [clang-format] Fix incorrect alignment of operator= overloads.

2022-01-17 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius added a comment. Could you check if your patch fixes https://github.com/llvm/llvm-project/issues/33044 as well? If so, please add tests. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D117421/new/ https://reviews.llvm.org/D117421 _

[PATCH] D117421: [clang-format] Fix incorrect alignment of operator= overloads.

2022-01-17 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius added a comment. With your patch I still see the bug in this case: struct S { constexpr S(const S &) = default; void f() = default; S &operator/**/=(S) {} }; Mind the `operator =`. It happens every time that `operator=` has only declaration (no def

[PATCH] D117421: [clang-format] Fix incorrect alignment of operator= overloads.

2022-01-17 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius added inline comments. Comment at: clang/lib/Format/WhitespaceManager.cpp:735 +// Do not align operator= overloads. +if (C.Tok->Previous && C.Tok->Previous->is(tok::kw_operator)) { + FormatToken *Next = C.Tok->Next; This should f

[PATCH] D117421: [clang-format] Fix incorrect alignment of operator= overloads.

2022-01-17 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius added inline comments. Comment at: clang/lib/Format/WhitespaceManager.cpp:735-742 +if (C.Tok->Previous && C.Tok->Previous->is(tok::kw_operator)) { + FormatToken *Next = C.Tok->Next; + while (Next && Next->NewlinesBefore == 0) { +if (

[PATCH] D94955: [clang-format] Treat ForEachMacros as loops

2022-01-17 Thread Marek Kurdej 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 rG1e512f022ad5: [clang-format] Treat ForEachMacros as loops (authored by curdeius). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D117398: [clang-format] Fix bug in parsing `operator<` with template

2022-01-17 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius added inline comments. Comment at: clang/lib/Format/FormatTokenLexer.cpp:432 + auto Forth = (Tokens.end() - 4)[0]; bool FourthTokenIsLess = false; MyDeveloperDay wrote: > isn't this going to crash if Tokens.size() is 3? It probably will. Anyway, it

[PATCH] D117421: [clang-format] Fix incorrect alignment of operator= overloads.

2022-01-17 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius added a comment. In D117421#3249126 , @glotchimo wrote: > I was tinkering with the use of `getPreviousNonComment` last night before > signing off and the problem that I noticed was that, though it stops the > `operator=` from being split and al

[PATCH] D115168: [clang-format] [PR49298] Sort includes pass will sort inside raw strings

2021-12-06 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius added inline comments. Comment at: clang/lib/Format/Format.cpp:2597 +// or we will sort the contents of the string. +// Skip past until we think we we the rawstring literal close. +if (Trimmed.contains("R\"(")) { "we we"? C

[PATCH] D115168: [clang-format] [PR49298] Sort includes pass will sort inside raw strings

2021-12-06 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius added inline comments. Comment at: clang/lib/Format/Format.cpp:2598 +// Skip past until we think we we the rawstring literal close. +if (Trimmed.contains("R\"(")) { + FormattingOff = true; HazardyKnusperkeks wrote: > curdeius wrote: > > This

[PATCH] D115168: [clang-format] [PR49298] Sort includes pass will sort inside raw strings

2021-12-08 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius requested changes to this revision. curdeius added inline comments. This revision now requires changes to proceed. Comment at: clang/lib/Format/Format.cpp:2589 + llvm::Regex RawStringRegex("R\"([A-Za-z]*)\\("); + SmallVector RawStringMatches; A raw s

[PATCH] D115050: [clang-format] PR48916 PointerAlignment not working when using C++20 init-statement in for loop

2021-12-08 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius added inline comments. Comment at: clang/unittests/Format/FormatTest.cpp:1947 verifyFormat("int&& c = f3();", Style); + verifyFormat("for (auto a = 0, b = 0; const auto& c : {1, 2, 3})", Style); curdeius wrote: > How about pointers/references in th

[PATCH] D115050: [clang-format] PR48916 PointerAlignment not working when using C++20 init-statement in for loop

2021-12-08 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius requested changes to this revision. curdeius added a comment. This revision now requires changes to proceed. Requesting changes so that appears correctly in the review queue. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D115050/new/ https://reviews.llvm.org/D115050 ___

[PATCH] D115050: [clang-format] PR48916 PointerAlignment not working when using C++20 init-statement in for loop

2021-12-08 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius added inline comments. Comment at: clang/unittests/Format/FormatTest.cpp:1947 verifyFormat("int&& c = f3();", Style); + verifyFormat("for (auto a = 0, b = 0; const auto& c : {1, 2, 3})", Style); HazardyKnusperkeks wrote: > curdeius wrote: > > curde

[PATCH] D115050: [clang-format] PR48916 PointerAlignment not working when using C++20 init-statement in for loop

2021-12-09 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius accepted this revision. curdeius added a comment. This revision is now accepted and ready to land. LGTM! Thanks. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D115050/new/ https://reviews.llvm.org/D115050 ___ cfe-commits mailing list

[PATCH] D115168: [clang-format] [PR49298] Sort includes pass will sort inside raw strings

2021-12-09 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius added inline comments. Comment at: clang/lib/Format/Format.cpp:2590 + llvm::Regex RawStringRegex( + "R\"([A-Za-z0-9_{}#<>%:;.?*+/^&\\$|~!=,'\\-]*)\\("); + SmallVector RawStringMatches; I'm picky but you missed `[` and `]`, no? C

[PATCH] D115168: [clang-format] [PR49298] Sort includes pass will sort inside raw strings

2021-12-09 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius added inline comments. Comment at: clang/lib/Format/Format.cpp:2590 + llvm::Regex RawStringRegex( + "R\"([A-Za-z0-9_{}#<>%:;.?*+/^&\\$|~!=,'\\-]*)\\("); + SmallVector RawStringMatches; MyDeveloperDay wrote: > curdeius wrote: > > I'm picky but you

[PATCH] D115168: [clang-format] [PR49298] Sort includes pass will sort inside raw strings

2021-12-10 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius accepted this revision. curdeius added a comment. This revision is now accepted and ready to land. LGTM. Great! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D115168/new/ https://reviews.llvm.org/D115168 ___ cfe-commits mailing list c

[PATCH] D115625: [clang-format] add support for cppm files

2021-12-13 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius accepted this revision. curdeius added a comment. LGTM. Comment at: clang/tools/clang-format/clang-format-diff.py:51 r'.*\.(cpp|cc|c\+\+|cxx|c|cl|h|hh|hpp|hxx|m|mm|inc|js|ts' - r'|proto|protodevel|java|cs|json)', +

[PATCH] D115647: [clang-format] FixNamespaceComments does not understand namespace aliases

2021-12-13 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius added inline comments. Comment at: clang/lib/Format/NamespaceEndCommentsFixer.cpp:27 // Returns "" for anonymous namespace. -std::string computeName(const FormatToken *NamespaceTok) { +bool computeName(const FormatToken *NamespaceTok, std::string &name) { assert(Name

[PATCH] D115647: [clang-format] FixNamespaceComments does not understand namespace aliases

2021-12-13 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius accepted this revision. curdeius added a comment. This revision is now accepted and ready to land. LGTM! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D115647/new/ https://reviews.llvm.org/D115647 ___ cfe-commits mailing list cfe-comm

[PATCH] D115673: [clang-format] C# switch expression formatting differs from normal switch formatting

2021-12-13 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius accepted this revision. curdeius added a comment. This revision is now accepted and ready to land. LGTM. AFAIK (which is very limited when it comes to C#), the cases can have also other expressions, not only ints and _. But that can be left for a different patch. Example from https://d

[PATCH] D115715: [clang-tidy] Fix llvm-header-guard for Windows paths containing drive letter (e.g. C:).

2021-12-14 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius created this revision. curdeius added reviewers: aaron.ballman, hokein, bkramer, salman-javed-nz, alexfh. Herald added subscribers: carlosgalvezp, xazax.hun. curdeius requested review of this revision. Herald added a project: clang-tools-extra. Herald added a subscriber: cfe-commits. Fix

[PATCH] D115715: [clang-tidy] Fix llvm-header-guard for Windows paths containing drive letter (e.g. C:).

2021-12-14 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius added a comment. In D115715#3191742 , @sberg wrote: >> So the check, for a file called e.g. "C:\test\test.h" would suggest the >> guard C:_TEST_TEST_H being an invalid name due to the presence of the colon. > > ...though the new suggestion `C__T

[PATCH] D115794: [clang-format] put non-empty catch block on one line with AllowShortBlocksOnASingleLine: Empty

2021-12-15 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius accepted this revision. curdeius added a comment. LGTM. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D115794/new/ https://reviews.llvm.org/D115794 ___ cfe-commits mailing list cfe-commits@lists.

[PATCH] D115803: [clang-format] Fix tabs when using BreakBeforeTernaryOperators=false.

2021-12-15 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius created this revision. curdeius added reviewers: MyDeveloperDay, HazardyKnusperkeks, owenpan. curdeius requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Fixes https://github.com/llvm/llvm-project/issues/52724. This is rather a worka

[PATCH] D115803: [clang-format] Fix tabs when using BreakBeforeTernaryOperators=false.

2021-12-15 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius added a comment. In D115803#3195115 , @MyDeveloperDay wrote: > So am I right in thinking the got replaced out because C.Spaces was > > C.StartOfTokenColumn? (i.e. appendIndentText was given a negative number as > the 4th argument) Yes, that

[PATCH] D115803: [clang-format] Fix tabs when using BreakBeforeTernaryOperators=false.

2021-12-16 Thread Marek Kurdej 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 rG27818f01fec2: [clang-format] Fix tabs when using BreakBeforeTernaryOperators=false. (authored by curdeius). Repository: rG LLVM Github Monorepo C

[PATCH] D115865: [clang-format] add support for branch attribute macros

2021-12-16 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius accepted this revision. curdeius added a comment. This revision is now accepted and ready to land. LGTM! Tested locally. Has Bolt landed? It seems that the CI adds bolt as the project but it isn't recognized... Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https:/

[PATCH] D115879: [clang-format] extern with new line brace without indentation

2021-12-16 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius requested changes to this revision. curdeius added inline comments. This revision now requires changes to proceed. Comment at: clang/unittests/Format/FormatTest.cpp:3815 + + Style.IndentExternBlock = FormatStyle::IEBS_AfterExternBlock; + Style.BreakBeforeBraces = Forma

[PATCH] D115903: [clang-format] Extra spaces surrounding arrow in templated member call in variable decl

2021-12-16 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius added inline comments. Comment at: clang/lib/Format/TokenAnnotator.cpp:1682 Current.NestingLevel == 0 && - !Current.Previous->is(tok::kw_operator)) { + !Current.Previous->isOneOf(tok::kw_operator, tok::identifier)) { //

[PATCH] D115903: [clang-format] Extra spaces surrounding arrow in templated member call in variable decl

2021-12-17 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius accepted this revision. curdeius added a comment. This revision is now accepted and ready to land. LGTM. Comment at: clang/lib/Format/TokenAnnotator.cpp:1682 Current.NestingLevel == 0 && - !Current.Previous->is(tok::kw_operator)) { +

[PATCH] D115879: [clang-format] extern with new line brace without indentation

2021-12-17 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius added inline comments. Comment at: clang/unittests/Format/FormatTest.cpp:3819 + Style.IndentExternBlock = FormatStyle::IEBS_NoIndent; + verifyFormat("extern \"C\"\n{ /*13*/\n}", Style); + verifyFormat("extern \"C\"\n{\n" owenpan wrote: > curdeius wrot

[PATCH] D115879: [clang-format] extern with new line brace without indentation

2021-12-17 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius accepted this revision. curdeius added a comment. This revision is now accepted and ready to land. LGTM! Comment at: clang/lib/Format/UnwrappedLineParser.cpp:1285-1287 +if (Style.BraceWrapping.AfterExternBlock) { + addUnwrappedLine(); } --

[PATCH] D115938: [clang-format] Formatter does not handle c++11 string literal prefix with stringize #

2021-12-17 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius added a comment. When at it, should we also take care of `LR"(string)"`, `uR...`, `u8R` and `UR`? Cf. https://en.cppreference.com/w/cpp/language/string_literal From MS doc: // Raw string literals containing unescaped \ and " auto R0 = R"("Hello \ world")"; // const char* auto R1

[PATCH] D115938: [clang-format] Formatter does not handle c++11 string literal prefix with stringize #

2021-12-17 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius added a comment. You shouldn't have added the outer quotes as `#str` adds them. This works: // clang-format off #define MyRawString(str) R#str void foo() { const auto * s1 = MyRawString((" Hello \ world ")); const auto * s2 = MyRawString(abc(" Hello \ world ")abc);

[PATCH] D115938: [clang-format] Formatter does not handle c++11 string literal prefix with stringize #

2021-12-17 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius accepted this revision. curdeius added a comment. This revision is now accepted and ready to land. LGTM. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D115938/new/ https://reviews.llvm.org/D115938 ___ cfe-commits mailing list cfe-comm

[PATCH] D115968: [clang-format] Refactor common handling of attributes. NFC.

2021-12-17 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius created this revision. curdeius added a reviewer: MyDeveloperDay. curdeius requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D115968 Files: clang/lib/Format/Unwrapped

[PATCH] D115968: [clang-format] Refactor common handling of attributes. NFC.

2021-12-17 Thread Marek Kurdej 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 rG9cf4b7266bbf: [clang-format] Refactor common handling of attributes. NFC. (authored by curdeius). Repository: rG LLVM Github Monorepo CHANGES SIN

[PATCH] D115972: [clang-format] Fix AlignConsecutiveAssignments breaking lambda formatting.

2021-12-17 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius created this revision. curdeius added reviewers: MyDeveloperDay, HazardyKnusperkeks, owenpan. curdeius requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Fixes https://github.com/llvm/llvm-project/issues/52772. This patch fixes the f

[PATCH] D115972: [clang-format] Fix AlignConsecutiveAssignments breaking lambda formatting.

2021-12-17 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius planned changes to this revision. curdeius added a comment. Actually, I've just run it on a larger codebase and found that this patch misbehaves. Need to rework and add faulty test cases. Sorry for the noise. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://re

[PATCH] D115990: AlignConsecutiveDeclarations not working for 'const' keyword in JavsScript

2021-12-19 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius accepted this revision. curdeius added a comment. This revision is now accepted and ready to land. LGTM! Optional nit comment. Comment at: clang/lib/Format/TokenAnnotator.cpp:1875 +// const a = in JavaScript. +if (Style.isJavaScript() && PreviousNotConst->is(to

[PATCH] D112019: [clang-format] [PR51412] AlignConsecutiveMacros fights with Visual Studio and resource.h

2021-12-19 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius added a comment. Please review the descriptions. Looks good, but I'm wondering if there are better names for these options. E.g. when `AlignConsecutiveMacrosIgnoreMax` is true, then `AlignConsecutiveMacrosMinWidth` is not minimum width, but required/fixed width except for long macro nam

[PATCH] D116007: [clang-format] Fix BreakBeforeBraces: Attach ignored with trailing requires-expression of function.

2021-12-19 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius created this revision. curdeius added reviewers: MyDeveloperDay, HazardyKnusperkeks, owenpan. curdeius requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Fixes https://github.com/llvm/llvm-project/issues/48312. Repository: rG LLVM

[PATCH] D116008: [clang-format] Fix wrong indentation of namespace identifiers after a concept declaration.

2021-12-19 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius created this revision. curdeius added reviewers: MyDeveloperDay, HazardyKnusperkeks, owenpan. curdeius requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Before this patch, the code: template concept a_concept = X<>; namespace

[PATCH] D116008: [clang-format] Fix wrong indentation of namespace identifiers after a concept declaration.

2021-12-19 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius updated this revision to Diff 395355. curdeius added a comment. Undo unrelated changes. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D116008/new/ https://reviews.llvm.org/D116008 Files: clang/lib/Format/UnwrappedLineParser.cpp clang/u

[PATCH] D116008: [clang-format] Fix wrong indentation of namespace identifiers after a concept declaration.

2021-12-20 Thread Marek Kurdej 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 rG960712ccc710: [clang-format] Fix wrong indentation of namespace identifiers after a concept… (authored by curdeius). Repository: rG LLVM Github Mo

[PATCH] D116007: [clang-format] Fix BreakBeforeBraces: Attach ignored with trailing requires-expression of function.

2021-12-20 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius added a comment. Ok, I'll have a look on it. Thanks for reporting. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D116007/new/ https://reviews.llvm.org/D116007 ___ cfe-commits mailing list cfe-com

[PATCH] D116049: [clang-format] Fix SplitEmptyRecord affecting SplitEmptyFunction.

2021-12-20 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius created this revision. curdeius added reviewers: MyDeveloperDay, HazardyKnusperkeks, owenpan. curdeius requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Fixes https://github.com/llvm/llvm-project/issues/50051. Given the style: Br

[PATCH] D115990: AlignConsecutiveDeclarations not working for 'const' keyword in JavsScript

2021-12-20 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius added inline comments. Comment at: clang/lib/Format/TokenAnnotator.cpp:1875 +// const a = in JavaScript. +if (Style.isJavaScript() && PreviousNotConst->is(tok::kw_const)) + return true; owenpan wrote: > MyDeveloperDay wrote: > > owenpan wrot

[PATCH] D116049: [clang-format] Fix SplitEmptyRecord affecting SplitEmptyFunction.

2021-12-21 Thread Marek Kurdej 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 rG07fe45130546: [clang-format] Fix SplitEmptyRecord affecting SplitEmptyFunction. (authored by curdeius). Changed prior to commit: https://reviews.l

[PATCH] D116183: [clang-format] Fix wrong indentation after trailing requires clause.

2021-12-22 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius created this revision. curdeius added reviewers: MyDeveloperDay, HazardyKnusperkeks, owenpan. curdeius requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Fixes https://github.com/llvm/llvm-project/issues/52834. Before this patch, cla

[PATCH] D116183: [clang-format] Fix wrong indentation after trailing requires clause.

2021-12-22 Thread Marek Kurdej 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 rGf66d602c3f58: [clang-format] Fix wrong indentation after trailing requires clause. (authored by curdeius). Repository: rG LLVM Github Monorepo CH

[PATCH] D116188: [clang-format] Fix short enums getting wrapped even when denied

2021-12-23 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius accepted this revision. curdeius added a comment. LGTM. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D116188/new/ https://reviews.llvm.org/D116188 ___ cfe-commits mailing list cfe-commits@lists.

[PATCH] D116229: [clang-format] Add option to align pointers in C style casts differently

2021-12-23 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius added a comment. Why this option is useful? Why would someone want to have a different alignment in casts than in other places? As such I'm opposed to introducing one more option. Is there any well established code style that has something like this? Repository: rG LLVM Github Monor

[PATCH] D116290: [clang-format] Add enforcement of consistent `class`/typename` keyword for template arguments

2021-12-28 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius added a comment. Looks nice already. Thanks for working on this. Comment at: clang/include/clang/Format/Format.h:3691 + /// COULD lead to incorrect code formatting due to incorrect decisions made + /// due to clang-formats lack of complete semantic information. As

[PATCH] D116318: [clang-format][NFC] Fix a bug in getPreviousToken() in the parser

2021-12-28 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius requested changes to this revision. curdeius added a comment. This revision now requires changes to proceed. Wasn't assert enough? Could you add a test with code that provokes the problem? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D11631

[PATCH] D116316: [clang-format] Add an experimental option to remove optional control statement braces in LLVM C++ code

2021-12-28 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius added a comment. Could you have a look at preceding reviews and see if there wasn't a similar patch before? I think that this option is a bit too limited. Only removing braces doesn't seem enough. Also, one should probably be able to decide when to add/remove them by e.g. setting the nu

[PATCH] D116314: [Clangfmt] Add style to separate definition blocks

2021-12-28 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius requested changes to this revision. curdeius added a comment. This revision now requires changes to proceed. Thanks for working on this. Please add unit tests that verify the expected behaviour. Keep them simple. You can inspire yourself from existing tests. Comment a

[PATCH] D116290: [clang-format] Add enforcement of consistent `class`/typename` keyword for template arguments

2021-12-28 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius added a comment. Thanks for addressing the comments quickly. I'll have one more thorough look in the coming days. Comment at: clang/lib/Format/TemplateArgumentKeywordFixer.cpp:55 +// For `auto` language version, be conservative and assume we are < C++17 +KeepT

[PATCH] D116283: [clang-format] Add an option to add a space between operator overloading and opening parentheses

2021-12-28 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius added inline comments. Comment at: clang/docs/ClangFormatStyleOptions.rst:3755 + * ``bool AfterOperatorOverloading`` If ``true``, put a space between operator overloading and opening parentheses. + I'm not fond of the current name, exactly the "Overl

[PATCH] D116318: [clang-format][NFC] Fix a bug in getPreviousToken() in the parser

2021-12-29 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius added a comment. I'm in the same position as @hazardyknusperkeks. If you need something to simplify the code you can add a helper `getPreviousTokenOrNull` or something like that in your patch. But we certainly don't want to pay for the `if` check each time we call `getPreviousToken`.

[PATCH] D116318: [clang-format][NFC] Fix a bug in getPreviousToken() in the parser

2021-12-29 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius added a comment. On another note, you can fix the comment typo without a review. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D116318/new/ https://reviews.llvm.org/D116318 ___ cfe-commits mailin

[PATCH] D116314: [clang-format] Add style to separate definition blocks

2021-12-29 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius requested changes to this revision. curdeius added a comment. This revision now requires changes to proceed. Nice job. Some minor comments. Please don't forget to reformat too. Comment at: clang/docs/ClangFormatStyleOptions.rst:3415 + + SeparateDefinitions + Ne

[PATCH] D72326: [clang-format] Rebased on master: Add option to specify explicit config file

2021-12-29 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius accepted this revision. curdeius added a comment. This revision is now accepted and ready to land. LGTM but please wait for the green light from other involved reviewers. Also, as noted in a comment, I'd like to see unrelated changes in a different patch (unless you convince me that it's

[PATCH] D72326: [clang-format] Rebased on master: Add option to specify explicit config file

2021-12-29 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius added a comment. Oh, and please rename the patch before landing. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72326/new/ https://reviews.llvm.org/D72326 ___ cfe-commits mailing list cfe-commits

[PATCH] D116290: [clang-format] Add enforcement of consistent `class`/typename` keyword for template arguments

2021-12-29 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius added inline comments. Comment at: clang/lib/Format/TemplateArgumentKeywordFixer.cpp:55 +// For `auto` language version, be conservative and assume we are < C++17 +KeepTemplateTemplateKW = (Style.Standard == FormatStyle::LS_Auto) || +

[PATCH] D116170: [clang-format] Add penalty for breaking after '('

2021-12-29 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius accepted this revision. curdeius added a comment. LGTM. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D116170/new/ https://reviews.llvm.org/D116170 ___ cfe-commits mailing list cfe-commits@lists.

[PATCH] D98433: [clang] [C++2b] [P1102] Accept lambdas without parameter list ().

2021-03-16 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius updated this revision to Diff 330901. curdeius marked 4 inline comments as done. curdeius added a comment. - Add test. - Remove unnecessary PrototypeScope.exit(). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D98433/new/ https://reviews.llv

[PATCH] D98433: [clang] [C++2b] [P1102] Accept lambdas without parameter list ().

2021-03-16 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius added a comment. Thank you for the review Aaron. I hope having addressed your comments so far. I'm not sure for the prototype scope though. Comment at: clang/lib/Parse/ParseExprCXX.cpp:1440 + } else if (getLangOpts().CPlusPlus2b) { +ParseScope PrototypeScope(this,

[PATCH] D98237: [clang-format] Option for empty lines after an access modifier.

2021-03-16 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius added a comment. That looks really good. Please add tests as indicated in the inline comments and fix the formatting (see clang-format warnings). Comment at: clang/include/clang/Format/Format.h:1912 +/// Keep existing empty lines after access modifiers. +/// M

[PATCH] D93938: [clang-format] Fixed AfterEnum handling

2021-03-16 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius added inline comments. Comment at: clang/lib/Format/TokenAnnotator.cpp:3683 +Style.BraceWrapping.AfterEnum; +bool isLineTooBig = (strlen(Right.TokenText.data()) + + Right.OriginalColumn) > Style.ColumnLimit; atirit wro

[PATCH] D98433: [clang] [C++2b] [P1102] Accept lambdas without parameter list ().

2021-03-16 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius added a comment. That's my question as well to be honest. I don't know what's the policy on that. Anyway, it will simplify the code a bit I think. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D98433/new/ https://reviews.llvm.org/D98433 __

[PATCH] D98429: [clang-format] Add new option to clang-format: SpaceBeforeForLoopSemiColon

2021-03-16 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius requested changes to this revision. curdeius added a comment. Could you point me to a style guide that formats the code in this way please? Comment at: clang/include/clang/Format/Format.h:2838 + /// If ``false``, spaces will be removed before semi-colons in for loop

[PATCH] D98433: [clang] [C++2b] [P1102] Accept lambdas without parameter list ().

2021-03-17 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius planned changes to this revision. curdeius added a comment. I've checked and GCC accepts lambdas without parens (but with specifiers) in all standard modes, giving a warning pre-C++2b. I guess it's reasonable to align the behaviour. Cf. https://github.com/gcc-mirror/gcc/commit/0f161cc84

[PATCH] D93938: [clang-format] Fixed AfterEnum handling

2021-03-17 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius requested changes to this revision. curdeius added inline comments. This revision now requires changes to proceed. Comment at: clang/lib/Format/TokenAnnotator.cpp:3679-3680 +if (remainingFile[whileIndex] != '\n' && +(remainingFile[whileIndex] == ' ' &

[PATCH] D93938: [clang-format] Fixed AfterEnum handling

2021-03-17 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius added inline comments. Comment at: clang/lib/Format/TokenAnnotator.cpp:3679-3680 +if (remainingFile[whileIndex] != '\n' && +(remainingFile[whileIndex] == ' ' && + remainingFile[whileIndex - 1] == ' ')) { + remainingLineCharCount++

[PATCH] D98433: [clang] [C++2b] [P1102] Accept lambdas without parameter list ().

2021-03-19 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius updated this revision to Diff 331795. curdeius added a comment. - Accept but warn in pre-C++2b mode when lambda is missing '()' and have lambda-specifiers. - Fix tests. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D98433/new/ https://revi

[PATCH] D98433: [clang] [C++2b] [P1102] Accept lambdas without parameter list ().

2021-03-22 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius added a comment. I'll fix the formatting with the next update. Comment at: clang/lib/Parse/ParseExprCXX.cpp:1445-1461 + unsigned TokKind = 0; + switch (Tok.getKind()) { + case tok::kw_mutable: TokKind = 0; break; + case tok::arrow: TokKind = 1; brea

[PATCH] D98433: [clang] [C++2b] [P1102] Accept lambdas without parameter list ().

2021-03-22 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius updated this revision to Diff 332340. curdeius added a comment. - Use a generic message. - Fix formatting. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D98433/new/ https://reviews.llvm.org/D98433 Files: clang/include/clang/Basic/Diagnos

[PATCH] D98433: [clang] [C++2b] [P1102] Accept lambdas without parameter list ().

2021-03-23 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius updated this revision to Diff 332558. curdeius added a comment. - Format. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D98433/new/ https://reviews.llvm.org/D98433 Files: clang/include/clang/Basic/DiagnosticParseKinds.td clang/lib/Pars

[PATCH] D98433: [clang] [C++2b] [P1102] Accept lambdas without parameter list ().

2021-03-24 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius updated this revision to Diff 332904. curdeius added a comment. - Address review comments (newline + remove diagnostic). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D98433/new/ https://reviews.llvm.org/D98433 Files: clang/include/clang

[PATCH] D98433: [clang] [C++2b] [P1102] Accept lambdas without parameter list ().

2021-03-24 Thread Marek Kurdej via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG0620e6f4b76a: [clang] [C++2b] [P1102] Accept lambdas without parameter list (). (authored by curdeius). Changed prior to commit: https://reviews.llvm.org/D98433?vs=332904&id=332967#toc Repository: rG

[PATCH] D99458: [clang-format] Fix east const pointer alignment of operators

2021-03-27 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius accepted this revision. curdeius added a comment. This revision is now accepted and ready to land. LGTM. But please wait for @mydeveloperday's review too. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D99458/new/ https://reviews.llvm.org/D9

[PATCH] D99458: [clang-format] Fix east const pointer alignment of operators

2021-03-27 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius added inline comments. Comment at: clang/lib/Format/TokenAnnotator.cpp:2958 // Space between the type and the * in: // operator void*() // operator char*() Nit: you might add `operator void const/volatile*()` to the list. Repository:

[PATCH] D99489: [clang] [PR49736] [C++2b] Correctly reject lambdas with requires clause and no parameter list

2021-03-29 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius created this revision. curdeius added reviewers: aaron.ballman, rsmith. curdeius requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. This fixes http://llvm.org/PR49736 caused by implementing http://wg21.link/P1102 (https://reviews.ll

[PATCH] D99458: [clang-format] Fix east const pointer alignment of operators

2021-03-29 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius added a comment. Thinking out loud, do we test `volatile` at all? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D99458/new/ https://reviews.llvm.org/D99458 ___ cfe-commits mailing list cfe-commit

[PATCH] D99489: [clang] [PR49736] [C++2b] Correctly reject lambdas with requires clause and no parameter list

2021-03-29 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius added inline comments. Comment at: clang/lib/Parse/ParseExprCXX.cpp:1435 + +WarnIfHasCUDATargetAttr(); } else if (Tok.isOneOf(tok::kw_mutable, tok::arrow, tok::kw___attribute, aaron.ballman wrote: > Rather than add this in both branches, I'd say t

[PATCH] D99489: [clang] [PR49736] [C++2b] Correctly reject lambdas with requires clause and no parameter list

2021-03-29 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius updated this revision to Diff 333910. curdeius added a comment. - Hoist common code. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D99489/new/ https://reviews.llvm.org/D99489 Files: clang/lib/Parse/ParseExprCXX.cpp clang/test/Parser/cx

[PATCH] D99489: [clang] [PR49736] [C++2b] Correctly reject lambdas with requires clause and no parameter list

2021-03-30 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius updated this revision to Diff 334064. curdeius marked 2 inline comments as done. curdeius added a comment. - Clean up tests. - Fix grammar comment. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D99489/new/ https://reviews.llvm.org/D99489 F

[PATCH] D99489: [clang] [PR49736] [C++2b] Correctly reject lambdas with requires clause and no parameter list

2021-03-30 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius added inline comments. Comment at: clang/test/Parser/cxx2a-template-lambdas.cpp:17 +#endif +auto L7 = [] requires true {}; // ? +auto L8 = [] requires true noexcept {}; aaron.ballman wrote: > This seems grammatically valid to me, was there a reason for t

[PATCH] D99489: [clang] [PR49736] [C++2b] Correctly reject lambdas with requires clause and no parameter list

2021-03-30 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius updated this revision to Diff 334075. curdeius added a comment. - Rebase to fix CI. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D99489/new/ https://reviews.llvm.org/D99489 Files: clang/lib/Parse/ParseExprCXX.cpp clang/test/Parser/cxx

<    3   4   5   6   7   8   9   >