[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] 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] 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] 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 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] 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 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: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. 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] 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. 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] 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] 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] 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] 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] 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 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. 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] 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. 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] 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] 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 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. 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] 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] D136100: [clang-format] Do not parse certain characters in pragma directives

MyDeveloperDay added a comment. In D136100#3988424 , @hans wrote: > Chromium is seeing a formatting regression after this: > https://github.com/llvm/llvm-project/issues/59473 Can we get more specific about what Chromium is seeing? Repository: rG LLV

[PATCH] D139801: [clang-format] Adds 'friend' to QualifierOrder.

MyDeveloperDay added a comment. I guess in principle this looks ok, but lets see what the others think @owenpan , @HazardyKnusperkeks Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D139801/new/ https://reviews.llvm.org/D139801 _

[PATCH] D138373: [clang-format] Don't eat two semicolons after namespace

MyDeveloperDay accepted this revision. MyDeveloperDay added a comment. This revision is now accepted and ready to land. Feels like this would only show if someone did namespace A { };; Which seems odd Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.o

[PATCH] D138373: [clang-format] Don't eat two semicolons after namespace

MyDeveloperDay added inline comments. Comment at: clang/lib/Format/UnwrappedLineParser.cpp:2999 -// Munch the semicolon after a namespace. This is more common than one would -// think. Putting the semicolon into its own line is very ugly. This comment

[PATCH] D139834: [clang-format] AllowShortCompoundRequirementOnASingleLine

MyDeveloperDay added inline comments. Comment at: clang/lib/Format/Format.cpp:809 Style.AllowShortCaseLabelsOnASingleLine); +IO.mapOptional("AllowShortCompoundRequirementOnASingleLine", + Style.AllowShortCompoundRequirementOnASingleLine);

[PATCH] D139801: [clang-format] Adds 'friend' to QualifierOrder.

MyDeveloperDay added a comment. I know I have situations where I like `inline` and `static` to be in a certain order to avoid them getting constantly swapped around, they are never going to be after the type but I still want them the same way, I'd be ok with allow `friend` for the same reason.

[PATCH] D139211: [WIP][clang-format] Properly handle the C11 _Generic keyword.

