[PATCH] D95538: [clang][Format] Evaluate FallbackStyle only if needed

2021-01-27 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 unit test, I personally don't understand what this is doing or why its needed, if its a bug can you reference a bug in bugzilla that explains the bug a l

[PATCH] D95017: [clang-format] Add case aware include sorting.

2021-01-28 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay accepted this revision. MyDeveloperDay added a comment. I like this much better LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D95017/new/ https://reviews.llvm.org/D95017 ___ cfe-commit

[PATCH] D94500: [clang-format] Rework Whitesmiths mode to use line-level values in UnwrappedLineParser

2021-01-30 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. My advice leave it out of the release, the next release comes round pretty quickly. This gives us 6 months of people who use the snapshots to iron out any issues. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D94500/

[PATCH] D95168: [clang-format] Add InsertBraces option

2021-01-30 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. I wonder if we should consider suggesting a different type of tool for clang `clang-reformat` A place where changes such as this and east/west fixer could be actively encouraged. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews

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

2021-01-30 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added inline comments. Comment at: clang/unittests/Format/FormatTest.cpp:995 TEST_F(FormatTest, ForEachLoops) { verifyFormat("void f() {\n" + " foreach (Item *item, itemlist) {\n" GoBigorGoHome wrote: > MyDeveloperDay wrote: > >

[PATCH] D69764: [clang-format] Add East/West Const fixer capability

2021-01-31 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. > What can be done to move this change along? I feel there has to be a fundamental acceptance that it is ok for clang-format to alter code (something it already does with sorting of includes, namespace comments). There were fairly strong opinions that clang-fo

[PATCH] D95168: [clang-format] Add InsertBraces option

2021-01-31 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. > If it's a drop in replacement (does everything clang-format does and more), > what's the benefit for that cost? I'm in agreement here, It was a suggestion just to appease those who don't like clang-format doing such things, but still allowing us to add code c

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

2021-02-02 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. I'm never going to be the one to accept patches where people change tests without making it really clear why they are changing it. You have to prove you are not regressing behaviour, I work on the Beyonce rule, "if you liked it you should have put a test on it"

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

2021-02-02 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. I guess I'm not making myself clear, I was just hoping you could be super explicit about what you are changing so anyone coming into this review would understand why the behaviour had changed (this is just pseudo code it may not be correct for all cases, but you

[PATCH] D95168: [clang-format] Add InsertBraces option

2021-02-02 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added inline comments. Comment at: clang/unittests/Format/FormatTest.cpp:17904 +" call_function(arg, arg);"; + EXPECT_EQ(IfSourceShort, format(IfSourceShort, Style)); + can these all be verifyFormats

[PATCH] D95168: [clang-format] Add InsertBraces option

2021-02-02 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added inline comments. Comment at: clang/unittests/Format/FormatTest.cpp:17996 +format(ForSourceLong, Style)); +} + MyDeveloperDay wrote: > MyDeveloperDay wrote: > > are you testing do/while? > whilst people discuss the ethics of modif

[PATCH] D95168: [clang-format] Add InsertBraces option

2021-02-02 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. I'm just going to say this here, but LLVM doesn't like the use of braces on single lines (I don't actually like that myself, but I go with the convention when I remember), but this is followed only if the reviewer catches it, but by and large its super easy for

[PATCH] D95168: [clang-format] Add InsertBraces option

2021-02-05 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added inline comments. Comment at: clang/unittests/Format/FormatTest.cpp:17996 +format(ForSourceLong, Style)); +} + tiagoma wrote: > tiagoma wrote: > > njames93 wrote: > > > MyDeveloperDay wrote: > > > > MyDeveloperDay wrote: > > > > >

[PATCH] D92257: [clang-format] Add option to control the space at the front of a line comment

2021-09-16 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. In D92257#3003281 , @byronhe wrote: > Hi guys,i found `SpacesInLineCommentPrefix` does not support other encoding > such as utf8 , > I am curious why there is a `isAlphanumeric` limit in > `BreakableLineCommentSection::Bre

