[PATCH] D159233: [clang-format][NFC] Change duplicate config files to symlinks

MyDeveloperDay added a comment. out of interest what happens on windows? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D159233/new/ https://reviews.llvm.org/D159233 ___ cfe-commits mailing list cfe-commit

[PATCH] D159051: [clang-format][NFC] Change EXPECT_EQ to verifyFormat or verifyNoChang

MyDeveloperDay accepted this revision. MyDeveloperDay added a comment. +1 to this before we goto github Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D159051/new/ https://reviews.llvm.org/D159051 ___ cfe-

[PATCH] D154093: [clang-format] Break long string literals in C#, etc.

MyDeveloperDay accepted this revision. MyDeveloperDay added a comment. I'm ok with this if @owenpan is Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D154093/new/ https://reviews.llvm.org/D154093 ___ cfe-c

[PATCH] D154093: [clang-format] Break long string literals in C#, etc.

MyDeveloperDay added inline comments. Comment at: clang/unittests/Format/FormatTestCSharp.cpp:221 TEST_F(FormatTestCSharp, CSharpFatArrows) { - verifyFormat("Task serverTask = Task.Run(async() => {"); + verifyIncompleteFormat("Task serverTask = Task.Run(async() => {"); veri

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

MyDeveloperDay added a comment. In D138263#3936536 , @HazardyKnusperkeks wrote: > In D138263#3936007 , @owenpan wrote: > >> I suppose it's fairly easy to annotate the `l_brace` of a namespace? If so, >> then wou

[PATCH] D155273: [clang-format] Add TypeNames option to disambiguate types/objects

MyDeveloperDay added a comment. I used isSimpleTypeSpecifier() in the east/west const fixer, I think this could probably help there to catch more places too.. (you don't need to do that here) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D155273/new

[PATCH] D154755: [clang-format] Fix formatting of if statements with BlockIndent

MyDeveloperDay requested changes to this revision. MyDeveloperDay added inline comments. This revision now requires changes to proceed. Comment at: clang/unittests/Format/FormatTest.cpp:25504 + +#if 0 verifyFormat("if (quitelongarg !=\n" we don't do this. ==

[PATCH] D154755: [clang-format] Fix formatting of if statements with BlockIndent

MyDeveloperDay added a comment. It looks like this was added in D109557: Adds a BlockIndent option to AlignAfterOpenBracket I'm trying to understand what bad stuff happens if you remove the "if" from `return !(Previous && Previous->is(tok::kw_for));` Reposit

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

MyDeveloperDay accepted this revision. MyDeveloperDay added a comment. This revision is now accepted and ready to land. This looks good can you wait for one of the others to accept too Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D155239/new/ https

[PATCH] D155273: [clang-format] Add TypeNames option to disambiguate types/objects

MyDeveloperDay added a comment. Nice! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D155273/new/ https://reviews.llvm.org/D155273 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm

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

MyDeveloperDay added a comment. We should never make assumptions about what people do/don't use F28299562: image.png If you have this you have to honour it.. if 'SpacesInParentheses ' was true then 'InAttributeSpecifiers' needs to be true by default, shouldn

[PATCH] D154184: [clang-format] Correctly annotate */&/&& in operator function calls

MyDeveloperDay added a comment. In D154184#4509744 , @rymiel wrote: > Shouldn't that regression already be handled by D155358 > ? nice catch @rymiel! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

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

MyDeveloperDay added a comment. We should never make assumptions about what people do/don't use F28299562: image.png If you have this you have to honour it.. if 'SpacesInParentheses ' was true then 'InAttributeSpecifiers' needs to be true by default, shouldn

[PATCH] D154755: [clang-format] Fix formatting of if statements with BlockIndent

MyDeveloperDay added inline comments. Comment at: clang/unittests/Format/FormatTest.cpp:25486 "comment\n" - " return;\n" "}", Why remove? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://r

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

MyDeveloperDay added inline comments. Comment at: clang/lib/Format/Format.cpp:1035 IO.mapOptional("SpaceInEmptyBlock", Style.SpaceInEmptyBlock); -IO.mapOptional("SpaceInEmptyParentheses", Style.SpaceInEmptyParentheses); IO.mapOptional("SpacesBeforeTrailingComments",

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

MyDeveloperDay added inline comments. Comment at: clang/lib/Format/TokenAnnotator.cpp:4046 + ? (Style.SpacesInParens == FormatStyle::SIPO_Always || + Style.SpacesInParentheses) + : true; isn't SpacesInParentheses mappe

[PATCH] D150083: [clang-format] ObjCPropertyAttributeOrder to sort ObjC property attributes

MyDeveloperDay added a comment. may remove the other build system files, I don't think its our responsiblity to keep them upto date. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D150083/new/ https://reviews.llvm.org/D150083 ___ cfe-commits m

[PATCH] D150083: [clang-format] ObjCPropertyAttributeOrder to sort ObjC property attributes

MyDeveloperDay added a comment. I don't have a problem with this, but I'm not an ObjC person so I can't really review from the validity of what its changing. If @owenpan and @HazardyKnusperkeks don't have an problem with this then I don't either as it likely won't impact most C++ users.

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

MyDeveloperDay added a comment. I reckon you could detect the `=` condition between the matching paren don't you? I ran this on my quite large code base and this was one of the only issues I've seen thus far.. This is a great feature, for my codebase this generated changes in about 10% of all

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

MyDeveloperDay added a comment. Here is a potential failure scenario (test.cpp) void *bar(); void foo() { void *p; int i = 0; while ((p = bar())) { i++; } } (.clang-format) Language: Cpp RemoveParentheses: ReturnStatement clang-format test.cpp > test2

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

MyDeveloperDay added inline comments. Comment at: clang/lib/Format/BreakableToken.cpp:199 +if (EscapeSequence && Advance == 2) { + switch (Text[1]) { sstwcw wrote: > MyDeveloperDay wrote: > > Can we add a unit test for escape sequences > \X which I ass

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

MyDeveloperDay added inline comments. Comment at: clang/lib/Format/BreakableToken.cpp:199 +if (EscapeSequence && Advance == 2) { + switch (Text[1]) { Can we add a unit test for escape sequences > \X which I assume this handles Repository: rG LLVM G

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

MyDeveloperDay added a comment. What would happen for say unicode escapes? x = "XXXThis is a string \u0041 With stuff after"; would it break after the \u? x = "XXXThis is a string \u 0041 With stuff after"; Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://revie

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

MyDeveloperDay accepted this revision. MyDeveloperDay added a comment. This revision is now accepted and ready to land. A thousand times yes! Comment at: clang/docs/ClangFormatStyleOptions.rst:4354 + + * ``RPS_None`` (in configuration: ``None``) +Do not remove parentheses.

[PATCH] D154467: [clang-format] Add Verilog suffixes to the scripts

MyDeveloperDay added a comment. What would happen for say unicode escapes? x = "XXXThis is a string \u0041 With stuff after"; would it break after the \u? x = "XXXThis is a string \u 0041 With stuff after"; Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://revie

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

MyDeveloperDay added a comment. What would happen in a \r\n example Comment at: clang/lib/Format/BreakableToken.cpp:207 + case 't': + case 'v': +AfterSpace = SplitPoint + 2; Are you testing \r \v \f Repository: rG LLVM Github Monorepo CHA

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

This revision was automatically updated to reflect the committed changes. Closed by commit rG899c86779440: [clang-format] Fixed bad performance with enabled qualifier fixer. (authored by Sedeniono, committed by MyDeveloperDay). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION ht

[PATCH] D154091: [clang-format] Prefer breaking long strings at new lines

MyDeveloperDay added a comment. is there a git issue for this, I'm struggling to understand the problem we are solving. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D154091/new/ https://reviews.llvm.org/D154091 ___

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

MyDeveloperDay added a subscriber: aaron.ballman. MyDeveloperDay added a comment. @owenpan @HazardyKnusperkeks @rymiel what are your feeling on how we should close old clang-format reviews like this? (@aaron.ballman any thoughts on how it should be done?) we end up with lots of reviews that ei

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

MyDeveloperDay added a comment. For us to land this for you we'll need your name and email Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D153228/new/ https://reviews.llvm.org/D153228 ___ cfe-commits maili

[PATCH] D153585: [clang-format] Fix align consecutive declarations over function pointers

MyDeveloperDay added a comment. Can you reference the GitHub bug Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D153585/new/ https://reviews.llvm.org/D153585 ___ cfe-commits mailing list cfe-commits@lists.

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

MyDeveloperDay accepted this revision. MyDeveloperDay added a comment. Thank you for the detailed analysis Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D153228/new/ https://reviews.llvm.org/D153228 ___ c

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

MyDeveloperDay added inline comments. Comment at: clang/lib/Format/Format.cpp:1484 LLVMStyle.PenaltyBreakFirstLessLess = 120; + LLVMStyle.PenaltyBreakReturn = 100; LLVMStyle.PenaltyBreakString = 1000; is 100 the current penalty? Comment

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

MyDeveloperDay added a comment. My only concern is that this changes behaviour without someone making the conscious decision to make that change, now as much as I can't understand why someone would want to put the `}` onto the same line, it will only take on person to say "hey I liked it like t

[PATCH] D153338: [clang-format] vim integration: Mention python3 variant of bindings

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/D153338/new/ https://reviews.llvm.org/D153338 ___

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

MyDeveloperDay added a comment. In principle I like this as its annoyed me for years Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D153205/new/ https://reviews.llvm.org/D153205 ___ cfe-commits mailing lis

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

MyDeveloperDay added inline comments. Comment at: clang/unittests/Format/FormatTest.cpp:25511 + // Designated initializers. + verifyFormat("int LongVariable[1] = {\n" + "[0] = 1000, [1] = 2000\n" nit [2] Repos

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

MyDeveloperDay added inline comments. Comment at: clang/lib/Format/FormatToken.cpp:79 bool FormatToken::opensBlockOrBlockTypeList(const FormatStyle &Style) const { + auto bk = getBlockKind(); // C# Does not indent object initialisers as continuations. Hazard

[PATCH] D153109: [clang-format][NFC] Clean up unit tests

MyDeveloperDay accepted this revision. MyDeveloperDay added a comment. Nice! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D153109/new/ https://reviews.llvm.org/D153109 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm

[PATCH] D152804: [clang-format] Propose a new solution to - Fix overlapping replacements before PPDirectives

MyDeveloperDay added a comment. So whilst this solves the unit tests (and the golden.h issue) that were added it DOESN'T solve the overlapping replacements that was the original bug. struct foo { void test() { #if 1 #else #endif } #if 1 private: #endif }; T

[PATCH] D152804: [clang-format] Propose a new solution to - Fix overlapping replacements before PPDirectives

MyDeveloperDay added a comment. but it does seem to resolve https://github.com/llvm/llvm-project/issues/63170 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D152804/new/ https://reviews.llvm.org/D152804 __

[PATCH] D152804: [clang-format] Propose a new solution to - Fix overlapping replacements before PPDirectives

MyDeveloperDay added a comment. ok this still doesn't resolve the original https://github.com/llvm/llvm-project/issues/62892 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D152804/new/ https://reviews.llvm.org/D152804 __

[PATCH] D152804: [clang-format] Propose a new solution to - Fix overlapping replacements before PPDirectives

MyDeveloperDay created this revision. MyDeveloperDay added a reviewer: owenpan. MyDeveloperDay added projects: clang, clang-format. Herald added a project: All. Herald added reviewers: rymiel, HazardyKnusperkeks. MyDeveloperDay requested review of this revision. Propose a new solution to D151954:

[PATCH] D152473: [clang-format] Add test case for issue 63170

MyDeveloperDay added a comment. Part of my concern was that the reversion removed a unit test, which effectively regressed a different fix, I'm not comfortable with that. I think we can resolve the original problem with the following fix. diff --git a/clang/lib/Format/UnwrappedLineFormatter.c

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

MyDeveloperDay added a comment. did you cover the case where I have a namespaced, static foo::Bar->operator==() foo::Bar->m_oper->operator==() foo::Bar->m_oper.operator==() foo::Bar.operator==() Comment at: clang/lib/Format/TokenAnnotator.cpp:4207-4210 +if (!To

[PATCH] D152473: [clang-format] Add test case for issue 63170

MyDeveloperDay added a comment. My additional concern is, is the original patch is the root cause of the regression?, so I’m struggling to understand why this in particular is being reverted or are we just going backwards through all commits? Repository: rG LLVM Github Monorepo CHANGES SINC

[PATCH] D152473: [clang-format] Add test case for issue 63170

MyDeveloperDay added a comment. In D152473#4409652 , @phosek wrote: > LGTM @phosek can I ask you review https://llvm.org/docs/CodeReview.html When providing an unqualified LGTM (approval to commit), it is the responsibility of the reviewer to have re

[PATCH] D152473: [clang-format] Add test case for issue 63170

MyDeveloperDay added a comment. In D152473#4409146 , @paulkirth wrote: > The point of this patch was to show a regression. I'm not trying to fix > anything per se. I'd like https://reviews.llvm.org/D151954 and anything that > depends on it reverted, si

[PATCH] D152473: [clang-format] Add test case for issue 63170

MyDeveloperDay reopened this revision. MyDeveloperDay added a comment. This revision is now accepted and ready to land. Did this need an immediate revert? was the unit tests failing? Comment at: clang/test/Format/overlapping-lines.cpp:1 +// RUN: grep -Ev "// *[A-Z-]+:" %s | cla

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

MyDeveloperDay added a comment. In D152443#4407438 , @KitsuneAlex wrote: > I had some issues with Git there because i messed up a merge, so the diff was > bad. Here's the proper diff. > NOTE: Clang-Format Team Automated Review Comment keep you hones

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

MyDeveloperDay added inline comments. Comment at: clang/docs/ClangFormatStyleOptions.rst:4709 + +**SpaceAfterOperatorKeyword** (``Boolean``) :versionbadge:`clang-format 17` :ref:`¶ ` + If ``true``, a space will be inserted after the 'operator' keyword. could we

[PATCH] D152473: [clang-format] Add test case for issue 63170

MyDeveloperDay added a comment. I presume the test fail, we don't check in tests that just fail without a fix, or the build would be stale. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D152473/new/ https://reviews.llvm.org/D152473 ___

[PATCH] D151761: clang-format: Add AlignConsecutiveShortCaseStatements

MyDeveloperDay added a comment. In D151761#4400056 , @galenelias wrote: > In D151761#4394758 , > @MyDeveloperDay wrote: > >> did you consider a case where the case falls through? (i.e. no return) >> >> "case l

[PATCH] D152305: [clang-format] Add the KeepEmptyLinesAtEOF option

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/D152305/new/ https://reviews.llvm.org/D152305 ___ cfe-commits mailing li

[PATCH] D151761: clang-format: Add AlignConsecutiveShortCaseStatements

MyDeveloperDay added a comment. did you consider a case where the case falls through? (i.e. no return) "case log::info :return \"info\";\n" "case log::warning :\n" "default : return \"default\";\n" CHANGES SINCE LAST ACTION https://reviews.llvm.org/D151761/new/ https://re

[PATCH] D151761: clang-format: Add AlignConsecutiveShortCaseStatements

MyDeveloperDay added inline comments. Comment at: clang/docs/ClangFormatStyleOptions.rst:798 + +case log::info: return "info:"; +case log::warning: return "warning:"; Do you think the documentation should give examples of the the other cases? CHANG

[PATCH] D151761: clang-format: Add AlignConsecutiveShortCaseStatements

MyDeveloperDay added a comment. Apart from the documentation this looks fine.. thats probably outside the scope of this change CHANGES SINCE LAST ACTION https://reviews.llvm.org/D151761/new/ https://reviews.llvm.org/D151761 ___ cfe-commits mailing

[PATCH] D151761: clang-format: Add AlignConsecutiveShortCaseStatements

MyDeveloperDay added inline comments. Comment at: clang/docs/ClangFormatStyleOptions.rst:790 + +**AlignConsecutiveShortCaseLabels** (``AlignConsecutiveStyle``) :versionbadge:`clang-format 17` :ref:`¶ ` + Style of aligning consecutive short case labels. HazardyK

[PATCH] D151761: clang-format: Add AlignConsecutiveShortCaseLabels

MyDeveloperDay added inline comments. Comment at: clang/docs/ClangFormatStyleOptions.rst:790 + +**AlignConsecutiveShortCaseLabels** (``AlignConsecutiveStyle``) :versionbadge:`clang-format 17` :ref:`¶ ` + Style of aligning consecutive short case labels. galeneli

[PATCH] D151761: clang-format: Add AlignConsecutiveShortCaseLabels

MyDeveloperDay added inline comments. Comment at: clang/docs/ClangFormatStyleOptions.rst:790 + +**AlignConsecutiveShortCaseLabels** (``AlignConsecutiveStyle``) :versionbadge:`clang-format 17` :ref:`¶ ` + Style of aligning consecutive short case labels. Did you

[PATCH] D151578: [Format/ObjC] Support NS_ASSUME_NONNULL_BEGIN and FOUNDATION_EXPORT in ObjC language guesser

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/D151578/new/ https://reviews.llvm.org/D151578 ___

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

MyDeveloperDay added a comment. I think we need to extract the context of the test from RenameTests to ensure we have it covered here. I don't personally normally run the entire LLVM suite. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D151047/new/

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

MyDeveloperDay added a comment. We don't normally land broken tests, even if they are disabled, its better for us if we get a fix at the same time ;-) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D151145/new/ https://reviews.llvm.org/D151145

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

MyDeveloperDay accepted this revision. MyDeveloperDay added a comment. This revision is now accepted and ready to land. Thank you for the indepth explaination in https://github.com/llvm/llvm-project/issues/59178, that was really helpful for me trying to understand what the problem was. I thik y

[PATCH] D150083: [clang-format] ObjCPropertyAttributeOrder to sort ObjC property attributes

MyDeveloperDay added inline comments. Comment at: clang/unittests/Format/ObjCPropertyAttributeOrderFixerTest.cpp:156 + + for(auto const& Attribute: getAllObjCAttributes()) { +Style.ObjCPropertyAttributeOrder = { "FIRST", Attribute, "LAST" }; I'm not a mass

[PATCH] D150506: Migrate {starts,ends}with_insensitive to {starts,ends}_with_insensitive (NFC)

MyDeveloperDay accepted this revision. MyDeveloperDay added a comment. This revision is now accepted and ready to land. no concerns from the clang-format front. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D150506/new/ https://reviews.llvm.org/D150

[PATCH] D150083: [clang-format] ObjCPropertyAttributeOrder to sort ObjC property attributes

MyDeveloperDay added inline comments. Comment at: clang/unittests/Format/ObjCPropertyAttributeOrderFixerTest.cpp:77 + verifyFormat("@property(a, b, c) int p;", "@property(b, a, c) int p;", Style); + verifyFormat("@property(a, b, c) int p;", "@property(c, b, a) int p;", Style)

[PATCH] D150083: [clang-format] ObjCPropertyAttributeOrder to sort ObjC property attributes

MyDeveloperDay added a comment. Interesting Comment at: clang/lib/Format/ObjCPropertyAttributeOrderFixer.cpp:69 + // Ignore the comma separators. + continue; +} else if (Tok->isOneOf(tok::identifier, tok::kw_class)) { You need a unit tests that t

[PATCH] D149647: [NFC][Clang]Fix static analyzer tool remarks about large copies by value

MyDeveloperDay added inline comments. Comment at: clang/lib/Format/Format.cpp:3486-3489 + Expanded.InsertBraces = true; + Passes.emplace_back([&](const Environment &Env) { +return BracesInserter(Env, Expanded).process(/*SkipAnnotation=*/true); }); ---

[PATCH] D149647: [NFC][Clang]Fix static analyzer tool remarks about large copies by value

MyDeveloperDay added a comment. ok! I'm not a massive lambda expert, but aren't we passing by const reference e.g. `const FormatStyle &Style`, can someone give me a lession as to why it thinks its by value? maybe I'm looking at the wrong "pass by" Repository: rG LLVM Github Monorepo CHANGE

[PATCH] D149647: [NFC][Clang]Fix static analyzer tool remarks about large copies by value

MyDeveloperDay added a comment. Can you link to the Coverity issue so we can see what it was complaining about? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D149647/new/ https://reviews.llvm.org/D149647

[PATCH] D149643: [clang-format] Correctly limit formatted ranges when specifying qualifier alignment

MyDeveloperDay added a comment. Thanks for the patch...this tells me people are starting to use this feature in anger!! i.e. your formatting via git-clang-format (which is brave!) ;-) which means you have the code modifying features perhaps on my default... LGTM, if you will need someone to lan

[PATCH] D148467: [clang-format] Add a new AfterCSharpProperty to BraceWrapping

MyDeveloperDay added a comment. Just an update on this, it works well but some of the cases are not tolerant to comments, I'm working my way through those issues CHANGES SINCE LAST ACTION https://reviews.llvm.org/D148467/new/ https://reviews.llvm.org/D148467

[PATCH] D149562: [clang-format] Stop comment disrupting indentation of Verilog ports

MyDeveloperDay added inline comments. Comment at: clang/include/clang/Format/Format.h:4027 + /// is probably for the port on the following line instead of the parenthesis + /// it follows. /// \code This seems an odd corner case Repository: rG LLVM Githu

[PATCH] D125171: [clang-format] Add a new clang-format option AlwaysBreakBeforeFunctionParameters

MyDeveloperDay added inline comments. Comment at: clang/lib/Format/Format.cpp:347 +IO.enumCase(Value, "false", FormatStyle::FPBS_Leave); +IO.enumCase(Value, "true", FormatStyle::FPBS_Always); + } yes remove these... this is why you need to mark the comme

[PATCH] D145435: [clang-format] Choose style (file) from within code for use in IDEs

MyDeveloperDay added a comment. if we don't plan on fixing the issues please abandon the review CHANGES SINCE LAST ACTION https://reviews.llvm.org/D145435/new/ https://reviews.llvm.org/D145435 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D149561: [clang-format] Recognize Verilog edge identifiers

MyDeveloperDay added a comment. Could you add anotator tests? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D149561/new/ https://reviews.llvm.org/D149561 ___ cfe-commits mailing list cfe-commits@lists.llv

[PATCH] D149088: [clang-format] Add run-clang-format.py script.

MyDeveloperDay added inline comments. Comment at: clang/tools/run-clang-format.py:63 +default="c,cc,cpp,cxx,c++,h,hh,hpp,hxx,h++") +parser.add_argument('-style', +help='formatting style', what about ``` run-cl

[PATCH] D149088: [clang-format] Add run-clang-format.py script.

MyDeveloperDay added a comment. is it possible to pass other arguments like `-n` if not running in place is there any protection from prevent stdout/stderr from overlapping..? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D149088/new/ https://revi

[PATCH] D145262: [clang-format] Treat AttributeMacros more like attribute macros

MyDeveloperDay added inline comments. Comment at: clang/lib/Format/ContinuationIndenter.h:231 /// Used e.g. to break like: + /// ``` /// functionCall(Parameter, otherCall( MyDeveloperDay wrote: > If you want this to look like code you need to use \code \en

[PATCH] D145262: [clang-format] Treat AttributeMacros more like attribute macros

MyDeveloperDay added inline comments. Comment at: clang/lib/Format/ContinuationIndenter.h:231 /// Used e.g. to break like: + /// ``` /// functionCall(Parameter, otherCall( If you want this to look like code you need to use \code \endcode CHANGES SINCE LA

[PATCH] D125171: [clang-format] Add a new clang-format option AlwaysBreakBeforeFunctionParameters

MyDeveloperDay added inline comments. Comment at: clang/include/clang/Format/Format.h:816 +/// \code +///original: +///int someFunction(int arg1, int arg2); Can you format this as we do the other documentation? Repository: rG LLVM Github

[PATCH] D148467: [clang-format] Add a new AfterCSharpProperty to BraceWrapping

MyDeveloperDay updated this revision to Diff 515372. MyDeveloperDay added a comment. Handle the case string Foo { set;get; } CHANGES SINCE LAST ACTION https://reviews.llvm.org/D148467/new/ https://reviews.llvm.org/D148467 Files: clang/docs/ClangFormatStyleOptions.rst clang

[PATCH] D148467: [clang-format] Add a new AfterCSharpProperty to BraceWrapping

MyDeveloperDay planned changes to this revision. MyDeveloperDay added a comment. There is another case I need to cover Style.BraceWrapping.AfterCSharpProperty = false; Style.AllowShortCSharpPropertiesOnASingleLine = false; Style.AlwaysBreakBetweenShortCSharpProperties = false; Style.Brace

[PATCH] D148467: [clang-format] Add a new AfterCSharpProperty to BraceWrapping

MyDeveloperDay updated this revision to Diff 515298. MyDeveloperDay marked 10 inline comments as done. MyDeveloperDay added a comment. Address review comment, add convenience functions to simplify conditions CHANGES SINCE LAST ACTION https://reviews.llvm.org/D148467/new/ https://reviews.llvm.

[PATCH] D148777: [clang-format] Hanlde leading whitespaces for JSON files

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

[PATCH] D148467: [clang-format] Add a new AfterCSharpProperty to BraceWrapping

MyDeveloperDay added a comment. In my C# project, these settings give me just what we tend to use. BreakBeforeBraces: Custom BraceWrapping: AfterFunction: true AfterCSharpProperty: true AllowShortCSharpPropertiesOnASingleLine: false AlwaysBreakBetweenShortCSharpProperties: false

[PATCH] D148467: [clang-format] Add a new AfterCSharpProperty to BraceWrapping

MyDeveloperDay updated this revision to Diff 515068. MyDeveloperDay added a comment. Sorry had a clang-format reflow comments moment, revert the unrelated comment changes simplify the if's a little CHANGES SINCE LAST ACTION https://reviews.llvm.org/D148467/new/ https://reviews.llvm.org/D1484

[PATCH] D148467: [clang-format] Add a new AfterCSharpProperty to BraceWrapping

MyDeveloperDay updated this revision to Diff 515060. MyDeveloperDay added a comment. Adds additional options to give us much greater control over how we format C# properties especially auto properties (Submitting this for safe keeping, I need to try and minimize the if expression, I'm sure it c

[PATCH] D148467: [clang-format] Add a new AfterCSharpProperty to BraceWrapping

MyDeveloperDay added a comment. There is more to this than meets the eye.. what we have so far, from existing `AfterFunction` use and the propsed here `AfterCSharpProperty` is... public Foo {<--- controlled by **AfterFunction **(rightly or wrongly) get {

[PATCH] D148467: [clang-format] Add a new AfterCSharpProperty to BraceWrapping

MyDeveloperDay updated this revision to Diff 514684. MyDeveloperDay added a comment. upload the correct patch file CHANGES SINCE LAST ACTION https://reviews.llvm.org/D148467/new/ https://reviews.llvm.org/D148467 Files: clang/docs/ClangFormatStyleOptions.rst clang/docs/ReleaseNotes.rst

[PATCH] D148467: [clang-format] Add a new AfterCSharpProperty to BraceWrapping

MyDeveloperDay updated this revision to Diff 514682. MyDeveloperDay added a comment. add init support and fix indentation issue when only one property is defined but the is an auto-property CHANGES SINCE LAST ACTION https://reviews.llvm.org/D148467/new/ https://reviews.llvm.org/D148467 File

[PATCH] D148467: [clang-format] Add a new AfterCSharpProperty to BraceWrapping

MyDeveloperDay added a comment. >> public Foo { >> set; >> get; >> } > > At least from my experience, the getter is specified before the setter, > though I'm unsure how important this is in your eyes. Lets hold off the idea of swapping `set;get` around from this patch, but this

[PATCH] D148467: [clang-format] Add a new AfterCSharpProperty to BraceWrapping

MyDeveloperDay planned changes to this revision. MyDeveloperDay added a comment. need to handle `init;` CHANGES SINCE LAST ACTION https://reviews.llvm.org/D148467/new/ https://reviews.llvm.org/D148467 ___ cfe-commits mailing list cfe-commits@lists.

[PATCH] D148467: [clang-format] Add a new AfterCSharpProperty to BraceWrapping

MyDeveloperDay updated this revision to Diff 514586. MyDeveloperDay marked 8 inline comments as done. MyDeveloperDay added a comment. Address review comments, still would like to solve the indenting issue. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D148467/new/ https://reviews.llvm.or

[PATCH] D148473: [clang-format] C# short ternary operator misinterpreted as a CSharpNullable

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG799b794d7699: [clang-format] C# short ternary operator misinterpreted as a CSharpNullable (authored by MyDeveloperDay). Changed prior to commit: h

[PATCH] D148473: [clang-format] C# short ternary operator misinterpreted as a CSharpNullable

MyDeveloperDay updated this revision to Diff 514343. MyDeveloperDay added a comment. simplify negative if CHANGES SINCE LAST ACTION https://reviews.llvm.org/D148473/new/ https://reviews.llvm.org/D148473 Files: clang/lib/Format/TokenAnnotator.cpp clang/unittests/Format/TokenAnnotatorTest.

[PATCH] D148472: [clang-format] CSharp don't allow there not to be a space between `is` and `[`

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 rG63395cb0b69d: [clang-format] CSharp don't allow there not to be a space between `is` and `[` (author

[PATCH] D143870: [clang-format] Remove all include duplicates not only those in the same block

MyDeveloperDay added a comment. > Regarding the comment that I must not change existing tests: I think this > rule is too strict, because those tests are mostly regression tests. > But a regression tests does not test for correctness. So if a test had > already a wrong assumption, it must be ch

[PATCH] D148467: [clang-format] Add a new AfterCSharpProperty to BraceWrapping

MyDeveloperDay added inline comments. Comment at: clang/include/clang/Format/Format.h:1220 +/// \endcode +bool AfterCSharpProperty; }; HazardyKnusperkeks wrote: > MyDeveloperDay wrote: > > HazardyKnusperkeks wrote: > > > Please sort. :) > > Are we sure

  1   2   3   4   5   6   7   8   9   10   >