MyDeveloperDay accepted this revision. MyDeveloperDay added inline comments. This revision is now accepted and ready to land. Comment at: clang/lib/Format/ContinuationIndenter.cpp:1691 +(CurrentState.IsCSharpGenericTypeConstraint) || GenericSelection || (Style.is

[PATCH] D131386: [clang-tidy] Added `ConstAlignment` option to `misc-const-correctness`

MyDeveloperDay added a comment. > The reason for me to work on this feature is the fact that clang-format > doesn't work reliable for changing the location of the const. After using > clang-tidy and clang-format some consts were still one the right hand side Its true we made the decision if cla

[PATCH] D128372: [Clang-Tidy] Empty Check

MyDeveloperDay added a comment. Ironically as an aside... looking back at the review because of the discource article. The whole reason why I added the [[nodiscard]] checker to clang-tidy 4 years ago was to cover exactly this use case...and many others like it where users have a function which

[PATCH] D139937: [clang-format] fix typo in doc for SLS_Inline

MyDeveloperDay requested changes to this revision. MyDeveloperDay added a comment. This revision now requires changes to proceed. I'm not sure if that is correct, as that doesn't fit the example (unless the example is wrong) It feels like its saying merge if its an argument of a function (sort i

[PATCH] D139834: [clang-format] AllowShortCompoundRequirementOnASingleLine

MyDeveloperDay added inline comments. Comment at: clang/lib/Format/Format.cpp:1265 LLVMStyle.AllowShortCaseLabelsOnASingleLine = false; + LLVMStyle.AllowShortCompoundRequirementOnASingleLine = true; LLVMStyle.AllowShortEnumsOnASingleLine = true; Backl1ght

[PATCH] D139786: [clang-format] AllowShortRequiresExpressionOnASingleLine

MyDeveloperDay added a comment. ditto my concerns with D139834: [clang-format] AllowShortCompoundRequirementOnASingleLine as to if we should have a Leave option CHANGES SINCE LAST ACTION https://reviews.llvm.org/D139786/new/ https://reviews.llvm.org/D13978

[PATCH] D139937: [clang-format] make doc for SLS_Inline more clearly

MyDeveloperDay accepted this revision. MyDeveloperDay added a comment. This revision is now accepted and ready to land. I think given that it confused you I'm fine with this change CHANGES SINCE LAST ACTION https://reviews.llvm.org/D139937/new/ https://reviews.llvm.org/D139937 __

[PATCH] D139834: [clang-format] AllowShortCompoundRequirementOnASingleLine

MyDeveloperDay added inline comments. Comment at: clang/lib/Format/Format.cpp:1265 LLVMStyle.AllowShortCaseLabelsOnASingleLine = false; + LLVMStyle.AllowShortCompoundRequirementOnASingleLine = true; LLVMStyle.AllowShortEnumsOnASingleLine = true; HazardyKnu

[PATCH] D140105: [clang-format] add missing config parse test for short lambda

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

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

MyDeveloperDay requested changes to this revision. MyDeveloperDay added a comment. This revision now requires changes to proceed. I'm not a fan of this approach of adding a "C" language, mainly because of the `.h` problem so ultimately it doesn't solve your problem. I think this is overkill for

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

MyDeveloperDay added a comment. I actually thought you were on the right lines with your change to parseAccessSpecifier() surely if you see "private:" "public:" or "protected:" but not `public::` that's when you call parseAccessSpecifier() I think mostly I believe clang-format is a 96% done de

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

MyDeveloperDay added a comment. For me the push back on the second language and having to pass it on the command line for the .h case is about VS Code, Vim, Visual Studio, Emacs having to recognize what language they are in and having to pass the -language=XXX flag, now we are pushing the inte

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

MyDeveloperDay added a comment. Isn't the reality that developers using `delete/new/try/catch/interface/public/protected/private/async/var/let` as variables the real problem? I'm slightly averse to pandering to them, and I definitely don't want to make my world more complex just to accommodate

[PATCH] D117520: [clang-format] Fix SeparateDefinitionBlocks issues

MyDeveloperDay added inline comments. Comment at: clang/lib/Format/DefinitionBlockSeparator.cpp:44 + if (CurrentToken->isOneOf(tok::kw_class, tok::kw_struct) || (Style.isJavaScript() && CurrentToken->TokenText == "function")) return true;

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

MyDeveloperDay requested changes to this revision. MyDeveloperDay added inline comments. This revision now requires changes to proceed. Comment at: clang/unittests/Format/FormatTest.cpp:9468 "typename ();"); verifyFormat("auto aa

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

MyDeveloperDay added a comment. run git clang-format before submitting (if making the patch from staged files) CHANGES SINCE LAST ACTION https://reviews.llvm.org/D117398/new/ https://reviews.llvm.org/D117398 ___ cfe-commits mailing list cfe-commits

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

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

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

MyDeveloperDay added inline comments. Comment at: .arclint:15-16 } -} +} \ No newline at end of file curdeius wrote: > Please don't change unrelated files. I don't know about others but I personally don't use arcanist... I stage my files, make a patch and up

[PATCH] D117520: [clang-format] Fix SeparateDefinitionBlocks issues

MyDeveloperDay accepted this revision. MyDeveloperDay added a comment. This revision is now accepted and ready to land. LGTM, I'll pull it down and try it here.. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D117520/new/ https://reviews.llvm.org/D117520 _

[PATCH] D117520: [clang-format] Fix SeparateDefinitionBlocks issues

MyDeveloperDay added a comment. I'm getting some sort of seemingly empty replacements going on, after `public:` I can't quite put my finger on what's happening. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D117520/new/ https://reviews.llvm.org/D117520 _

