[PATCH] D132131: [clang-format] Adds a formatter for aligning trailing comments over empty lines

2022-08-25 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added inline comments. Comment at: clang/unittests/Format/FormatTest.cpp:20044 Style.Language = FormatStyle::LK_Cpp; - CHECK_PARSE_BOOL(AlignTrailingComments); CHECK_PARSE_BOOL(AllowAllArgumentsOnNextLine); don't remove Repository: rG LL

[PATCH] D132719: [clang-format] Rework removeBraces() in Format.cpp

2022-08-26 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/D132719/new/ https://reviews.llvm.org/D132719 ___

[PATCH] D132131: [clang-format] Adds a formatter for aligning trailing comments over empty lines

2022-08-26 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay accepted this revision. MyDeveloperDay added inline comments. This revision is now accepted and ready to land. Comment at: clang/lib/Format/Format.cpp:649 IO.mapOptional("AlignOperands", Style.AlignOperands); -IO.mapOptional("AlignTrailingComments", Style.

[PATCH] D132131: [clang-format] Adds a formatter for aligning trailing comments over empty lines

2022-08-26 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added inline comments. Comment at: clang/docs/ClangFormatStyleOptions.rst:797 +AlignConsecutiveMacros: + Enabled: true + AcrossEmptyLines: true why do we need Enabled? isn't it just ``` false: AcrossEmptyLines: false AcrossComments:

[PATCH] D134049: [clang-format] Disallow trailing return arrows to be operators

2022-09-16 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added inline comments. Comment at: clang/unittests/Format/TokenAnnotatorTest.cpp:448 + EXPECT_EQ(Tokens[11]->FakeLParens.size(), 0u); + EXPECT_TOKEN(Tokens[5], tok::kw_requires, TT_RequiresClause); + should you not check that Tokens[21] is a TT_T

[PATCH] D133589: [clang-format] JSON formatting add new option for controlling newlines in json arrays

2022-09-19 Thread MyDeveloperDay via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG95b394711106: [clang-format] JSON formatting add new option for controlling newlines in json… (authored by MyDeveloperDay). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://revie

[PATCH] D134233: [clang-format] Wrap inserted braces only if preceded by comments

2022-09-20 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 love the little and often approach, its so much easier to review, LGTM... Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134233/

[PATCH] D134042: [clang-format] Fix alignment in #else preprocessor blocks

2022-09-20 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. Should we test nested if/else/endif CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134042/new/ https://reviews.llvm.org/D134042 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-

[PATCH] D134049: [clang-format] Disallow trailing return arrows to be operators

2022-09-20 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. @rymiel you definitely deserve the commit access: comments and initial investigations and possible bug locations in github issues, defect focused initial commits, way to go!, Welcome aboard.. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION htt

[PATCH] D134329: [clang-format][NFC] Reformat clang/lib/Format using 6257832bf94f

2022-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. LGTM, we of all people have to dogfood this stuff.. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134329/new/ https://reviews.ll

[PATCH] D134325: [clang-format] Look ahead before consuming `bool` in requires clause.

2022-09-21 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. In D134325#3804817 , @zzyyyl wrote: > What if `concept c = (bool)(...)` ? > > or `concept c = static_cast(...)` Aren't these handled elsewhere in the case (by the static_cast and l_paren rules)? but possibly worth a test

[PATCH] D134325: [clang-format] Look ahead before consuming `bool` in requires clause.

2022-09-21 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. given that #57538 manifested itself as a incorrect format (no doubt because of incorrect token annotation), should we add a verifyFormat tests somewhere for the example from the github issue, (it just protects the formatting for being broken by some other change

[PATCH] D134587: [clang-format] Correctly annotate static and consteval lambdas

2022-09-24 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/D134587/new/ https://reviews.llvm.org/D134587 ___

[PATCH] D134652: [clang-format] Add Really Basic Carbon Support/Infrastructure

2022-09-26 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay created this revision. MyDeveloperDay added reviewers: chandlerc, HazardyKnusperkeks, owenpan, curdeius, klimek. MyDeveloperDay added projects: clang-format, clang. Herald added a project: All. MyDeveloperDay requested review of this revision. Its great when clang-format is the ubi

[PATCH] D134652: [clang-format] Add Really Basic Carbon Support/Infrastructure

2022-09-26 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay updated this revision to Diff 462959. MyDeveloperDay added a comment. Ensure the colon stick with the variable declaraion for (var name: String in names) { Console.Print(name); } CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134652/new/ https://reviews.llvm.org

[PATCH] D134652: [clang-format] Add Really Basic Carbon Support/Infrastructure

2022-09-26 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay updated this revision to Diff 462966. MyDeveloperDay added a comment. Treat a class like a C# or Javascript class there is no semi after the brace `};` class Sum { var a: i32; fn Add[me:Self](var num: i32) -> i32 { var total: i32 = me.a + num; return total;

[PATCH] D134652: [clang-format] Add Really Basic Carbon Support/Infrastructure

2022-09-26 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay updated this revision to Diff 463000. MyDeveloperDay added a comment. Fix pointer function arguments Add release notes Fix more `TT_TrailingReturnArrow` cases CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134652/new/ https://reviews.llvm.org/D134652 Files: clang/.clang

[PATCH] D134652: [clang-format] Add Really Basic Carbon Support/Infrastructure

2022-09-26 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay updated this revision to Diff 463001. MyDeveloperDay added a comment. Remove .clang-format change CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134652/new/ https://reviews.llvm.org/D134652 Files: clang/docs/ClangFormatStyleOptions.rst clang/docs/ReleaseNotes.rst cl

[PATCH] D134700: [clang-format] Fix a bug with C++ `export import `

2022-09-26 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/D134700/new/ https://reviews.llvm.org/D134700 ___

[PATCH] D134626: [clang-format] Correctly indent closing brace of compound requires

2022-09-26 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. Hmm…it’s nice when it’s this simple, but I get an uneasy feeling about the knock on effects. I’d say go for it but perhaps we could run it over a larger code base to see the implications Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://

[PATCH] D134042: [clang-format] Fix alignment in #else preprocessor blocks

2022-09-26 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay accepted this revision. MyDeveloperDay added a comment. LGTM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134042/new/ https://reviews.llvm.org/D134042 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.

[PATCH] D134329: [clang-format][NFC] Reformat clang/lib/Format using 6257832bf94f

2022-09-27 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. Unfortunately this is failing the "premerge checks" F24716144: image.png Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134329/new/ https://reviews.llvm.org/D134329 _

[PATCH] D127484: [clang-format] Use tabs on GNU style

2022-09-27 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. @a4lg If we have decided NOT to do this can we Abandon the review so as to remove it from the reviewers "clang-format" lists? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D127484/new/ https://reviews.llvm.org/D12748

[PATCH] D121370: [clang-format] SortIncludes should support "@import" lines in Objective-C

2022-09-27 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. @kwk have we bottomed out on this issue? its been stale for a bit. If we are not going to work on it further can be "Abandon" the review to get it off our lists? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D121370/

[PATCH] D134626: [clang-format] Correctly indent closing brace of compound requires

2022-09-27 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/D134626/new/ https://reviews.llvm.org/D134626 ___

[PATCH] D134733: [clang-format][chore] transparent #include name regex

2022-09-27 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added inline comments. Comment at: clang/lib/Tooling/Inclusions/HeaderIncludes.cpp:400 +llvm::Regex getCppIncludeRegex() { + static const char CppIncludeRegexPattern[] = + R"(^[\t\ ]*#[\t\ ]*(import|include)[^"<]*(["<][^">]*[">]))"; What if t

[PATCH] D134652: [clang-format] Add Basic Carbon Support/Infrastructure to clang-format

2022-09-28 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay updated this revision to Diff 463502. MyDeveloperDay retitled this revision from "[clang-format] Add Really Basic Carbon Support/Infrastructure" to "[clang-format] Add Basic Carbon Support/Infrastructure to clang-format". MyDeveloperDay added a comment. Improve the handling of gen

[PATCH] D134733: [clang-format][chore] transparent #include name regex

2022-09-28 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added inline comments. Comment at: clang/lib/Tooling/Inclusions/HeaderIncludes.cpp:407 +const llvm::SmallVectorImpl &Matches) { + if (Matches.size() >= 3) { +return Matches[2]; kwk wrote: > MyDeveloperDay wrote: > > ‘>= 2’ > @MyDeveloperDa

[PATCH] D134329: [clang-format][NFC] Reformat clang/lib/Format using 6257832bf94f

2022-09-28 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. I'm not sure what version of clang-format they use in the premerge checks, I don't know who we contact to get it updated. I guess the clang-format check might come before any building of the code. You know if we relex the "Unknown" YAML option, this would go a lo

[PATCH] D134329: [clang-format][NFC] Reformat clang/lib/Format using 6257832bf94f

2022-09-29 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. yeah I'm ok with that too. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134329/new/ https://reviews.llvm.org/D134329 ___ cfe-commits mailing list cfe-commits@lists.llvm.o

[PATCH] D134852: [clang-format][NFC] Clean up class HeaderIncludes and Format.cpp

2022-09-29 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added inline comments. Comment at: clang/lib/Format/Format.cpp:2774 - -const char CppIncludeRegexPattern[] = -R"(^[\t\ ]*#[\t\ ]*(import|include)[^"<]*(["<][^">]*[">]))"; Did I miss where this comes from now? Repository: rG LLVM Github Mono

[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

2022-09-29 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. The use of a response file was actually new to me, I suspect it isn't widely know about, but yes in theory it could be used. As this feature has been landed for sometime, I'm not included to remove it, there could be other people using it. Repository: rG LLVM

[PATCH] D134652: [clang-format] Add Basic Carbon Support/Infrastructure to clang-format

2022-09-30 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. @HazardyKnusperkeks thank you, any thoughts from you or others on if you feel its ok for me to continue? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134652/new/ https://reviews.llvm.org/D134652 ___ cfe-commi

[PATCH] D134652: [clang-format] Add Basic Carbon Support/Infrastructure to clang-format

2022-10-02 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. I'm not actually really considering the new "keywords" much at the moment (less match), some of this is really about formatting the Carbon fundamentals (SpaceBetween etc..) to a Google style. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134652/new/ htt

[PATCH] D135026: [clang-format] Handle C# interpolated verbatim string prefix @$

2022-10-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. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D135026/new/ https://reviews.llvm.org/D135026 ___

[PATCH] D135115: [clang-format] update --files help description

2022-10-04 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added inline comments. Comment at: clang/docs/ClangFormat.rst:29 USAGE: clang-format [options] [ ...] if response files are so common place why not say it here `clang-format [options] [@file]` I personally don't believe we need to documen

[PATCH] D135115: [clang-format] update --files help description

2022-10-04 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. Its Is a minor point, but when using --files, the shell will allow me to autocomplete.. clang-format -verbose -n -files ./cla clang-format -verbose -n -files ./clang/d etc... all the way to the file clang-format -verbose -n -files ./clang/docs/too

[PATCH] D135466: [clang-format] Add support to remove unnecessary semicolons after function definition

2022-10-07 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay created this revision. MyDeveloperDay added reviewers: HazardyKnusperkeks, owenpan, curdeius, rymiel, Eugene.Zelenko. MyDeveloperDay added projects: clang, clang-format. Herald added a project: All. MyDeveloperDay requested review of this revision. Fixes: #58217 This change is to

[PATCH] D135466: [clang-format] Add support to remove unnecessary semicolons after function definition

2022-10-08 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. In D135466#3844287 , @owenpan wrote: > @MyDeveloperDay Cool! > > In D135466#3843792 , > @HazardyKnusperkeks wrote: > >> Maybe even extend the option to other unnecessary semicolons

[PATCH] D135466: [clang-format] Add support to remove unnecessary semicolons after function definition

2022-10-08 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay updated this revision to Diff 466291. MyDeveloperDay marked 16 inline comments as done. MyDeveloperDay added a comment. - Switch to the looping method and remove the need to change parseBlock (phew!) thanks @owenpan for the guidance - Add tests for double `;;` - Change the name to

[PATCH] D135466: [clang-format] Add support to remove unnecessary semicolons after function definition

2022-10-08 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added inline comments. Comment at: clang/docs/ClangFormatStyleOptions.rst:3779 + return a>b?a:b;return a>b?a:b; +}; }; + owenpan wrote: > lahwaacz wrote: > > There is an e

[PATCH] D135466: [clang-format] Add support to remove unnecessary semicolons after function definition

2022-10-09 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. There is a simple fix to allow the empty functions in UnwrappedLineParser::calculateBraceTypes F24856496: image.png by moving this clause out of the "isJavaScript()" clause this will ensure for `foo() {}` the `{` is considere

[PATCH] D135466: [clang-format] Add support to remove unnecessary semicolons after function definition

2022-10-09 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay updated this revision to Diff 466358. MyDeveloperDay marked 6 inline comments as done. MyDeveloperDay added a comment. Minor documentation changes based on review feedback CHANGES SINCE LAST ACTION https://reviews.llvm.org/D135466/new/ https://reviews.llvm.org/D135466 Files:

[PATCH] D135466: [clang-format] Add support to remove unnecessary semicolons after function definition

2022-10-09 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay marked an inline comment as done. MyDeveloperDay added inline comments. Comment at: clang/unittests/Format/FormatTest.cpp:26773 + Style); + + // These tests are here to show a problem that may not be easily owenpan wrote: > owenpan w

[PATCH] D135466: [clang-format] Add support to remove unnecessary semicolons after function definition

2022-10-09 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. MyDeveloperDay marked an inline comment as done. Closed by commit rG69e772768e95: [clang-format] Add support to remove unnecessary semicolons after function… (authored

[PATCH] D135707: [clang-format] Correctly annotate star/amp in function pointer params

2022-10-11 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/D135707/new/ https://reviews.llvm.org/D135707 ___

[PATCH] D135707: [clang-format] Correctly annotate star/amp in function pointer params

2022-10-14 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. Thanks @rymiel its great to have another active contributor, especially one who seems so focused on github issues. Really appreciate your recent contributions. Can we start including you as one of the default reviewers that we use to review our stuff? Repositor

[PATCH] D132131: [clang-format] Adds a formatter for aligning trailing comments over empty lines

2022-10-14 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. Can you mark your comments done, I think you missed removing some braces that @HazardyKnusperkeks asked you to remove Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D132131/new/ https://reviews.llvm.org/D132131 _

[PATCH] D132131: [clang-format] Adds a formatter for aligning trailing comments over empty lines

2022-10-14 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. I think you also need to rebase to get rid of the "ClangFormatStyleOptions.rst" (which means you need to rebase and rerun dump_format_style.py Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D132131/new/ https://review

[PATCH] D135740: [clang-format] Fix multiple preprocessor if sections parsing incorrectly

2022-10-14 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. Did you test this with nested #if #endif? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D135740/new/ https://reviews.llvm.org/D135740 ___ cfe-commits mailing list cfe-comm

[PATCH] D136100: [clang-format] Do not parse certain characters in pragma directives

2022-10-17 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. Pretty interesting, it looks ok from what I can tell, let the others take a look Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136100/new/ https://reviews.llvm.org/D136100 ___

[PATCH] D135707: [clang-format] Correctly annotate star/amp in function pointer params

2022-10-17 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. I learn a lot from watching the others code review. Definitely not an expert here either, just try to help where I can. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D135707/new/ https://reviews.llvm.org/D135707 ___

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

2022-12-22 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. does it have any unit tests? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D140543/new/ https://reviews.llvm.org/D140543 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mai

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

2022-12-24 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay accepted this revision. MyDeveloperDay added a comment. LGTM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D140543/new/ https://reviews.llvm.org/D140543 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.

[PATCH] D140843: [clang-format] fix template closer followed by >

2023-01-03 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. I'm good with this but I'd be interesting in @owenpan and @HazardyKnusperkeks opinion. Comment at: clang/lib/Format/TokenAnnotator.cpp:169-171 +CurrentToken->getStartOfNonWhitespace() == +CurrentToken->Next->getStart

[PATCH] D140835: [clang-format] Improve UnwrappedLineParser::mightFitOnOneLine()

2023-01-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. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D140835/new/ https://reviews.llvm.org/D140835 ___

[PATCH] D140339: [clang-format] Remove special logic for parsing concept definitions.

2023-01-03 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay accepted this revision. MyDeveloperDay added a comment. Thank you for this patch Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D140339/new/ https://reviews.llvm.org/D140339 ___ cfe-commits

[PATCH] D140767: [clang-format] Require space before noexcept qualifier

2023-01-03 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay accepted this revision. MyDeveloperDay added a comment. Nice! Thank you for another great patch! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D140767/new/ https://reviews.llvm.org/D140767 ___

[PATCH] D140956: [clang-format] Add an option for breaking after C++ attributes

2023-01-04 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. This LGTM Should we care about other forms of attributes like `__stdcall` etc..? I personally don't, just I assume from the tests this isn't covered (maybe just add that limitation to the doc if it is indeed a limitation) Repository: rG LLVM Github Monorepo

[PATCH] D141035: [clang-format] Add an option to insert a newline at EOF if missing

2023-01-05 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 think I've died an gone to heaven!! LGTM... Happy New Year! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141035/new/ https:/

[PATCH] D140843: [clang-format] fix template closer followed by >

2023-01-05 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay accepted this revision. MyDeveloperDay added a subscriber: rymiel. MyDeveloperDay added a comment. This revision is now accepted and ready to land. LGTM, thank you for adding an annotator test, I'd like one of @owenpan, @HazardyKnusperkeks or @rymiel to comment before commit, just

[PATCH] D140956: [clang-format] Add an option for breaking after C++11 attributes

2023-01-05 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay accepted this revision. MyDeveloperDay added a comment. I'm good with that. I like small pieces of contained work and not open ended reviews that try to cover everything. If someone wants to extend this to include the old form, then that can be a completely different review. But t

[PATCH] D140956: [clang-format] Add an option for breaking after C++11 attributes

2023-01-05 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. In D140956#4028288 , @owenpan wrote: > In D140956#4028147 , > @MyDeveloperDay wrote: > >> If someone wants to extend this to include the old form, then that can be a >> completely

[PATCH] D141230: [clang-format-diff.py] give clang-format-diff a job pool (10x speed)

2023-01-09 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay requested changes to this revision. MyDeveloperDay added a comment. This revision now requires changes to proceed. You've lost the first part of the patch in this latest diff Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141230/new/

[PATCH] D141098: [clang-format][NFC] Set DeriveLineEnding to false in config files

2023-01-09 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 kind of agree, it would be nice not to have to use dos2unix and it be set by default. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm

[PATCH] D140843: [clang-format] fix template closer followed by >

2023-01-09 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. In D140843#4029641 , @owenpan wrote: > In D140843#4028142 , > @MyDeveloperDay wrote: > >> I'd like one of @owenpan, @HazardyKnusperkeks or @rymiel to comment before >> commit, ju

[PATCH] D141288: [clang-format] Inherit RightAlign options across scopes

2023-01-09 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay accepted this revision. MyDeveloperDay added a comment. Nice! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141288/new/ https://reviews.llvm.org/D141288 ___ cfe-commits mailing list cfe-co

[PATCH] D140956: [clang-format] Add an option for breaking after C++11 attributes

2023-01-10 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. Before M=0 C=0 T=Unknown S=1 F=0 B=0 BK=0 P=0 Name=identifier L=25 PPK=2 FakeLParens= FakeRParens=0 II=0x1aa92d99900 Text='a' M=0 C=0 T=TemplateOpener S=0 F=0 B=0 BK=0 P=30 Name=less L=26 PPK=2 FakeLParens= FakeRParens=0 II=0x0 Text='<

[PATCH] D137181: [clang-format] Don't use 'PPIndentWidth' inside multi-line macros

2022-11-01 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. This needs a unit test in clang/unittest/Format Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137181/new/ https://reviews.llvm.org/D137181 ___ cfe-commits mailing list cfe

[PATCH] D90121: clang-format: Add a consumer to diagnostics engine

2022-11-03 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. The issue that caused this issue was fix by the fix in https://reviews.llvm.org/rGa92cf5a5a0cd01145f8db2ae09334a8b43a1271b , from the clang-format perspective I don't really feel we need to revisit bringing in these libraries. I think I proved that moving everyt

[PATCH] D137409: [clang-format][NFCish] Alphabetical sort Format.(h|pp)

2022-11-07 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay accepted this revision. MyDeveloperDay added inline comments. This revision is now accepted and ready to land. Comment at: clang/unittests/Format/FormatTest.cpp:23323 "WhitespaceSensitiveMacros: ['FOO', 'BAR']"))); - std::vector NonDe

[PATCH] D91950: [clang-format] Add BreakBeforeInlineASMColon configuration

2022-11-09 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. I think this is them.. https://twitter.com/xen_org/status/1397581514318225412 https://xenproject.org/ CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91950/new/ https://reviews.llvm.org/D91950 ___ cfe-commits m

[PATCH] D137755: [clang-format] Treats &/&& as reference when followed by ',' or ')'.

2022-11-10 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. Its always good if you have a github issue to add that to the summary (just to tie stuff together), if you don't feel free to go over there an log a bug. Comment at: clang/lib/Format/TokenAnnotator.cpp:2355 +NextToken->isOneOf(tok::arrow

[PATCH] D137755: [clang-format] Treats &/&& as reference when followed by ',' or ')'.

2022-11-10 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. On first inspection this looks ok, but lets wait for the others chime in, please add the annotator test too. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137755/new/ https://reviews.llvm.org/D137755 __

[PATCH] D137762: [clang-format] avoid breaking )( with BlockIndent

2022-11-10 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. Definitely needs tests Comment at: clang/lib/Format/ContinuationIndenter.cpp:720 (!Previous.Previous || !Previous.Previous->isOneOf( - tok::kw_for, tok::kw_while, tok::kw_switch)) && + tok:

[PATCH] D137755: [clang-format] Treats &/&& as reference when followed by ',' or ')'.

2022-11-10 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay accepted this revision. MyDeveloperDay added a comment. This revision is now accepted and ready to land. For me this looks good, but please wait for one of the others to double confirm. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D13

cfe-commits@lists.llvm.org

2023-01-24 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. There is part of me that feels we should not rush this in to get it into 16 (which was due to branch today), I feel it might be a good idea to commit this after branching, and let it live in 'main' so we can take for a real test drive CHANGES SINCE LAST ACTION

cfe-commits@lists.llvm.org

2023-01-25 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. I think they have branched (https://clang.llvm.org/docs/ClangFormatStyleOptions.html) the documentation has changed to 17.0 We would need your name and email in order to commit on your behalf. I checked this patch out and tested it on my large project and I didn

cfe-commits@lists.llvm.org

2023-01-26 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. > Good to know it worked as expected on your project as well. I have some concerns (mainly because it feels like it could be quite invasive), hence I'm delaying on the LGTM, I really want to run this on a large code base, which I think if we commit post the bra

[PATCH] D142804: [clang-format] Support clang-format on/off line comments as prefixes

2023-01-30 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 CHANGES SINCE LAST ACTION https://reviews.llvm.org/D142804/new/ https://reviews.llvm.org/D142804 ___ cfe-commits mailing li

[PATCH] D141098: [clang-format][NFC] Set LineEnding to LF in config files

2023-02-06 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. I guess we'll need to revert this until the buildbots are updated. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141098/new/ https://reviews.llvm.org/D141098 ___ cfe-commi

[PATCH] D143178: Add new clang-format alignment option

2023-02-06 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay requested changes to this revision. MyDeveloperDay added a comment. This revision now requires changes to proceed. Please add the unit tests Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143178/new/ https://reviews.llvm.org/D143178

[PATCH] D143178: Add new clang-format alignment option

2023-02-06 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. Can you please do a full context diff see (https://llvm.org/docs/Contributing.html) Comment at: clang/include/clang/Format/Format.h:225 bool AlignCompound; +// TODO +bool AcrossParameterDeclarations; Ok you have to

[PATCH] D143178: Add new clang-format alignment option

2023-02-06 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. please point to a github issue which outlines what you are fixing and why Comment at: clang/lib/Format/Format.cpp:35 #include "clang/Basic/SourceManager.h" +#include "clang/Format/Format.h" #include "clang/Lex/Lexer.h" why movi

[PATCH] D143178: Add new clang-format alignment option

2023-02-06 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. " just to get feedback whether this option would get accepted or not." Honestly I can't tell from the code what problem you are trying to fix. The github issue and unit tests are vital for us to understand what you are trying to achieve (and to ensure you are not

[PATCH] D138446: [clang-format][docs] Add ability to link to specific config options

2023-01-12 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay accepted this revision. MyDeveloperDay added a comment. So if I understand completely its so its easier to pick up an option and share as a hyperlink i.e. `<.>/llvm-project/clang/html/ClangFormatStyleOptions.html#allowallconstructorinitializersonnextline` F26159404: image.pn

[PATCH] D141230: [clang-format-diff.py] give clang-format-diff a job pool (10x speed)

2023-01-12 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay requested changes to this revision. MyDeveloperDay added a comment. This revision now requires changes to proceed. On my laptop running import os; import multiprocessing def main(): num = len(os.sched_getaffinity(0)) numcpu = multiprocessing.cpu_count()

[PATCH] D141230: [clang-format-diff.py] give clang-format-diff a job pool (10x speed)

2023-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. I can see why you have the desire for something that takes 22 seconds and not 16 mins! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.

[PATCH] D141654: [clang-format] Replace DeriveLineEnding and UseCRLF with LineEnding

2023-01-16 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/D141654/new/ https://reviews.llvm.org/D141654 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.

cfe-commits@lists.llvm.org

2023-01-18 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added inline comments. Comment at: clang/unittests/Format/TokenAnnotatorTest.cpp:181 + ASSERT_EQ(Tokens.size(), 5u) << Tokens; + EXPECT_TOKEN(Tokens[1], tok::amp, TT_BinaryOperator); + how does this differentiate from `MyType & val2;` is that

[PATCH] D142412: [clang-format] Put peekNextToken(/*SkipComment=*/true) to good use

2023-01-24 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay accepted this revision. MyDeveloperDay added inline comments. This revision is now accepted and ready to land. Comment at: clang/lib/Format/UnwrappedLineParser.cpp:3592-3601 case tok::kw_bool: // bool is only allowed if it is directly followed by a pare

[PATCH] D127484: [clang-format] Use tabs on GNU style

2022-06-10 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. Oh gosh! It hard to make such changes and someone else not call them a regression, I’m not sure how I feel. We do kind of have to be able to fix bugs and if this gets closer to gnu style then I guess it’s better Repository: rG LLVM Github Monorepo CHANGES SIN

[PATCH] D127390: [clang-format][NFC] Remove unused FormatStyle members

2022-06-11 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. If we remove it from here, then it will get removed from the documentation F23403281: image.png Also if you remove it, then the ClangFormatStyleOptions.rst needs to be regenerated. But somehow I think we need that comment no?

[PATCH] D127484: [clang-format] Use tabs on GNU style

2022-06-11 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. We get heavily criticized for changing defaults between versions, I think if someone want to use GNU style, and they choose to now have this option set it should be done so in their own .clang-format file On reflection, I think its wrong of us to change that now

[PATCH] D127270: [clang-format] Add space in placement new expression

2022-06-11 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. Thank you for the patch, was there a github issue for this? Can we just validate that those requirements are covered here too while we are at it? https://github.com/llvm/llvm-project/issues/54703 https://github.com/llvm/llvm-project/issues/41501 ( I know this is

[PATCH] D127270: [clang-format] Add space in placement new expression

2022-06-11 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added inline comments. Comment at: clang/include/clang/Format/Format.h:3498 +/// \endcode +bool AfterPlacementNew; /// If ``true``, put a space between operator overloading and opening HazardyKnusperkeks wrote: > Please sort after `Aft

[PATCH] D127578: [clang-format] Handle deprecated options in dump_format_style.py

2022-06-12 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay accepted this revision. MyDeveloperDay added a comment. This revision is now accepted and ready to land. Great thanks Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D127578/new/ https://reviews.llvm.org/D127578 ___

[PATCH] D127873: [clang-format] Fix misplacemnt of `*` in declartion of pointer to struct

2022-06-15 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. what about (may not be useful but compiles) struct { int foo; } &&ptr2 = {}; https://godbolt.org/z/rbb8x3hKP Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D127873/new/ https://reviews.llvm.org/D127873

[PATCH] D127873: [clang-format] Fix misplacemnt of `*` in declartion of pointer to struct

2022-06-15 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay requested changes to this revision. MyDeveloperDay added a comment. This revision now requires changes to proceed. Thank you for the patch, just some observations Comment at: clang/unittests/Format/TokenAnnotatorTest.cpp:93 + EXPECT_EQ(Tokens.size(), 10u) << Tok

[PATCH] D127270: [clang-format] Add space in placement new expression

2022-06-16 Thread MyDeveloperDay via Phabricator via cfe-commits
MyDeveloperDay added a comment. I think you are missing a PARSE unit test Comment at: clang/docs/ClangFormatStyleOptions.rst:4261 + **AfterPlacementOperator** (``AfterPlacementOperatorStyle``) :versionbadge:`clang-format 14` +Defines in which cases to put a space betwee

<    11   12   13   14   15   16   17   18   19   20   >