[PATCH] D121069: [clang-format] Minimize the damage caused by AlignArrayOfStructures when working on non square arrays

2022-03-06 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. NOTE: I've tried to collate all the reported crashing examples and run this fix through them (both Left and Right) all pass except this one below void foo() { auto thing = test{ { {something}, //A } }; } This is not related to i

[PATCH] D121069: [clang-format] Minimize the damage caused by AlignArrayOfStructures when working on non square arrays

2022-03-06 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay updated this revision to Diff 413301. MyDeveloperDay marked 2 inline comments as done. MyDeveloperDay added a comment. There were no checks for if the cell would run off the end of the CellDescs structure, this is part of the reason for the crashes that the PrevIter->Index becomes

[PATCH] D121069: [clang-format] Minimize the damage caused by AlignArrayOfStructures when working on non square arrays

2022-03-06 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added inline comments. Comment at: clang/lib/Format/WhitespaceManager.cpp:1118 Changes[CellIter->Index].Spaces = CellDescs.InitialSpaces; ++CellIter; + for (auto i = 1U; i < CellDescs.CellCounts[0]; i++, ++CellIter) { I don't understand in

[PATCH] D121069: [clang-format] Minimize the damage caused by AlignArrayOfStructures when working on non square arrays

2022-03-06 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. @feg208 I could do with some clarity on the algorithm, am I correct in thinking it requires that the first row, contain at least the maximum number of columns for the rest of the array. I sort of noticed it was fine if the first row was larger { {1,2,3 }

[PATCH] D121069: [clang-format] Minimize the damage caused by AlignArrayOfStructures when working on non square arrays

2022-03-06 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. In D121069#3362448 , @feg208 wrote: >> This seems like a reasonable compromise I kind of feel this might be the best initial option, I don't want to disable it completely because it looks like people are trying to use it,

[PATCH] D121132: [clang-format] Handle C# 9 `init` accessor specifier.

2022-03-07 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay accepted this revision. MyDeveloperDay added a comment. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D121132/new/ https://reviews.llvm.org/D121132 ___ cfe-commits mailing list cfe-com

[PATCH] D91949: [clang-format] Add BeforeStructInitialization option in BraceWrapping configuration

2021-04-26 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added inline comments. Comment at: clang/lib/Format/UnwrappedLineParser.cpp:2705 } - if (FormatTok->Tok.is(tok::l_brace)) { + if (FormatTok->Tok.is(tok::equal)) { +nextToken(); shouldn't this be ```FormatTok->Tok.is(tok::equal) && Style.

[PATCH] D101344: [clang-format] Add `SpacesInAngles: Leave` option to keep spacing inside angle brackets as is.

2021-04-28 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay accepted this revision. MyDeveloperDay added a comment. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D101344/new/ https://reviews.llvm.org/D101344 ___ cfe-commits mailing list cfe-com

[PATCH] D101702: [clang-format] Add more support for C# 8 nullables

2021-05-03 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. Thank you for the patch Comment at: clang/lib/Format/TokenAnnotator.cpp:3197 +Right.is(TT_CSharpNullCoalescingAssignment)) + return true; + should this honour SpaceBeforeAssignmentOperators ? Repository: rG LLVM

[PATCH] D101628: [Format] Don't sort includes if DisableFormat is true

2021-05-03 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay accepted this revision. MyDeveloperDay added a comment. This revision is now accepted and ready to land. I've seen this asked for multiple time, I think this is fine. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D101628/new/ https://

[PATCH] D100727: [clang-format] Add options to AllowShortIfStatementsOnASingleLine to apply to "else if" and "else".

2021-05-03 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. Still looks good to me. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D100727/new/ https://reviews.llvm.org/D100727 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D101860: [clang-format] Fix C# nullable-related errors

2021-05-05 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay accepted this revision. MyDeveloperDay added a comment. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D101860/new/ https://reviews.llvm.org/D101860 ___ cfe-commits mailing list cfe-com

[PATCH] D91949: [clang-format] Add BeforeStructInitialization option in BraceWrapping configuration

2021-05-08 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. I'd like @curdeius input, my concern is its too simplistic and won't cover all the uses cases if (FormatTok->Tok.is(tok::equal) && Style.BraceWrapping.BeforeStructInitialization) { nextToken(); addUnwrappedLine(); } else if (FormatTok->To

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

2022-01-01 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay accepted this revision. MyDeveloperDay added a comment. LGTM thank you for this patch, I'll also definately use this myself. Comment at: clang/docs/ClangFormatStyleOptions.rst:3398 +**SeparateDefinitionBlocks** (``SeparateDefinitionStyle``) :versionbadge:`clan

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

2022-01-02 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay accepted this revision. MyDeveloperDay added a comment. This revision is now accepted and ready to land. LGTM, could you add a release note into docs/ReleaseNotes.rst? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D116283/new/ https:/

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

2022-01-02 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. Can you add a release note to docs/ReleaseNotes.rst CHANGES SINCE LAST ACTION https://reviews.llvm.org/D116314/new/ https://reviews.llvm.org/D116314 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lis

[PATCH] D116494: [clang-format] spacesRequiredBetween is not honouring clang-format off/on

2022-01-02 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay created this revision. MyDeveloperDay added reviewers: curdeius, HazardyKnusperkeks, owenpan, lahwaacz. MyDeveloperDay added projects: clang, clang-format. MyDeveloperDay requested review of this revision. https://github.com/llvm/llvm-project/issues/52881 It seems that clang-format

[PATCH] D116494: [clang-format] spacesRequiredBetween is not honouring clang-format off/on

2022-01-02 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay updated this revision to Diff 396929. MyDeveloperDay added a comment. Switch to verifyFormat in the tests CHANGES SINCE LAST ACTION https://reviews.llvm.org/D116494/new/ https://reviews.llvm.org/D116494 Files: clang/lib/Format/TokenAnnotator.cpp clang/unittests/Format/Form

[PATCH] D116527: [clang-format] Fix indentation for array variables with alignment of consecutive assignments and declarations.

2022-01-03 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. The fix looks ok, butr I don't understand the cause of the side effect? is it explicitly aligning to the v of `auto v`? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D116527/new/ https://reviews.llvm.org/D116527 ___

[PATCH] D106349: [clang-format] respect AfterEnum for enums

2022-01-03 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. no objection from me, we need to clean up the non landed accepted reviews or it wastes our time in the first place. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D106349/new/ https://reviews.llvm.org/D106349 __

[PATCH] D116494: [clang-format] spacesRequiredBetween is not honouring clang-format off/on

2022-01-03 Thread MyDeveloperDay 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 rGcd2b050fa499: [clang-format] spacesRequiredBetween is not honouring clang-format off/on (authored by MyDeveloperDay). Repository: rG LLVM Github M

[PATCH] D111545: [Clang][NFC] Fix multiline comment prefixes in function headers

2022-01-04 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. That is more correct, but actually, you normally make the comment match the param name (I think there might even be a clang-tidy check for that?) i.e. /*isBitCodeSDL=*/ Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.or

[PATCH] D116527: [clang-format] Fix indentation for array variables with alignment of consecutive assignments and declarations.

2022-01-04 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. This is born out by setting ContinuationIndentWidth: 8 int i = 0; float i2 = 0; auto v = type{ i = 1, // (i = 2), // i = 3// }; So maybe the original test is correct.. Repository: rG LLVM Github Monorepo CHANGE

[PATCH] D116527: [clang-format] Fix indentation for array variables with alignment of consecutive assignments and declarations.

2022-01-04 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. Could we resolve this with just // Continued braced list. if (Changes[ScopeStart - 2].Tok->isNot(tok::identifier) && Changes[ScopeStart - 1].Tok->is(tok::l_brace) && Changes[i].Tok->isNot(tok::r_brace)) return true; Repository: rG LLVM Githu

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

2022-01-04 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. I'm seeing some odd behaviour here.. void foo() {} /* ABC */ void bar() {} becomes void foo() {} /* ABC */ void bar() {} If the ABC comment is a doxygen style comment it doesn't make sense to separate it from the function below Repository:

[PATCH] D116592: [clang-format] Missing space after cast in a macro

2022-01-04 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay created this revision. MyDeveloperDay added reviewers: curdeius, HazardyKnusperkeks, owenpan. MyDeveloperDay added projects: clang, clang-format. MyDeveloperDay requested review of this revision. https://github.com/llvm/llvm-project/issues/52979 Though SpaceAfterCStyleCast is set t

[PATCH] D116592: [clang-format] Missing space after cast in a macro

2022-01-04 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added inline comments. Comment at: clang/lib/Format/TokenAnnotator.cpp:1887 + // parentheses, look past it as these might be chained casts. + if (LeftOfParens->is(tok::r_paren) && !LeftOfParens->is(TT_CastRParen)) { if (!LeftOfParens->MatchingPar

[PATCH] D116638: [ClangFormat] Fix ignoring JavaScriptWrapImport when ColumnWidth: 0

2022-01-05 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. Thank you for the patch, could you just handle the documentation, but otherwise pretty good Comment at: clang/docs/ClangFormatStyleOptions.rst:2820 +**JavaScriptWrapImports** (``Boolean``) :versionbadge:`clang-format 3.9` + Whether to wrap Ja

[PATCH] D116592: [clang-format] Missing space after cast in a macro

2022-01-05 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay updated this revision to Diff 397473. MyDeveloperDay added a comment. Fix nit and add a few more tests just to be sure CHANGES SINCE LAST ACTION https://reviews.llvm.org/D116592/new/ https://reviews.llvm.org/D116592 Files: clang/lib/Format/TokenAnnotator.cpp clang/unittest

[PATCH] D116638: [clang-format] Fix ignoring JavaScriptWrapImport when ColumnWidth: 0

2022-01-05 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added inline comments. Comment at: clang/unittests/Format/FormatTestJS.cpp:1948 + Style.JavaScriptWrapImports = false; verifyFormat("import {VeryLongImportsAreAnnoying, VeryLongImportsAreAnnoying," " VeryLongImportsAreAnnoying, VeryLongImportsA

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

2022-01-05 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. @ksyx Did you see this issue https://github.com/llvm/llvm-project/issues/52976 In its current form, this patch causes huge amounts of flux in projects that document code like (anyone using doxygen likely, + most normal people ;-) ) ... } // My function -

[PATCH] D116638: [clang-format] Fix ignoring JavaScriptWrapImport when ColumnWidth: 0

2022-01-05 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. > Make JavaScriptWrapImports: true *always* wrap imports to multiple lines. > This will be noisy and ugly. Isn't this what `prettier` does when effectively the ColumnLimit is exceeded. i.e. import { Controller, Get, Post, Req } from '@nestjs/common'; becomes

[PATCH] D116658: [clang-format][NFC] Fix typo in comment

2022-01-05 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. Welcome @pjessesco, not bad 1 day turn around on your first patch isn't bad!! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D116658/new/ https://reviews.llvm.org/D116658 __

[PATCH] D116592: [clang-format] Missing space after cast in a macro

2022-01-06 Thread MyDeveloperDay 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 rG49d311874edc: [clang-format] Missing space after cast in a macro (authored by MyDeveloperDay). Repository: rG LLVM Github Monorepo CHANGES SINCE

[PATCH] D116726: [clang-format] Fix a crash (assertion) in qualifier alignment when matching template closer is null

2022-01-06 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay created this revision. MyDeveloperDay added reviewers: owenpan, curdeius, HazardyKnusperkeks, JohelEGP. MyDeveloperDay added projects: clang, clang-format. MyDeveloperDay requested review of this revision. https://github.com/llvm/llvm-project/issues/53008 template using A = quan

[PATCH] D116726: [clang-format] Fix a crash (assertion) in qualifier alignment when matching template closer is null

2022-01-06 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay updated this revision to Diff 397806. MyDeveloperDay added a comment. The cause is because Next is a comment, not a template opener, handle that case. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D116726/new/ https://reviews.llvm.org/D116726 Files: clang/lib/Format/Qua

[PATCH] D116638: [clang-format] Fix ignoring JavaScriptWrapImport when ColumnWidth: 0

2022-01-06 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. I would say for the `ColumnLimit:0` case, we don't have to wrap a single import like this: for `JavaScriptWrapImports :true` import { Get } from '@nestjs/common'; For more than one import then I'd say it should do: import { Get, Req } fr

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

2022-01-06 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay accepted this revision. MyDeveloperDay added a comment. This revision is now accepted and ready to land. Owen I think we should push ahead with this rather than D95168: [clang-format] Add Insert/Remove Braces option as I've looked at what you've d

[PATCH] D116726: [clang-format] Fix a crash (assertion) in qualifier alignment when matching template closer is null

2022-01-06 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay updated this revision to Diff 397853. MyDeveloperDay marked 3 inline comments as done. MyDeveloperDay added a comment. Use getNextNonComment() which has some const-ness knock ons (but probably not a bad thing) address the review concerns by adding more tests (which indeed highligh

[PATCH] D116638: [clang-format] Fix ignoring JavaScriptWrapImport when ColumnWidth: 0

2022-01-06 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. > (1) very few people use... I don't think it's actually the behavior people > will want Subjective or Objective opinion? https://github.com/search?o=desc&q=%22ColumnLimit%3A+0%22&s=indexed&type=Code 95,000+ occurrences of "ColumnLimit" in github YAML files

[PATCH] D116726: [clang-format] Fix a crash (assertion) in qualifier alignment when matching template closer is null

2022-01-06 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay updated this revision to Diff 397898. MyDeveloperDay added a comment. Simplify down the test to make it easier to read, keep perhaps slightly duplicated tests to just ensure we have the coverage CHANGES SINCE LAST ACTION https://reviews.llvm.org/D116726/new/ https://reviews.ll

[PATCH] D116726: [clang-format] Fix a crash (assertion) in qualifier alignment when matching template closer is null

2022-01-06 Thread MyDeveloperDay 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 rG031d3ece3f2e: [clang-format] Fix a crash (assertion) in qualifier alignment when matching… (authored by MyDeveloperDay). Repository: rG LLVM Githu

[PATCH] D116767: [clang-format] Fix `BraceWrapping: AfterFunction` affecting synchronized blocks in Java.

2022-01-07 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay accepted this revision. MyDeveloperDay added a comment. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D116767/new/ https://reviews.llvm.org/D116767 ___ cfe-commits mailing list cfe-com

[PATCH] D116795: [clang-format] Use range-for loops. NFC.

2022-01-07 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay accepted this revision. MyDeveloperDay added a comment. This revision is now accepted and ready to land. LGTM so much easier to read!! Thank you Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D116795/new/ https://reviews.llvm.org/D1167

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

2022-01-07 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay accepted this revision. MyDeveloperDay added a comment. I agree performance of this if is unlikely to be a game changer performance wise Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D116318/new/ https://reviews.llvm.org/D116318

[PATCH] D116638: [clang-format] Fix ignoring JavaScriptWrapImport when ColumnWidth: 0

2022-01-07 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. > applies just as well to the proposal of force-wrapping at >= 2 imports Absolutely.. but it justifies that this option has got to the point that its no longer one thing or the other based on our personal subjective opinions... now we need to break down what we n

[PATCH] D116806: [clang-format] Ensure we can correctly parse lambda in the template argument list

2022-01-07 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay created this revision. MyDeveloperDay added reviewers: HazardyKnusperkeks, curdeius, JohelEGP. MyDeveloperDay added projects: clang, clang-format. MyDeveloperDay requested review of this revision. https://github.com/llvm/llvm-project/issues/46505 The presence of a lambda in an argu

[PATCH] D116859: Fix for: clang-format: break added to macro define with ColumnLimit: 0

2022-01-10 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added inline comments. Comment at: clang/unittests/Format/FormatTest.cpp:4811 + verifyFormat("#define STRINGIFY(t) #t\n" + "#define MAKEVERSIONSTRING(x, y, z, build) STRINGIFY(x) \".\" STRINGIFY(y) \".\" STRINGIFY(z) \".\" STRINGIFY(build)\n", +

[PATCH] D116806: [clang-format] Ensure we can correctly parse lambda in the template argument list

2022-01-10 Thread MyDeveloperDay via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG5c2e7c9ca043: [clang-format] Ensure we can correctly parse lambda in the template argument… (authored by MyDeveloperDay). Changed prior to commit: https://reviews.llvm.org/D116806?vs=398124&id=398521#to

[PATCH] D116920: [clang-format] clang-format eats space in front of attributes for operator delete

2022-01-10 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay created this revision. MyDeveloperDay added reviewers: curdeius, HazardyKnusperkeks, owenpan, thakis. MyDeveloperDay added projects: clang, clang-format. MyDeveloperDay requested review of this revision. https://github.com/llvm/llvm-project/issues/27037 Sorry its taken so long to g

[PATCH] D116920: [clang-format] clang-format eats space in front of attributes for operator delete

2022-01-10 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added inline comments. Comment at: clang/lib/Format/TokenAnnotator.cpp:1729 } else if (Current.is(tok::r_paren)) { if (rParenEndsCast(Current)) Current.setType(TT_CastRParen); curdeius wrote: > The current solution looks a bit

[PATCH] D116920: [clang-format] clang-format eats space in front of attributes for operator delete

2022-01-11 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay updated this revision to Diff 398861. MyDeveloperDay added a comment. Line.MightBeFunctionDecl is not true in this case Added new tests CHANGES SINCE LAST ACTION https://reviews.llvm.org/D116920/new/ https://reviews.llvm.org/D116920 Files: clang/lib/Format/TokenAnnotator.cp

[PATCH] D116920: [clang-format] clang-format eats space in front of attributes for operator delete

2022-01-11 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay updated this revision to Diff 398863. MyDeveloperDay added a comment. clang-format CHANGES SINCE LAST ACTION https://reviews.llvm.org/D116920/new/ https://reviews.llvm.org/D116920 Files: clang/lib/Format/TokenAnnotator.cpp clang/unittests/Format/FormatTest.cpp Index: cla

[PATCH] D116920: [clang-format] clang-format eats space in front of attributes for operator delete

2022-01-11 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay updated this revision to Diff 398864. MyDeveloperDay added a comment. remove additional space CHANGES SINCE LAST ACTION https://reviews.llvm.org/D116920/new/ https://reviews.llvm.org/D116920 Files: clang/lib/Format/TokenAnnotator.cpp clang/unittests/Format/FormatTest.cpp

[PATCH] D116920: [clang-format] clang-format eats space in front of attributes for operator delete

2022-01-12 Thread MyDeveloperDay 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 rG7ee4236789bb: [clang-format] clang-format eats space in front of attributes for operator… (authored by MyDeveloperDay). Changed prior to commit: h

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

2022-01-13 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay created this revision. MyDeveloperDay added reviewers: mprobst, krasimir, HazardyKnusperkeks, curdeius, owenpan. MyDeveloperDay added projects: clang, clang-format. MyDeveloperDay requested review of this revision. https://github.com/llvm/llvm-project/issues/49858 The following is

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

2022-01-13 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added inline comments. Comment at: clang/lib/Format/TokenAnnotator.cpp:3524 return false; if (Left.is(TT_JsTypeOperator) || Right.is(TT_JsTypeOperator)) return false; mprobst wrote: > shouldn't you change this line here? You know

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

2022-01-13 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added inline comments. Comment at: clang/lib/Format/TokenAnnotator.cpp:3524 return false; if (Left.is(TT_JsTypeOperator) || Right.is(TT_JsTypeOperator)) return false; mprobst wrote: > curdeius wrote: > > HazardyKnusperkeks wrote:

[PATCH] D116859: [clang-format] Fix break being added to macro define with ColumnLimit: 0

2022-01-13 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay accepted this revision. MyDeveloperDay added a comment. This revision is now accepted and ready to land. Assuming all other tests pass, I'm ok Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D116859/new/ https://reviews.llvm.org/D116859

[PATCH] D117289: [clang-format] Fix namespace end comments fixer with anonymous namespaces.

2022-01-14 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay accepted this revision. MyDeveloperDay 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/D117289/new/ https://reviews.llvm.org/D117289 ___

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

2022-01-14 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay accepted this revision. MyDeveloperDay 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/D117142/new/ https://reviews.llvm.org/D117142 ___

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

2022-01-14 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay updated this revision to Diff 399922. MyDeveloperDay marked 10 inline comments as done. MyDeveloperDay added a comment. Make suggested improvements Add additional test case CHANGES SINCE LAST ACTION https://reviews.llvm.org/D117197/new/ https://reviews.llvm.org/D117197 Files:

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

2022-01-14 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D94955/new/ https://reviews.llvm.org/D94955 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.or

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

2022-01-14 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added inline comments. Comment at: clang/lib/Format/TokenAnnotator.cpp:3522 + if ((Left.is(TT_JsTypeOperator) && Right.isTypeOrIdentifier()) || + (Left.isTypeOrIdentifier() || Left.is(TT_TemplateCloser)) && + Right.is(TT_JsTypeOperator))

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

2022-01-14 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay accepted this revision. MyDeveloperDay added a comment. LGTM go for it. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D116316/new/ https://reviews.llvm.org/D116316 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://

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

2022-01-16 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay accepted this revision. MyDeveloperDay added a comment. So simple but so nice, LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D117414/new/ https://reviews.llvm.org/D117414 ___ cfe-commi

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

2022-01-16 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay requested changes to this revision. MyDeveloperDay added inline comments. This revision now requires changes to proceed. Comment at: .arclint:5 "type": "script-and-regex", - "script-and-regex.script": "bash utils/arcanist/clang-format.sh", + "scrip

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

2022-01-17 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. In D117421#3246627 , @glotchimo wrote: > I don't know why, but `clang-format` reformatted most if not all of the > long/block comments in `WhitespaceManager.cpp`. Will it be necessary for me > to revert the changes to tho

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

2022-01-17 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added inline comments. Comment at: clang/unittests/Format/FormatTest.cpp:16172 + "int &operator() = default;\n" + "int &operator=() {", + Alignment); can you add a test showing what happens when you don't h

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

2022-01-17 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added inline comments. Comment at: clang/lib/Format/UnwrappedLineParser.cpp:2504 nextToken(); - addUnwrappedLine(); +addUnwrappedLine(); +return; I get what your trying to do but what happens in this scenario `private::mynamespace::

[PATCH] D114837: format: Remove redundant calls to guessIsObjC to speed up clang-format on unknown file types

2022-01-17 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay requested changes to this revision. MyDeveloperDay added a comment. This revision now requires changes to proceed. This needs a full context diff and ideally some unit tests Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D114837/new/ h

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

2022-01-17 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added inline comments. Comment at: clang/lib/Format/FormatTokenLexer.cpp:432 + auto Forth = (Tokens.end() - 4)[0]; bool FourthTokenIsLess = false; isn't this going to crash if Tokens.size() is 3? Repository: rG LLVM Github Monorepo CHANG

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

2021-12-06 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay created this revision. MyDeveloperDay added reviewers: HazardyKnusperkeks, curdeius, owenpan, njames93. MyDeveloperDay added projects: clang, clang-format. MyDeveloperDay requested review of this revision. https://bugs.llvm.org/show_bug.cgi?id=49298 clang-format does not respect ra

[PATCH] D22505: [clang-format] Access Modifier Use Normal Indent

2021-12-06 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. I recently reviewed this item, but I wonder what the overlap with D94661: [clang-format] [PR19056] Add support for access modifiers indentation is Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://review

[PATCH] D115103: Leak Sanitizer port to Windows

2021-12-07 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added inline comments. Comment at: compiler-rt/lib/lsan/lsan.h:20 +#elif SANITIZER_WINDOWS +# include "lsan_win.h" #endif vitalybuka wrote: > MyDeveloperDay wrote: > > clang-format? > @clemenswasser Can you please a separate tiny patch which cla

[PATCH] D115103: Leak Sanitizer port to Windows

2021-12-07 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a subscriber: kuhnel. MyDeveloperDay added inline comments. Comment at: compiler-rt/lib/lsan/lsan_common.h:48 +#elif SANITIZER_NETBSD || SANITIZER_FUCHSIA || SANITIZER_WINDOWS +# define CAN_SANITIZE_LEAKS 1 #else vitalybuka wrote: > MyDevel

[PATCH] D115103: Leak Sanitizer port to Windows

2021-12-07 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. oh! gosh I'm sorry I didn't realize that `lsan` has this non standard LLVM style .clang-format, my apologies! D100238: [sanitizer] Set IndentPPDirectives: AfterHash in .clang-format kind of surprised TBH, doesn't feel the inde

[PATCH] D115103: Leak Sanitizer port to Windows

2021-12-07 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added inline comments. Comment at: compiler-rt/lib/lsan/lsan.h:20 +#elif SANITIZER_WINDOWS +# include "lsan_win.h" #endif MyDeveloperDay wrote: > clemenswasser wrote: > > vitalybuka wrote: > > > clemenswasser wrote: > > > > MyDeveloperDay wrote:

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

2021-12-08 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay updated this revision to Diff 392674. MyDeveloperDay added a comment. Allow for option Raw String CharSequences CHANGES SINCE LAST ACTION https://reviews.llvm.org/D115168/new/ https://reviews.llvm.org/D115168 Files: clang/lib/Format/Format.cpp clang/unittests/Format/SortIn

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

2021-12-08 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added inline comments. Comment at: clang/lib/Format/Format.cpp:2605-2607 + if (!CharSequence.empty()) { +RawStringTermination = ")" + CharSequence + "\""; + } curdeius wrote: > Since `CharSequence` is empty, you might want to remo

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

2021-12-08 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. `So, you missed the digits and the characters: _{}[]#<>%:;.?*+-/^&|~!=,'.` The standard is nuts sometimes... let me add that madness in ;-) CHANGES SINCE LAST ACTION https://reviews.llvm.org/D115168/new/ https://reviews.llvm.org/D115168 __

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

2021-12-08 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay updated this revision to Diff 392703. MyDeveloperDay added a comment. Add some more unit tests CHANGES SINCE LAST ACTION https://reviews.llvm.org/D115050/new/ https://reviews.llvm.org/D115050 Files: clang/lib/Format/FormatToken.cpp clang/lib/Format/FormatToken.h clang/li

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

2021-12-08 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay updated this revision to Diff 393048. MyDeveloperDay added a comment. Add more unit tests CHANGES SINCE LAST ACTION https://reviews.llvm.org/D115050/new/ https://reviews.llvm.org/D115050 Files: clang/lib/Format/FormatToken.cpp clang/lib/Format/FormatToken.h clang/lib/For

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

2021-12-09 Thread MyDeveloperDay via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG2a73a1ac57f0: [clang-format] PR48916 PointerAlignment not working when using C++20 init… (authored by MyDeveloperDay). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.ll

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

2021-12-09 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay updated this revision to Diff 393188. MyDeveloperDay added a comment. Add a suitable amount of C++ madness, (to the limit of what I could coax the regex to accept) CHANGES SINCE LAST ACTION https://reviews.llvm.org/D115168/new/ https://reviews.llvm.org/D115168 Files: clang/

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

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

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

2021-12-10 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay updated this revision to Diff 393448. MyDeveloperDay added a comment. Add support for | but still can't get [] to work in the regex (checked the unit tests for Support/Regex and its not clear if its supported) CHANGES SINCE LAST ACTION https://reviews.llvm.org/D115168/new/ htt

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

2021-12-10 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay updated this revision to Diff 393474. MyDeveloperDay added a comment. Thank @HazardyKnusperkeks you got me over the line, I was able to escape the `[` but not the `]` so I used your trick there "R\"(([\\[A-Za-z0-9_{}#<>%:;.?*+/^&\\$|~!=,'\\-]|])*)\\(" Add unit tests to cover t

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

2021-12-12 Thread MyDeveloperDay via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG05bea533d1fc: [clang-format] [PR49298] Sort includes pass will sort inside raw strings (authored by MyDeveloperDay). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm

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

2021-12-13 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay created this revision. MyDeveloperDay added reviewers: ChuanqiXu, HazardyKnusperkeks, curdeius, krasimir. MyDeveloperDay added projects: clang, clang-format. MyDeveloperDay requested review of this revision. C++20 Modules current style is to assign a new file suffix .cppm instead o

[PATCH] D114583: [clang-format] Adjust braced list detection

2021-12-13 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. We might want to revert this until we get a solution to avoid flip flopping peoples formats, but this to me comes down to what we've been saying before about using the TT_ types to labels the types of `() {} []` I think in hindsight adding l_brace here just too g

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

2021-12-13 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added inline comments. Comment at: clang/tools/clang-format/git-clang-format:81 'mm', # ObjC++ - 'cc', 'cp', 'cpp', 'c++', 'cxx', 'hh', 'hpp', 'hxx', # C++ + 'cc', 'cp', 'cpp', 'c++', 'cxx', 'hh', 'hpp', 'hxx', 'cppm' # C++ 'cu', 'cuh',

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

2021-12-13 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay updated this revision to Diff 393851. MyDeveloperDay added a comment. Missed a comma CHANGES SINCE LAST ACTION https://reviews.llvm.org/D115625/new/ https://reviews.llvm.org/D115625 Files: clang/tools/clang-format/clang-format-diff.py clang/tools/clang-format/git-clang-for

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

2021-12-13 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay updated this revision to Diff 393858. MyDeveloperDay added a comment. Move cppm to be with other cxx extensions CHANGES SINCE LAST ACTION https://reviews.llvm.org/D115625/new/ https://reviews.llvm.org/D115625 Files: clang/tools/clang-format/clang-format-diff.py clang/tools

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

2021-12-13 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay created this revision. MyDeveloperDay added reviewers: HazardyKnusperkeks, curdeius, owenpan. MyDeveloperDay added projects: clang, clang-format. Herald added a subscriber: jeroen.dobbelaere. MyDeveloperDay requested review of this revision. https://github.com/llvm/llvm-project/issu

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

2021-12-13 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay updated this revision to Diff 394010. MyDeveloperDay added a comment. Look backwards from the { rather than scanning the namespace CHANGES SINCE LAST ACTION https://reviews.llvm.org/D115647/new/ https://reviews.llvm.org/D115647 Files: clang/lib/Format/NamespaceEndCommentsFix

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

2021-12-13 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay created this revision. MyDeveloperDay added reviewers: HazardyKnusperkeks, curdeius, jbcoe. MyDeveloperDay added projects: clang, clang-format. MyDeveloperDay requested review of this revision. https://github.com/llvm/llvm-project/issues/52677 clang-format doesn't format C# switch

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

2021-12-14 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay updated this revision to Diff 394161. MyDeveloperDay added a comment. Add other support module file types for completeness CHANGES SINCE LAST ACTION https://reviews.llvm.org/D115625/new/ https://reviews.llvm.org/D115625 Files: clang/tools/clang-format/clang-format-diff.py

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

2021-12-14 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay updated this revision to Diff 394163. MyDeveloperDay added a comment. use endsWith() CHANGES SINCE LAST ACTION https://reviews.llvm.org/D115647/new/ https://reviews.llvm.org/D115647 Files: clang/lib/Format/NamespaceEndCommentsFixer.cpp clang/unittests/Format/NamespaceEndCo

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

2021-12-14 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. In D115673#3191408 , @curdeius wrote: > 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 fr

<    17   18   19   20   21   22   23   24   25   >