[PATCH] D117520: [clang-format] Fix SeparateDefinitionBlocks issues

MyDeveloperDay added a comment. For class Test { public: static void foo() { } }; F21703157: image.png $ clang-format -n test.cxx test.cxx:3:8: warning: code should be clang-formatted [-Wclang-format-violations ] public:

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

MyDeveloperDay added a comment. @HazardyKnusperkeks (its not without merits to add a "C" Language the more i think about it, but I'm not sure quite in the form presented here) If we DID add a "C" language then we need to add something like you suggested isCpp() already means more than we thin

[PATCH] D117520: [clang-format] Fix SeparateDefinitionBlocks issues

MyDeveloperDay added a comment. ok, this is better but not quite right still.. I though I mentioned this before (please add as a unit test) HRESULT Foo::bar() {} HRESULT Foo::bar() {} formats as HRESULT Foo::bar() {} HRESULT Foo::bar() {} If you are windows develop

[PATCH] D117520: [clang-format] Fix SeparateDefinitionBlocks issues

MyDeveloperDay added a comment. Works nicely now... LGTM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D117520/new/ https://reviews.llvm.org/D117520 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailma

[PATCH] D117769: [clang-format] Refactor: add FormatToken::hasWhitespaceBefore(). NFC.

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

[PATCH] D117759: [clang-format][NFC] Clean up tryMergeLessLess()

MyDeveloperDay accepted this revision. MyDeveloperDay added a comment. LGTM, at first my head was confused with First[-1] but I think its ok. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D117759/new/ https://reviews.llvm.org/D117759 ___ cfe-

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

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

[PATCH] D117520: [clang-format] Fix SeparateDefinitionBlocks issues

MyDeveloperDay added a comment. I think its been fixed, in the last but one diff. Generally speaking we simply cannot have --output-replacements-xml , -n or --dry-run show replacements that amount to nothing because this is the mechanism by which 100s of repos fail commits thinking their is a c

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

MyDeveloperDay accepted this revision. MyDeveloperDay added a comment. new nits but basically I think this looks like it might handle this ok? LGTM We need to run this across a large code base to check Comment at: clang/lib/Format/UnwrappedLineFormatter.cpp:106 + bool ret

[PATCH] D117520: [clang-format] Fix SeparateDefinitionBlocks issues

MyDeveloperDay added a comment. maybe slightly related https://github.com/llvm/llvm-project/issues/53183 in that this is also impacted by the 5 character `TT_FunctionLikeOrFreestandingMacro` CHANGES SINCE LAST ACTION https://reviews.llvm.org/D117520/new/ https://reviews.llvm.org/D117520 __

[PATCH] D118220: [clang-format] Correctly format lambdas with variadic template parameters.

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

[PATCH] D118220: [clang-format] Correctly format lambdas with variadic template parameters.

MyDeveloperDay added a comment. In D118220#3273416 , @HazardyKnusperkeks wrote: > You guys have an enormous speed currently. I'm just trying to keep up with @curdeius, but my real work keeps getting in the way, but I'm loving the momentum and trying to

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

MyDeveloperDay added a comment. please mark your review comments as done when done so we know its a complete review CHANGES SINCE LAST ACTION https://reviews.llvm.org/D116638/new/ https://reviews.llvm.org/D116638 ___ cfe-commits mailing list cfe-c

[PATCH] D118337: [clang-format] Fix AllowShortFunctionsOnASingleLine: InlineOnly with wrapping after record.

MyDeveloperDay added inline comments. Comment at: clang/lib/Format/UnwrappedLineFormatter.cpp:280 +if (Style.isJavaScript() && (*I)->Last->is(TT_FunctionLBrace)) + return true; + we didn't need this anymore? Repository: rG LLVM Github

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

MyDeveloperDay added a comment. > This patch, for the moment, basically disables alignment in the lambdas which > is not what I want. I sort of feel that's ok for now. > This is still a WIP, but I'd like to have your opinion on how to tackle this > problem. What are we trying to achieve? what

[PATCH] D119682: [clang-format][docs] Fix incorrect 'clang-format 13' configuration options markers

MyDeveloperDay added inline comments. Comment at: clang/include/clang/Format/Format.h:2540-2541 ///} /// \endcode - /// \version 13 + /// \version 15 bool IndentRequiresClause; HazardyKnusperkeks wrote: > I like that suggestion.. Repository: r

[PATCH] D119682: [clang-format][docs] Fix incorrect 'clang-format 13' configuration options markers

MyDeveloperDay added a comment. I like `/// In clang-format 13 and 14 it was named ``IndentRequires``.` The code in theory should still support it so it should be documented. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D119682/new/ https://review

[PATCH] D120315: [clang-format] fix preprocessor nesting after https://github.com/llvm/llvm-project/commit/529aa4b011c4ae808d658022ef643c44dd9b2c9c

MyDeveloperDay added inline comments. Comment at: clang/unittests/Format/FormatTest.cpp:22727 guessLanguage("foo.h", "#define FOO(...) auto bar = [] __VA_ARGS__;")); + // Only only one of the two preprocessor regions has ObjC-like code. + EXPECT_EQ(FormatStyle::LK_ObjC,