[PATCH] D92257: [clang-format] Add option to control the space at the front of a line comment

2021-09-16 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. I missed the most important step! 8. Add LLVM contributor to your CV. No seriously I mean it. I interview people all the time, if I saw that on a CV, it would immediately start a conversation about what/who/why you did it. (allowing me to look up your contributi

[PATCH] D109557: Adds a BreakBeforeClosingParen option

2021-09-16 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added inline comments. Comment at: clang/unittests/Format/FormatTest.cpp:22410 + ");", + Style); +} Can you assert the cases it won't do (given your description it seem like it would do it for all before ) cases) ``` i

[PATCH] D109557: Adds a BreakBeforeClosingParen option

2021-09-16 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added inline comments. Comment at: clang/unittests/Format/FormatTest.cpp:22414 + + EXPECT_EQ("int a = (int)b;", format("int a = (\n" + "int\n" can't this just be verifyFormat but with 3 arguments? Reposit

[PATCH] D109951: [clang-format] Constructor initializer lists format with pp directives

2021-09-17 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. was there a bug report against this? what was it doing before? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D109951/new/ https://reviews.llvm.org/D109951 ___ cfe-commits m

[PATCH] D109951: [clang-format] Constructor initializer lists format with pp directives

2021-09-17 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. I see, let me add some other prominent reviewers I do see what you mean SomeClass::Constructor() : x { x } #if DEBUG , y { 0 } #endif {} SomeClass::Constructor() : x{x} , y{0} {} Repository: rG LLVM Github Monorepo CHANG

[PATCH] D109951: [clang-format] Constructor initializer lists format with pp directives

2021-09-17 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added inline comments. Comment at: clang/unittests/Format/FormatTest.cpp:19299 + ", d{d} {\n}", + Style); +} I think its good to add what you expect without the CONDITION too! Repository: rG LLVM Github Monorepo CHA

[PATCH] D109951: [clang-format] Constructor initializer lists format with pp directives

2021-09-17 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added inline comments. Comment at: clang/unittests/Format/FormatTest.cpp:19299 + ", d{d} {\n}", + Style); +} MyDeveloperDay wrote: > I think its good to add what you expect without the CONDITION too! Can you add doubly n

[PATCH] D109557: Adds a BreakBeforeClosingParen option

2021-09-20 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. we prefer verifyFormat because it does some additional checks to ensure if the code is altered it still goes to how you expect. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D109557/new/ https://reviews.llvm.org/D109

[PATCH] D69764: [clang-format] Add Left/Right Const fixer capability

2021-09-20 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay updated this revision to Diff 373656. MyDeveloperDay marked an inline comment as done. MyDeveloperDay added a comment. - Resolve the final issue I had regarding overlapping replacements (was adding double spaces between keywords), - Add ability to add a warning into the documentati

[PATCH] D69764: [clang-format] Add Left/Right Const fixer capability

2021-09-20 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay updated this revision to Diff 373657. MyDeveloperDay added a comment. Missed dump_format_style.py update CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69764/new/ https://reviews.llvm.org/D69764 Files: clang/docs/ClangFormatStyleOptions.rst clang/docs/ReleaseNotes.rst

[PATCH] D69764: [clang-format] Add Left/Right Const fixer capability

2021-09-20 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay updated this revision to Diff 373681. MyDeveloperDay added a comment. Remove debug code Tidy a few comments Remove Qualifier Order defaults (must be specified for Custom) CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69764/new/ https://reviews.llvm.org/D69764 Files: cl

[PATCH] D69764: [clang-format] Add Left/Right Const fixer capability