[PATCH] D120315: [clang-format] fix preprocessor nesting after https://github.com/llvm/llvm-project/commit/529aa4b011c4ae808d658022ef643c44dd9b2c9c

MyDeveloperDay added inline comments. Comment at: clang/lib/Format/FormatToken.h:1077 + // For internal use by clang-format. + IdentifierInfo *kw_clang_format_internal_ident_after_define; + I'd be ok with just `kw_internal_ident_after_define` Repository: r

[PATCH] D120315: [clang-format] fix preprocessor nesting after https://github.com/llvm/llvm-project/commit/529aa4b011c4ae808d658022ef643c44dd9b2c9c

MyDeveloperDay accepted this revision. MyDeveloperDay added a comment. This revision is now accepted and ready to land. Just a few nits, but LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D120315/new/ https://reviews.llvm.org/D120315 __

[PATCH] D119599: [clang-format] Add option to align compound assignments like `+=`

MyDeveloperDay added a comment. Could you mark the comments as done (if they are done) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D119599/new/ https://reviews.llvm.org/D119599 ___ cfe-commits mailing l

[PATCH] D119599: [clang-format] Add option to align compound assignments like `+=`

MyDeveloperDay added subscribers: njames93, grimar. MyDeveloperDay added a comment. Does the YAML changes need unit tests in (llvm/unittests/Support) ? also we should pull in a reviewer from llvm/Support (maybe @njames93, @grimar ) who have made YAML specific changes in this header. Repositor

[PATCH] D119599: [clang-format] Add option to align compound assignments like `+=`

MyDeveloperDay added inline comments. Comment at: clang/docs/tools/dump_format_style.py:121 def __str__(self): -return '\n'.join(map(str, self.values)) +return self.comment + '\n' + '\n'.join(map(str, self.values)) Can this change be separate? why is

[PATCH] D120398: [format] follow up: Use unsigned char as the base of all enums in FormatStyle

MyDeveloperDay added a comment. Is there a clang-tidy check for this? I mean if we are doing this because it helps reduce the size of FormatStyle and its recommended practice for small enums, then we should have a clang-tidy check that catches us on this. (no?) Repository: rG LLVM Github Mon

[PATCH] D120217: [clang-format] Add an option to insert braces after control statements

MyDeveloperDay added a comment. @owenpan I didn't get to this one to review it, but it looks good to me and thank you for following through with this, I'll abandon the other implementation Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D120217/new/

[PATCH] D95168: [clang-format] Add Insert/Remove Braces option

MyDeveloperDay abandoned this revision. MyDeveloperDay added a comment. Abandoning as replaced with a better solution D120217: [clang-format] Add an option to insert braces after control statements CHANGES SINCE LAST ACTION https://reviews.llvm.org/D95168/n

[PATCH] D120631: [clang-format][docs] Fix incorrect 'clang-format 12' option markers

MyDeveloperDay added a comment. once again thank you! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D120631/new/ https://reviews.llvm.org/D120631 ___ cfe-commits mailing list cfe-commits@lists.llvm.org ht

[PATCH] D105408: [clang-format] Pass a TextDiagnosticPrinter when we can not create tempory file.

MyDeveloperDay added a comment. This is no longer needed, please abanson Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D105408/new/ https://reviews.llvm.org/D105408 ___ cfe-commits mailing list cfe-commit

[PATCH] D120710: [clang-format] QualifierOrder does not reorder template arguments

MyDeveloperDay created this revision. MyDeveloperDay added reviewers: HazardyKnusperkeks, curdeius. MyDeveloperDay added projects: clang, clang-format. MyDeveloperDay requested review of this revision. https://github.com/llvm/llvm-project/issues/53981 Reorder the qualifiers inside the template ar

[PATCH] D120712: [clang-format][docs] handle explicit enum values