2021-09-21 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay updated this revision to Diff 373795. MyDeveloperDay marked 11 inline comments as done. MyDeveloperDay added a comment. Address review comments. - reorder functions to match header Fix an overlapping replacement issue when going from Right to Left (just jump past the token once d

[PATCH] D69764: [clang-format] Add Left/Right Const fixer capability

2021-09-21 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay updated this revision to Diff 373802. MyDeveloperDay added a comment. Allow the use of "TypenameMacros" to allow for Capitialized types (be aware they should be non pointer types and don't have to be macros, so may need its own future "NonPtrTypes" setting) CHANGES SINCE LAST AC

[PATCH] D109951: [clang-format] Constructor initializer lists format with pp directives

2021-09-21 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. mark the comments as done once addressed please Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D109951/new/ https://reviews.llvm.org/D109951 ___ cfe-commits mailing list cfe

[PATCH] D109557: Adds a BreakBeforeClosingParen option

2021-09-21 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay accepted this revision. MyDeveloperDay added a comment. This revision is now accepted and ready to land. This seems ok, might be worth adding a release note Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D109557/new/ https://reviews.ll

[PATCH] D69764: [clang-format] Add Left/Right Const fixer capability

2021-09-21 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added inline comments. Comment at: clang/lib/Format/QualifierAlignmentFixer.cpp:28 + +static void replaceToken(const SourceManager &SourceMgr, + tooling::Replacements &Fixes, HazardyKnusperkeks wrote: > Out of curiosity, on

[PATCH] D69764: [clang-format] Add Left/Right Const fixer capability

2021-09-21 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay updated this revision to Diff 373817. MyDeveloperDay added a comment. If we don't specify types in the QualifierOrder (say static or inline as in default Left/Right) then don't push const through them just because they can be a type of qualifier we support. i.e. only push through

[PATCH] D109557: Adds a BreakBeforeClosingParen option

2021-09-21 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/D109557/new/ https://reviews.llvm.org/D109557 ___ cfe-commits mailing list cfe-com

[PATCH] D69764: [clang-format] Add Left/Right Const fixer capability

2021-09-21 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay updated this revision to Diff 373954. MyDeveloperDay added a comment. Its no longer essential to compare the fixes against the original code, as this was needed because of an artifact with adding double spaces and was resolved with checking for if the previous token already ended

[PATCH] D109951: [clang-format] Constructor initializer lists format with pp directives

2021-09-22 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/D109951/new/ https://reviews.llvm.org/D109951 ___ cfe-commits mailing list cfe-com

[PATCH] D69764: [clang-format] Add Left/Right Const fixer capability

2021-09-22 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay updated this revision to Diff 374294. MyDeveloperDay marked 3 inline comments as done. MyDeveloperDay added a comment. Remove use of Typenamemacros (we'll solve this a different way later) CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69764/new/ https://reviews.llvm.org/D

[PATCH] D69764: [clang-format] Add Left/Right Const fixer capability

2021-09-22 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay updated this revision to Diff 374298. MyDeveloperDay added a comment. Use better vector initialization CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69764/new/ https://reviews.llvm.org/D69764 Files: clang/docs/ClangFormatStyleOptions.rst clang/docs/ReleaseNotes.rst

[PATCH] D69764: [clang-format] Add Left/Right Const fixer capability

2021-09-23 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay updated this revision to Diff 374455. MyDeveloperDay marked 3 inline comments as done. MyDeveloperDay added a comment. Fix clang-format and clang-tidy issues CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69764/new/ https://reviews.llvm.org/D69764 Files: clang/docs/Clan

[PATCH] D69764: [clang-format] Add Left/Right Const fixer capability

2021-09-23 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added inline comments. Comment at: clang/lib/Format/QualifierAlignmentFixer.cpp:449 +return false; + if (Tok->is(TT_TypenameMacro)) +return false; HazardyKnusperkeks wrote: > I think this is still right, because it is a type (according to

[PATCH] D110252: Added note about Whatstyle and Unformat

2021-09-23 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. Whatstyle has not been updated since Dec 2020, is it going to cover the latest style configuration options? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D110252/new/ https://reviews.llvm.org/D110252 ___

[PATCH] D69764: [clang-format] Add Left/Right Const fixer capability

2021-09-23 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. If there are no other objections I'm going to land this shortly. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69764/new/ https://reviews.llvm.org/D69764 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D69764: [clang-format] Add Left/Right Const fixer capability

2021-09-23 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 rGa44ab1702539: [clang-format] Add Left/Right Const fixer capability (authored by MyDeveloperDay). Repository: rG LLVM Github Monorepo CHANGES SINC

[PATCH] D110083: [clang-offload-bundler][docs][NFC] Add archive unbundling documentation

2021-09-23 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. Do you mind if I fix the build failures by encasing the codelike section in a .. code::? diff --git a/clang/docs/ClangOffloadBundler.rst b/clang/docs/ClangOffloadBundler.rst index 312c45602c99..ac4c5b51904f 100644 --- a/clang/docs/ClangOffloadBundler.rst

[PATCH] D110359: [clang-format] [NFC] ensure clang-format command-line argument sets up the default left/right qualifier ordering

2021-09-23 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay created this revision. MyDeveloperDay added a reviewer: HazardyKnusperkeks. MyDeveloperDay added a project: clang-format. MyDeveloperDay requested review of this revision. Herald added a project: clang. When specifying the alignment direction on the command line ensure we set up th

[PATCH] D110359: [clang-format] [NFC] ensure clang-format command-line argument sets up the default left/right qualifier ordering

2021-09-23 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. This is to support D69764: [clang-format] Add Left/Right Const fixer capability Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D110359/new/ https://reviews.llvm.org/D110359 _

[PATCH] D110359: [clang-format] [NFC] ensure clang-format command-line argument sets up the default left/right qualifier ordering

2021-09-23 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay updated this revision to Diff 374652. MyDeveloperDay added a comment. Allow passing of desired qualifier order on the command line clang-format --qualifier-alignment="const static type volatile" test_file.cpp CHANGES SINCE LAST ACTION https://reviews.llvm.org/D110359/new/ ht

[PATCH] D110359: [clang-format] ensure clang-format command-line argument sets up the default left/right qualifier ordering

2021-09-23 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay updated this revision to Diff 374660. MyDeveloperDay added a comment. Rename the ConstAlignment variable CHANGES SINCE LAST ACTION https://reviews.llvm.org/D110359/new/ https://reviews.llvm.org/D110359 Files: clang/tools/clang-format/ClangFormat.cpp Index: clang/tools/clan

[PATCH] D110083: [clang-offload-bundler][docs][NFC] Add archive unbundling documentation

2021-09-24 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. Thanks... https://github.com/llvm/llvm-project/commit/7890afddecff01119f4d5e8825b43dd2c8361648 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D110083/new/ https://reviews.llvm.org/D110083

[PATCH] D69764: [clang-format] Add Left/Right Const fixer capability

2021-09-24 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. In D69764#3019409 , @nemanjai wrote: > This broke buildbots that have -Werror specified. I pushed in a fix in > https://reviews.llvm.org/rG76d845cb169f048cb6f2176c3e7a6534dc5af097. > Also, please consider formatting your co

[PATCH] D110359: [clang-format] ensure clang-format command-line argument sets up the default left/right qualifier ordering

2021-09-24 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 rG87ab958641fa: [clang-format] ensure clang-format command-line argument sets up the default… (authored by MyDeveloperDay). Changed prior to commit:

[PATCH] D108765: [docs] Fix documentation of clang-format BasedOnStyle type

2021-09-24 Thread MyDeveloperDay via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG9e8fff26f374: [clang-format][docs] Fix documentation of clang-format BasedOnStyle type (authored by FederAndInk, committed by MyDeveloperDay). Changed prior to commit: https://reviews.llvm.org/D108765?

[PATCH] D109557: Adds a BreakBeforeClosingParen option

2021-09-24 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. We would need your name and email address to commit this for you in the form git commit --amend --author="John Doe " See https://llvm.org/docs/DeveloperPolicy.html Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D10955

[PATCH] D110392: [clang-format] Left/Right alignment fixer can cause false positive replacements when they don't actually change anything

2021-09-24 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay created this revision. MyDeveloperDay added reviewers: curdeius, HazardyKnusperkeks. MyDeveloperDay added projects: clang, clang-format. MyDeveloperDay requested review of this revision. Earlier during the development of D69764: [clang-format] Add Left/Right Const fixer capability

[PATCH] D110432: [clang-format][docs] mark new clang-format configuration options based on which version they would GA

2021-09-24 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay created this revision. MyDeveloperDay added reviewers: HazardyKnusperkeks, curdeius, krasimir. MyDeveloperDay added projects: clang, clang-format. MyDeveloperDay requested review of this revision. Sometimes I see people unsure about which versions they can use in clang-format becau

[PATCH] D110432: [clang-format][docs] mark new clang-format configuration options based on which version they would GA

2021-09-24 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. This is just a very rough idea, feel free to pull it apart. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D110432/new/ https://reviews.llvm.org/D110432 ___ cfe-commits mail

[PATCH] D109557: Adds a BreakBeforeClosingParen option

2021-09-24 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. FYI, this is a very aggressive change, I highly recommend you run this over a large code base before landing. to double check, here is one slight oddity which I cannot determine if its correct or not. void foo() { if (quitelongarg != (alsolongarg - 1)) {

[PATCH] D101868: [clang-format] Adds a formatter for aligning arrays of structs

2021-09-24 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. There are a number of bugs logged against this feature, are you still around to look into them? https://bugs.llvm.org/show_bug.cgi?id=51926 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D101868/new/ https://reviews.

[PATCH] D110432: [clang-format][docs] mark new clang-format configuration options based on which version they would GA

2021-09-24 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. I agree this looks better F19213646: image.png Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D110432/new/ https://reviews.llvm.org/D110432 ___

[PATCH] D110392: [clang-format] Left/Right alignment fixer can cause false positive replacements when they don't actually change anything

2021-09-25 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay updated this revision to Diff 375023. MyDeveloperDay marked 2 inline comments as done. MyDeveloperDay added a comment. Address review comment CHANGES SINCE LAST ACTION https://reviews.llvm.org/D110392/new/ https://reviews.llvm.org/D110392 Files: clang/lib/Format/QualifierAli

[PATCH] D110392: [clang-format] Left/Right alignment fixer can cause false positive replacements when they don't actually change anything

2021-09-25 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 rGc2ec5dd20953: [clang-format] Left/Right alignment fixer can cause false positive replacements… (authored by MyDeveloperDay). Repository: rG LLVM G

[PATCH] D110432: [clang-format][docs] mark new clang-format configuration options based on which version they would GA

2021-09-27 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay updated this revision to Diff 375177. MyDeveloperDay marked 2 inline comments as done. MyDeveloperDay added a comment. Mark all options with their version, Move the \version into the comment Warning if we add new options without a version I think this is a fairly good representati

[PATCH] D110432: [clang-format][docs] mark new clang-format configuration options based on which version they would GA

2021-09-27 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added inline comments. Comment at: clang/docs/tools/dump_format_style.py:237 +field_type, field_name, trailcomment, version = re.match(r'([<>:\w(,\s)]+)\s+(\w+)\s*(\/\*version=([0-9.]*)\*\/)*;', line).groups() +

[PATCH] D110432: [clang-format][docs] mark new clang-format configuration options based on which version they would GA

2021-09-27 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay updated this revision to Diff 375206. MyDeveloperDay marked 6 inline comments as done. MyDeveloperDay added a comment. - Minor available version corrections - Addressing review comments CHANGES SINCE LAST ACTION https://reviews.llvm.org/D110432/new/ https://reviews.llvm.org/D11

[PATCH] D110432: [clang-format][docs] mark new clang-format configuration options based on which version they would GA

2021-09-27 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added inline comments. Comment at: clang/docs/tools/dump_format_style.py:218 + if line.startswith(r'/// \version'): +match = re.match(r'/// \\version\s*(?P[0-9.]+)*',line) +if match: HazardyKnusperkeks wrote: > HazardyKnusperke

[PATCH] D110432: [clang-format][docs] mark new clang-format configuration options based on which version they would GA

2021-09-28 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 rG23a5090c6ac7: [clang-format][docs] mark new clang-format configuration options based on which… (authored by MyDeveloperDay). Repository: rG LLVM G

[PATCH] D69764: [clang-format] Add Left/Right Const fixer capability

2021-09-29 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay marked 2 inline comments as done. MyDeveloperDay added inline comments. Comment at: clang/docs/ClangFormatStyleOptions.rst:3233 + + ``QualifierAlignment`` COULD lead to incorrect code generation. + HazardyKnusperkeks wrote: > simon.giesecke wrote

[PATCH] D110801: [clang-format] [docs] [NFC] improve clarity in the QualifierAlignment warning

2021-09-30 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay created this revision. MyDeveloperDay added reviewers: HazardyKnusperkeks, simon.giesecke. MyDeveloperDay added projects: clang, clang-format. MyDeveloperDay requested review of this revision. Improve the clarity and guidance of the warning when using code modifying option in clang

[PATCH] D69764: [clang-format] Add Left/Right Const fixer capability

2021-09-30 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay marked an inline comment as done. MyDeveloperDay added a comment. I'm not a wordsmith but how about D110801: [clang-format] [docs] [NFC] improve clarity in the QualifierAlignment warning Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[PATCH] D110803: [clang-format][docs][NFC] correct the "first supported versions" of some of the clang-format options

2021-09-30 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay created this revision. MyDeveloperDay added a reviewer: HazardyKnusperkeks. MyDeveloperDay added projects: clang, clang-format. MyDeveloperDay requested review of this revision. Some of the first supported version field were incorrectly attributed to a later branch. It wasn't poss

[PATCH] D110801: [clang-format] [docs] [NFC] improve clarity in the QualifierAlignment warning

2021-09-30 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added inline comments. Comment at: clang/include/clang/Format/Format.h:1903 + /// lead to incorrect code generation due to a lack of semantic information + /// especially in the presense of macros, care should be take to review code + /// changes made by this

[PATCH] D110801: [clang-format] [docs] [NFC] improve clarity in the QualifierAlignment warning

2021-09-30 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay updated this revision to Diff 376140. MyDeveloperDay added a comment. Try and improve the wordage CHANGES SINCE LAST ACTION https://reviews.llvm.org/D110801/new/ https://reviews.llvm.org/D110801 Files: clang/docs/ClangFormatStyleOptions.rst clang/include/clang/Format/Forma

[PATCH] D110801: [clang-format] [docs] [NFC] improve clarity in the QualifierAlignment warning

2021-09-30 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. > We have no such indication but want to play safe until this has been tested > in the wild. We have no indication of issues yet, other than those of using macros, any we do find we would hope to fix, much like any other clang-format bug. I have run this on a nu

[PATCH] D110833: [clang-format] Add ControlStatementsAndFunctionDefinitionsExceptControlMacros option to SpaceBeforeParens

2021-09-30 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. First impressions is `ControlStatementsAndFunctionDefinitionsExceptControlMacros` is a mouthful... Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D110833/new/ https://reviews.llvm.org/D110833

[PATCH] D110833: [clang-format] Add ControlStatementsAndFunctionDefinitionsExceptControlMacros option to SpaceBeforeParens

2021-09-30 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. I feel this is going the way of BraceWrapping in that it should be: SpaceBeforeParens: - BeforeMacro: false - BeforeFunction: true Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D110833/new/ https://revie

[PATCH] D110833: [clang-format] Add ControlStatementsAndFunctionDefinitionsExceptControlMacros option to SpaceBeforeParens

2021-10-01 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added inline comments. Comment at: clang/docs/ClangFormatStyleOptions.rst:3649 ``SBPO_ControlStatementsExceptForEachMacros`` remains an alias for backward compatibility. Now I look back here, why where these Macro considered the same as

[PATCH] D110833: [clang-format] Add ControlStatementsAndFunctionDefinitionsExceptControlMacros option to SpaceBeforeParens

2021-10-01 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. The use of "Custom" would kind of match the BraceWrapping, as for `SpaceBeforeParensCustom` either that or `SpaceBeforeParensStyles` , `SpaceBeforeParensSettings`, `SpaceBeforeParensOptions` I guess I don't really mind, (I always find naming things hard!) maybe t

[PATCH] D110833: [clang-format] Add ControlStatementsAndFunctionDefinitionsExceptControlMacros option to SpaceBeforeParens

2021-10-01 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. Just adding a few more clang-format big guns as reviewers, its probably good to get some more input before embarking on what is going to probably be a reasonably sized change. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llv

[PATCH] D110801: [clang-format] [docs] [NFC] improve clarity in the QualifierAlignment warning

2021-10-02 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 rGdd3caa99bd87: [clang-format] [docs] [NFC] improve clarity in the QualifierAlignment warning (authored by MyDeveloperDay). Changed prior to commit:

[PATCH] D109951: [clang-format] Constructor initializer lists format with pp directives

2021-10-02 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 rG3d209c76ddb5: [clang-format] Constructor initializer lists format with pp directives (authored by guitard0g, committed by MyDeveloperDay). Repositor

[PATCH] D111000: [clang-format] allow clang-format to be passed a file of filenames so we can add a regression suite of "clean clang-formatted files" from LLVM

2021-10-03 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. In D111000#3038308 , @HazardyKnusperkeks wrote: > Basically okay, I would have made 3 commits out of it: > > 1. Add the function to clang-format > 2. The code clean up of the python script > 3. The additional file generati

[PATCH] D95168: [clang-format] Add InsertBraces option

2021-10-03 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. @tiagoma are you still interested in pursuing this? I have some suggestions 1. I'd like to move the BraceInserter Into its own .cpp and .h files (like I did with the QualifierAlignmentFixer) 2. I'd like to move the unit tests into their own .cpp file (because I t

[PATCH] D95168: [clang-format] Add InsertBraces option

2021-10-03 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added inline comments. Comment at: clang/include/clang/Format/Format.h:908 + /// If set to ``BIS_Always`` will always insert braces. + /// The default is the disabled value of ``BIS_Never``. + BraceInsertionStyle InsertBraces; Please add somethi

[PATCH] D95168: [clang-format] Add InsertBraces option

2021-10-03 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added inline comments. Comment at: clang/lib/Format/Format.cpp:2844 + +if (Style.InsertBraces != FormatStyle::BIS_Never) + Passes.emplace_back([&](const Environment &Env) { Why is this just C++ (LK_Cpp is also Objective C++ and C and Objec

[PATCH] D95168: [clang-format] Add InsertBraces option

2021-10-03 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 patch needs rebasing to main Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D95168/new/ https://reviews.llvm.org/D9

[PATCH] D95168: [clang-format] Add InsertBraces option

2021-10-04 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. In D95168#3039033 , @owenpan wrote: > In D95168#3038531 , @MyDeveloperDay > wrote: > >> @tiagoma are you still interested in pursuing this? I have some suggestions >> >> 1. I'd like

[PATCH] D110252: Added note about Whatstyle and Unformat

2021-10-04 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. IMHO is that I don't personally feel we need to endorse of someone else's that could be potentially incomplete, These tools are interesting (per say) but I have no idea how accurate they are in terms of using all the options or how much they keep pace, for every

[PATCH] D95168: [clang-format] Add InsertBraces option

2021-10-05 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. This will fail in the presence of macro line concatination if (x) return true; else return false; #define RETURN(x) \ if (x) \ return true;\ else \ return false; it will remove the trailing \ if (x) { return tru

[PATCH] D94661: [clang-format] [PR19056] Add support for access modifiers indentation

2021-02-13 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay accepted this revision. MyDeveloperDay added a comment. the LGTM, please mark your comments as done when done. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D94661/new/ https://reviews.llvm.org/D94661 ___ cfe-commits mailing lis

[PATCH] D95168: [clang-format] Add InsertBraces option

2021-02-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. LGTM, we need to run this on a large code base to ensure it doesn't make mistakes in real code, but I think this looks good. Repository: rG LLVM Github Monorepo CHANGES SIN

[PATCH] D96744: clang-format IncludeBlocks: Regroup determination of "main" for framework-style includes fix

2021-02-16 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. I think I'd be concern about the `` case (lets ignore the fact that I should be including so imagine I'm writing a string class and I have string.cpp including "string.h" but as part of my string class because I'm somehow extending std::string #include "stri

[PATCH] D97137: Bug fix of https://bugs.llvm.org/show_bug.cgi?id=49175

2021-02-21 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added inline comments. Comment at: clang/unittests/Format/FormatTest.cpp:14261 + "unsigned int Const* c;", + Alignment); } can you test west const too? ``` Const unsigned int* c; const unsigned int* d; Const unsigned i

[PATCH] D103678: [Format] Fix incorrect pointer/reference detection

2021-06-04 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added inline comments. Comment at: clang/unittests/Format/FormatTest.cpp:8768 + verifyFormat("void f() { a.operator()(a * a); }"); + verifyFormat("void f() { a->operator()(a & a); }"); } can you add void f() { a.operator()(*a * *a); } Reposit

[PATCH] D103678: [Format] Fix incorrect pointer/reference detection

2021-06-04 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added inline comments. Comment at: clang/lib/Format/TokenAnnotator.cpp:243 + bool OperatorCalledAsMemberFunction = + Prev->Previous && Prev->Previous->isOneOf(tok::period, tok::arrow); + Contexts.back().IsExpression = OperatorCalledAsMemberFuncti

[PATCH] D101868: [clang-format] Adds a formatter for aligning arrays of structs

2021-06-06 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. In D101868#2799563 , @HazardyKnusperkeks wrote: > Remains the issue with the alignment, I would like to know @MyDeveloperDay 's > opinion on that. Should the values be right aligned, or left aligned? As far > as I see al

[PATCH] D103678: [Format] Fix incorrect pointer/reference detection

2021-06-08 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay accepted this revision. MyDeveloperDay added inline comments. Comment at: clang/lib/Format/TokenAnnotator.cpp:236 +Prev = Prev->Previous; +assert(Prev); + } Do we need to worry about `Prev` ever being null? Does the assert ever

[PATCH] D101868: [clang-format] Adds a formatter for aligning arrays of structs

2021-06-08 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. In D101868#2803670 , @feg208 wrote: > Ok. Given @HazardyKnusperkeks comment I'll make the default be left alignment > and update the tests accordingly You could add `AlignArrayOfStructuresMemberAlignment` `Left:Right` y

[PATCH] D101868: [clang-format] Adds a formatter for aligning arrays of structs

2021-06-08 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. Better still how about having ArraryMemberAligmentEnum AlignArrayOfStructures of "None, Left and Right" I find boolean options don't stay boolean for long! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D101868/new

[PATCH] D101868: [clang-format] Adds a formatter for aligning arrays of structs

2021-06-08 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. > Save the name. I should have it rolled up shortly. You an keep the name, just change the type to be an enum Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D101868/new/ https://reviews.llvm.org/D101868 _

[PATCH] D103678: [Format] Fix incorrect pointer/reference detection

2021-06-08 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added inline comments. Comment at: clang/lib/Format/TokenAnnotator.cpp:236 +Prev = Prev->Previous; +assert(Prev); + } curdeius wrote: > MyDeveloperDay wrote: > > Do we need to worry about `Prev` ever being null? Does the assert

[PATCH] D103678: [Format] Fix incorrect pointer/reference detection

2021-06-08 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added inline comments. Comment at: clang/unittests/Format/FormatTest.cpp:8283-8286 + // TODO: + // Calling an operator as a non-member function is hard to distinguish + // verifyFormat("void f() { operator*(a & a); }"); + // verifyFormat("void f() { operator&(a

[PATCH] D101868: [clang-format] Adds a formatter for aligning arrays of structs

2021-06-09 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added inline comments. Comment at: clang/lib/Format/WhitespaceManager.h:233 + /// Align Array Initializers over all \c Changes + void alignArrayInitializers(); Can ensure the comment starts with a capital and finishes with punctuation (I perso

<    7   8   9   10   11   12   13   14   15   16   >