MyDeveloperDay added a comment. I'm not quite sure how I feel about adding functionality that doesn't actually do anything. Comment at: clang/docs/tools/dump_format_style.py:174-175 return '* ``%s`` (in configuration: ``%s``)\n%s' % ( -self.name, -re.sub('

[PATCH] D120712: [clang-format][docs] handle explicit enum values

MyDeveloperDay added a comment. > My reasoning was that I'm going to introduce an explicit enum value Any reason why? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D120712/new/ https://reviews.llvm.org/D120712 _

[PATCH] D120712: [clang-format][docs] handle explicit enum values

MyDeveloperDay added inline comments. Comment at: clang/docs/tools/dump_format_style.py:174-175 return '* ``%s`` (in configuration: ``%s``)\n%s' % ( -self.name, -re.sub('.*_', '', self.config), +self.clean_name, +self.clean_config, do

[PATCH] D120398: [format] follow up: Use unsigned char as the base of all enums in FormatStyle

MyDeveloperDay added a comment. Before this lands can we have a discussion about what clarity this gives us?, because I think it makes the code more unreadable, but surely we are saving just 7x(3 bytes) (the difference between and int and a unsigned char for 7 enums) Is saving 21 bytes valuabl

[PATCH] D120712: [clang-format][docs] handle explicit enum values

MyDeveloperDay added subscribers: HazardyKnusperkeks, owenpan, curdeius. MyDeveloperDay added a comment. Does it need to be so "cloak and dagger"? ;-) We always welcome patches, but please think about logging the idea in github issues (and assigning it to yourself), and use the good will of the

[PATCH] D120398: [format] follow up: Use unsigned char as the base of all enums in FormatStyle

MyDeveloperDay added a comment. In my own area I gave FormatStyle a copy constructor and made it private, it hardly caused any issues, this isn't something that we tend copy about very much, we tend copy construct it only when we do getLLVMStyle() to make another style say like google style sur

[PATCH] D120712: [clang-format][docs] handle explicit enum values

MyDeveloperDay added a comment. I have left, (some quite harsh opinions on this idea here, I'm sorry): https://github.com/llvm/llvm-project/issues/54137 If you do want to contribute can I suggest you start with an item off this list, I think it will give you good insight into what is involved wi

[PATCH] D120806: [clang-format] Recognize "if consteval".

MyDeveloperDay accepted this revision. MyDeveloperDay added a comment. This revision is now accepted and ready to land. LFTM Do we also need to consider this in the context of "AllowShortIfStatments"? (that would be in a different review in my view) Repository: rG LLVM Github Monorepo CHAN

[PATCH] D120931: [clang-format] fix namepsace format when the name is macro expansion

MyDeveloperDay added inline comments. Comment at: clang/unittests/Format/NamespaceEndCommentsFixerTest.cpp:76 +"int j;\n" +"}// namespace", +fixNamespaceEndComments("#define M(x) x##x\n" but its not an anonymous namespace? wou

[PATCH] D112181: [docs] Remove hard-coded version numbers from sphinx configs

MyDeveloperDay added a comment. Herald added a project: All. Can we drop the git from everywhere? F22325692: image.png Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112181/new/ https://reviews.llvm.org/D112181

[PATCH] D114231: [clang][docs][dataflow] Added an introduction to dataflow analysis

MyDeveloperDay added a comment. Herald added a project: All. Did you add somewhere what dependency we need in order to build the documentation now as mine is having problems with "recommonmark" /usr/bin/sphinx-build -n ./docs ./html Running Sphinx v3.4.3 Configuration error: There is

[PATCH] D120931: [clang-format] Fix namespace format when the name is a macro expansion

MyDeveloperDay added inline comments. Comment at: clang/unittests/Format/NamespaceEndCommentsFixerTest.cpp:112 + EXPECT_EQ("#define M(x) x##x\n" +"namespace [[deprecated(\"foo\")]] A::inline M(x)::A {\n" +"int i;\n" Is this 2 bugs in one?

[PATCH] D120931: [clang-format] Fix namespace format when the name is a macro expansion

MyDeveloperDay added a comment. > Its assignment only happens in MacroExpander which is never used This is something that @klimek was working on. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D120931/new/ https://reviews.llvm.org/D120931 _

[PATCH] D120931: [clang-format] Fix namespace format when the name is a macro expansion

MyDeveloperDay added inline comments. Comment at: clang/unittests/Format/NamespaceEndCommentsFixerTest.cpp:112 + EXPECT_EQ("#define M(x) x##x\n" +"namespace [[deprecated(\"foo\")]] A::inline M(x)::A {\n" +"int i;\n" zequanwu wrote: > MyDe

[PATCH] D120710: [clang-format] QualifierOrder does not reorder template arguments

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG28bb040ded83: [clang-format] QualifierOrder does not reorder template arguments (authored by MyDeveloperDay). Changed prior to commit: https://rev

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

MyDeveloperDay created this revision. MyDeveloperDay added reviewers: curdeius, HazardyKnusperkeks, owenpan. MyDeveloperDay added projects: clang, clang-format. Herald added a project: All. MyDeveloperDay requested review of this revision. I have lost count of the number of times this has been rep

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

MyDeveloperDay updated this revision to Diff 413286. MyDeveloperDay added a comment. Update to always use the first row number of columns or unit tests crash CHANGES SINCE LAST ACTION https://reviews.llvm.org/D121069/new/ https://reviews.llvm.org/D121069 Files: clang/lib/Format/WhitespaceM

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

MyDeveloperDay updated this revision to Diff 413288. MyDeveloperDay added a comment. Add more producers CHANGES SINCE LAST ACTION https://reviews.llvm.org/D121069/new/ https://reviews.llvm.org/D121069 Files: clang/lib/Format/WhitespaceManager.cpp clang/lib/Format/WhitespaceManager.h cl

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

MyDeveloperDay updated this revision to Diff 413298. MyDeveloperDay added a comment. Fix review comments and clang-format CHANGES SINCE LAST ACTION https://reviews.llvm.org/D121069/new/ https://reviews.llvm.org/D121069 Files: clang/lib/Format/WhitespaceManager.cpp clang/lib/Format/Whites

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

MyDeveloperDay updated this revision to Diff 413299. MyDeveloperDay marked 4 inline comments as done. MyDeveloperDay added a comment. Lets get the comment aligned with the code CHANGES SINCE LAST ACTION https://reviews.llvm.org/D121069/new/ https://reviews.llvm.org/D121069 Files: clang/lib

<    16   17   18   19   20   21   22   23   24   25   >