[PATCH] D58404: [clang-format] Add basic support for formatting C# files

MyDeveloperDay created this revision. MyDeveloperDay added reviewers: djasper, klimek, krasimir, benhamilton, JonasToth. MyDeveloperDay added a project: clang. Herald added subscribers: jdoerfert, mgorny. This revision adds basic support for formatting C# files with clang-format, I know the barr

[PATCH] D57896: Variable names rule

MyDeveloperDay added a comment. In D57896#1402280 , @zturner wrote: > Since someone already accepted this, I suppose I should mark require changes > to formalize my dissent As it was Chris @lattner who accepted it, is your request for changes just base

[PATCH] D52150: [clang-format] BeforeHash added to IndentPPDirectives

MyDeveloperDay added a comment. Could you add some text into the docs/ReleaseNotes.rst to say you are adding a new option take a look at https://clang.llvm.org/extra/ReleaseNotes.html#improvements-to-clang-tidy for an example CHANGES SINCE LAST ACTION https://reviews.llvm.org/D52150/new/

[PATCH] D52150: [clang-format] BeforeHash added to IndentPPDirectives

MyDeveloperDay marked an inline comment as done. MyDeveloperDay added inline comments. Comment at: lib/Format/UnwrappedLineFormatter.cpp:15 #include #define DEBUG_TYPE "format-formatter" Nice! its the irony of clang-format code is that its not clang formatte

[PATCH] D58178: [clang-tidy] RawStringLiteralCheck: isRawStringLiteral doesn't check all erroneous cases

MyDeveloperDay added a comment. Could I ask if you don't want to pursue this review that you Abandon it. Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58178/new/ https://reviews.llvm.org/D58178 ___ cfe-

[PATCH] D57687: [clang-format] Add style option AllowShortLambdasOnASingleLine

MyDeveloperDay added a comment. Could you add a line in the docs/ReleaseNotes.rst to say what you are adding CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57687/new/ https://reviews.llvm.org/D57687 ___ cfe-commits mailing list cfe-commits@l

[PATCH] D57687: [clang-format] Add style option AllowShortLambdasOnASingleLine

MyDeveloperDay added inline comments. Comment at: clang/lib/Format/Format.cpp:649 LLVMStyle.AllowShortIfStatementsOnASingleLine = false; + LLVMStyle.AllowShortLambdasOnASingleLine = FormatStyle::SLS_All; LLVMStyle.AllowShortLoopsOnASingleLine = false; What

[PATCH] D58178: [clang-tidy] RawStringLiteralCheck: isRawStringLiteral doesn't check all erroneous cases

MyDeveloperDay added a comment. In D58178#1405430 , @gmit wrote: > In D58178#1405397 , @MyDeveloperDay > wrote: > > > Could I ask if you don't want to pursue this review that you Abandon it. > > > Sure, I already w

[PATCH] D58404: [clang-format] Add basic support for formatting C# files

MyDeveloperDay updated this revision to Diff 187748. MyDeveloperDay added a comment. Add to release notes CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58404/new/ https://reviews.llvm.org/D58404 Files: docs/ClangFormat.rst docs/ClangFormatStyleOptions.rst docs/ReleaseNotes.rst

[PATCH] D28462: clang-format: Add new style option AlignConsecutiveMacros

MyDeveloperDay added a comment. I'm not a code owner here but we seem to be lacking people who are either available, able, willing or allowed to approve code reviews for some of the code in the clang-tooling (not just clang format). The code owners do an extremely good job at doing a deep revi

[PATCH] D28462: clang-format: Add new style option AlignConsecutiveMacros

MyDeveloperDay added a comment. In D28462#1405711 , @klimek wrote: > In D28462#1405360 , @MyDeveloperDay > wrote: > > > I'm not a code owner here but we seem to be lacking people who are either > > available, able

[PATCH] D28462: clang-format: Add new style option AlignConsecutiveMacros

MyDeveloperDay added a comment. > Yes, we are, and as you said, it needs a set of patches to learn the code > base enough to contribute, to build up trust. This is easiest when the code > base is currently under active development, but many people consider > clang-format "good enough" minus bug

[PATCH] D58404: [clang-format] Add basic support for formatting C# files

MyDeveloperDay added a comment. Note to self...Underdevelopment: - adding support for => (JSBigArrow) for C# Lambas - adding support for $"liternal {string} with arguments" CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58404/new/ https://reviews.llvm.org/D58404 _

[PATCH] D57896: Variable names rule

MyDeveloperDay added a comment. In D57896#1406336 , @zturner wrote: > ... I can't argue with anything you say.. but I guess to reinforce your point introducing what is effectively a 3rd style would likely cause even more jarring... Repository: rG

[PATCH] D57896: Variable names rule

MyDeveloperDay added a comment. In D57896#1406407 , @zturner wrote: > If I read the post correctly, it was actually agreeing with me (because it > said "to reinforce your point...". Meaning that something such as > `lowerCaseCamel` would be the third st

[PATCH] D58404: [clang-format] Add basic support for formatting C# files

MyDeveloperDay updated this revision to Diff 187969. MyDeveloperDay added a comment. Increase C# formatting capabilities - don't split regions markers across lines - lexer support for verbatim string literals - support for interpolated string literals (C#6) - support for interpolated verbatim st

[PATCH] D63062: [clang-format] Added New Style Rule: BitFieldDeclsOnSeparateLines

MyDeveloperDay requested changes to this revision. MyDeveloperDay added a comment. This revision now requires changes to proceed. You need to add a unit test in clang/unittests Comment at: docs/ClangFormatStyleOptions.rst:195 +**BitFieldDeclsOnSeparateLines** (``bool``) + If `

[PATCH] D63062: [clang-format] Added New Style Rule: BitFieldDeclsOnSeparateLines

MyDeveloperDay requested changes to this revision. MyDeveloperDay added inline comments. This revision now requires changes to proceed. Comment at: docs/ClangFormatStyleOptions.rst:194 +**BitFieldDeclsOnSeparateLines** (``bool``) + If ``true``, Align Bitfield Declarations on s

[PATCH] D63062: [clang-format] Added New Style Rule: BitFieldDeclsOnSeparateLines

MyDeveloperDay added a comment. mark sure you mark off the comments as you consider them done. Comment at: lib/Format/FormatToken.h:519 +/// Returns whether the token is a Bit field, and checks whether +/// the Style is C / C++. this looks like it needs a cla

[PATCH] D63062: [clang-format] Added New Style Rule: OnePerLineBitFieldDecl

MyDeveloperDay added a comment. Nit: if you going for "OnePerLine" in the name rather than "Break" can you keep it consistent with `ConstructorInitializerAllOnOneLineOrOnePerLine` and put the type first and then the operation i.e. `BitFieldOnePerLine` (this will help keep all BitField options

[PATCH] D63062: [clang-format] Added New Style Rule: BitFieldDeclarationsOnePerLine

MyDeveloperDay added inline comments. Comment at: lib/Format/FormatToken.h:524 +T = T->getPreviousNonComment(); +return (T->Tok.is(tok::comma) && Tok.is(tok::identifier) && +T->Next->Tok.is(tok::colon)); do you think you might need (T && T->T

[PATCH] D64695: [clang-format] Added new style rule: SortNetBSDIncludes

MyDeveloperDay added a comment. Do you think there is anything about this algorithm that could be parameterized so that other projects could utilize it? I guess its not completely clear how this differs from just using the IncludeCategories? Repository: rC Clang CHANGES SINCE LAST ACTION

[PATCH] D64695: [clang-format] Added new style rule: SortNetBSDIncludes

MyDeveloperDay added a comment. There also seems like alot of duplication between the existing sortCppIncludes I think the only difference here is really just SmallVector Indices; SmallVector Includes_p; for (unsigned i = 0, e = Includes.size(); i != e; ++i) { unsigned pl = getNe

[PATCH] D64695: [clang-format] Added new style rule: SortNetBSDIncludes

MyDeveloperDay added a comment. In D64695#1584740 , @Manikishan wrote: > In D64695#1584706 , @MyDeveloperDay > wrote: > > > There also seems like alot of duplication between the existing > > sortCppIncludes > > >

[PATCH] D64695: [clang-format] Added new style rule: SortNetBSDIncludes

MyDeveloperDay added inline comments. Comment at: unittests/Format/SortIncludesTest.cpp:709 + "#include \"pathnames.h\"\n", + sort("#include \n" + "#include \n" should you add a test which has groups defined already, I'm un

[PATCH] D64695: [clang-format] Added new style rule: SortNetBSDIncludes

MyDeveloperDay added a comment. I appreciate what you've done to make this patch less duplicated code..but now you've almost got to the point where being able to control it all via configuration. I'm with @rdwampler I think you should be able to extend the include categories...and then this be

[PATCH] D63062: [clang-format] Added New Style Rule: BitFieldDeclarationsOnePerLine

MyDeveloperDay added inline comments. Comment at: unittests/Format/FormatTest.cpp:3671 + ); +} MyDeveloperDay wrote: > please add a test with comments (it will get logged) > > > ``` > unsigned int baz : 11, /*motor control flags*/ > add

[PATCH] D65149: [Format] Add test demonstrating PR42722

MyDeveloperDay added a comment. We don't normally commit a failing test alone, otherwise the build machines will be broken until it gets fixed.. just add this test with your fix. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65149/new/ https://rev

[PATCH] D65043: [Format] Add C++20 standard to style options

MyDeveloperDay added inline comments. Comment at: clang/lib/Format/Format.cpp:76 +IO.enumCase(Value, "C++20", FormatStyle::LS_Cpp20); IO.enumCase(Value, "Auto", FormatStyle::LS_Auto); } Prior to C++20 being confirmed and given the line 2373 being Lan

[PATCH] D64695: [clang-format] Added new style rule: SortNetBSDIncludes

MyDeveloperDay added a comment. Assuming this works and the other unit tests don't show issues then this LGTM. Please consider running this on your NetBSD code base before committing, if possible please also run on clang code based to ensure existing sorted headers aren't sorted unexpectedly.

[PATCH] D65648: [clang-format] Add support to SpacesBeforeTrailingComments to add spaces before Block comments.

MyDeveloperDay requested changes to this revision. MyDeveloperDay added inline comments. This revision now requires changes to proceed. Comment at: lib/Format/TokenAnnotator.cpp:2163 +if (Current->is(TT_BlockComment)){ + std::cout << "TYPE"isOneOf(TT_TemplateC

[PATCH] D65149: [Format] Add test demonstrating PR42722

MyDeveloperDay added inline comments. Comment at: clang/unittests/Format/FormatTest.cpp:5184 + "c++;\n" + "d++\n" + " });\n" modocache wrote: > This is a passing test that demonstrates that the indentation of `c+

[PATCH] D66059: [clang-format] Expand AllowShortBlocksOnASingleLine for WebKit

MyDeveloperDay added a comment. LGTM Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66059/new/ https://reviews.llvm.org/D66059 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mai

[PATCH] D66332: [clang-format] Fix the bug that joins template closer and > or >>

MyDeveloperDay accepted this revision. MyDeveloperDay added a comment. This revision is now accepted and ready to land. LGTM Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66332/new/ https://reviews.llvm.org/D66332 __

[PATCH] D66384: [clang-format] Fix a bug that joins template closer and =

MyDeveloperDay accepted this revision. MyDeveloperDay added a comment. This revision is now accepted and ready to land. LGTM Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66384/new/ https://reviews.llvm.org/D66384 __

[PATCH] D61256: [clang-format][docs] Fix the Google C++ and Chromium style guide URLs

MyDeveloperDay added a comment. This patch is needs rebasing Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61256/new/ https://reviews.llvm.org/D61256 ___ cfe-commits mailing list cfe-commits@lists.llvm

[PATCH] D60225: [clang-format] [PR19056] Add support for indenting class members and methods one level under the modifiers

MyDeveloperDay added a comment. It was more about not having time to pursue it at this time.. I didn't feel I should hog the PR if someone else wanted to take a look. Even though its "Abandoned" it can be brought back at anytime, I don't like to sit on reviews for too long as it doesn't show th

[PATCH] D28462: clang-format: Add new style option AlignConsecutiveMacros

MyDeveloperDay added a comment. When I spoke with one of the code owners, they seemed to have a problem accepting this review based on there not being a general description/understanding of how this algorithm works. Having said that, all of the "AlignToken" based functions (alignConsecutiveXXX

[PATCH] D28462: clang-format: Add new style option AlignConsecutiveMacros

MyDeveloperDay accepted this revision. MyDeveloperDay added a comment. This revision is now accepted and ready to land. @klimek responded via the mailing list http://lists.llvm.org/pipermail/cfe-commits/Week-of-Mon-20190506/270976.html CHANGES SINCE LAST ACTION https://reviews.llvm.org/D2846

[PATCH] D61281: [clang-format] Fixed self assignment

MyDeveloperDay requested changes to this revision. MyDeveloperDay added a comment. This revision now requires changes to proceed. please simply remove line 249 Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61281/new/ https://reviews.llvm.org/D61281 ___

[PATCH] D61281: [clang-format] Fixed self assignment

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

[PATCH] D61386: [clang-tidy] Add support writing a check as a Transformer rewrite rule.

MyDeveloperDay added a comment. Just a passing comment, Is this: a) a different type of check that would be used for a different type of check than previous clang-tidy checks b) or is this a replacement on the previous style? Is there any documentation about the benefits of this method vs the o

[PATCH] D55433: [clang-tidy] Adding a new modernize use nodiscard checker

MyDeveloperDay marked 6 inline comments as done. MyDeveloperDay added a comment. >> >> >> unsigned BlockOrCode = 0; >> llvm::ErrorOr Res = skipUntilRecordOrBlock(Stream, BlockOrCode); >> if (!Res) >> Res.getError(); >> > > AFAIK `llvm::Error` must be consumed because if it goes

[PATCH] D55433: [clang-tidy] Adding a new modernize use nodiscard checker

MyDeveloperDay updated this revision to Diff 178108. MyDeveloperDay marked an inline comment as done. MyDeveloperDay added a comment. Addressing review comments - don't add [[nodiscard]] onto function taking template arguments - add more unit tests to cover other discard possibilities [[clang::w

[PATCH] D55433: [clang-tidy] Adding a new modernize use nodiscard checker

MyDeveloperDay marked 3 inline comments as done. MyDeveloperDay added inline comments. Comment at: test/clang-tidy/modernize-use-nodiscard.cpp:126 +template +class Bar +{ MyDeveloperDay wrote: > JonasToth wrote: > > MyDeveloperDay wrote: > > > curdeius wrote: > >

[PATCH] D55433: [clang-tidy] Adding a new modernize use nodiscard checker

MyDeveloperDay updated this revision to Diff 178123. MyDeveloperDay added a comment. Add additional constraints on member functions - do not add [[nodiscard]] to variadic function - do not add [[nodiscard]] to std::function templates argument functions - do not add [[nodiscard]] to functions retu

[PATCH] D55433: [clang-tidy] Adding a new modernize use nodiscard checker

MyDeveloperDay marked 3 inline comments as done. MyDeveloperDay added inline comments. Comment at: test/clang-tidy/modernize-use-nodiscard.cpp:126 +template +class Bar +{ MyDeveloperDay wrote: > MyDeveloperDay wrote: > > JonasToth wrote: > > > MyDeveloperDay wrot

[PATCH] D55433: [clang-tidy] Adding a new modernize use nodiscard checker

MyDeveloperDay updated this revision to Diff 178217. MyDeveloperDay marked 2 inline comments as done. MyDeveloperDay added a comment. Fix review comments - minor changes to code comments - add more unit tests for typedef'd template types CHANGES SINCE LAST ACTION https://reviews.llvm.org/D554

[PATCH] D55433: [clang-tidy] Adding a new modernize use nodiscard checker

MyDeveloperDay marked 7 inline comments as done. MyDeveloperDay added inline comments. Comment at: clang-tidy/modernize/UseNodiscardCheck.cpp:25 +static bool isAliasedTemplateParamType(const QualType &ParamType) { + return (ParamType.getCanonicalType().getAsString().find("type-p

[PATCH] D55433: [clang-tidy] Adding a new modernize use nodiscard checker

MyDeveloperDay updated this revision to Diff 178355. MyDeveloperDay marked 2 inline comments as done. MyDeveloperDay added a comment. - Rebase - Reduce false-positives by not adding [[nodiscard]] to functions if the class has mutable fields (removed as known issues) and add tests CHANGES SINCE

[PATCH] D55433: [clang-tidy] Adding a new modernize use nodiscard checker

MyDeveloperDay marked an inline comment as done. MyDeveloperDay added inline comments. Comment at: docs/clang-tidy/checks/modernize-use-nodiscard.rst:23 +Such functions have no means of altering any state or passing values other than +via the return type. Unless the member functi

[PATCH] D55433: [clang-tidy] Adding a new modernize use nodiscard checker

MyDeveloperDay updated this revision to Diff 178367. MyDeveloperDay added a comment. -Refactor the checker to remove the llvm::any_of by using a QualType ast-matcher with hasAnyParameter(hasType(x)) - i.e. ...unless(hasAnyParameter(hasType(isNonConstReferenceOrPointer(... -Reduce the SNR ra

[PATCH] D55433: [clang-tidy] Adding a new modernize use nodiscard checker

MyDeveloperDay updated this revision to Diff 178654. MyDeveloperDay marked 3 inline comments as done. MyDeveloperDay added a comment. Fix review comments - Fix minor spelling mistakes - add additional boost::function unit test CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55433/new/ ht

[PATCH] D55433: [clang-tidy] Adding a new modernize use nodiscard checker

MyDeveloperDay added a comment. In D55433#1334150 , @curdeius wrote: > LGTM. > Any ideas for improvement, @JonasToth? > I'd rather have it merged and improve on it later if there are ideas on how > to do it better. Thanks for the LGTM, as you know I d

[PATCH] D55433: [clang-tidy] Adding a new modernize use nodiscard checker

MyDeveloperDay updated this revision to Diff 178656. MyDeveloperDay added a comment. Remove unused matcher variable CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55433/new/ https://reviews.llvm.org/D55433 Files: clang-tidy/modernize/CMakeLists.txt clang-tidy/modernize/ModernizeTidy

[PATCH] D55433: [clang-tidy] Adding a new modernize use nodiscard checker

MyDeveloperDay updated this revision to Diff 179445. MyDeveloperDay added a comment. - Rebase - Add a couple of extra test cases to ensure we don't add the NO_DISCARD macro when a clang or gcc attribute is present - Ping to ask code owners to review and possibly commit if happy. CHANGES SINCE L

[PATCH] D55433: [clang-tidy] Adding a new modernize use nodiscard checker

MyDeveloperDay updated this revision to Diff 180073. MyDeveloperDay marked 14 inline comments as done. MyDeveloperDay added a comment. Rebase - update to address most review comments from @JonasToth - simplify matcher (with less unless) - remove the need to look for "type-parameter-" string CHA

[PATCH] D55433: [clang-tidy] Adding a new modernize use nodiscard checker

MyDeveloperDay added inline comments. Comment at: clang-tidy/modernize/UseNodiscardCheck.cpp:30 +// TODO: Find a better way of detecting a function template. +static bool isFunctionTemplate(const QualType &ParamType) { + // Try to catch both std::function and boost::function ---

[PATCH] D55433: [clang-tidy] Adding a new modernize use nodiscard checker

MyDeveloperDay added a comment. let me look further into that, it works for std::function but not for const std::function or std::function &, const std::function & CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55433/new/ https://reviews.llvm.org/D55433 _

[PATCH] D55433: [clang-tidy] Adding a new modernize use nodiscard checker

MyDeveloperDay updated this revision to Diff 180212. MyDeveloperDay marked 3 inline comments as done. MyDeveloperDay added a comment. Address review comments - remove using strings to locate std::function and boost::function arguments using matcher with ``hasAnyName()`` instead - add tests to ch

[PATCH] D55433: [clang-tidy] Adding a new modernize use nodiscard checker

MyDeveloperDay added inline comments. Comment at: clang-tidy/modernize/UseNodiscardCheck.cpp:30 +// TODO: Find a better way of detecting a function template. +static bool isFunctionTemplate(const QualType &ParamType) { + // Try to catch both std::function and boost::function ---

[PATCH] D55433: [clang-tidy] Adding a new modernize use nodiscard checker

MyDeveloperDay updated this revision to Diff 180266. MyDeveloperDay marked 4 inline comments as done. MyDeveloperDay added a comment. Update with view comments from @JonasToth - remove unnecessary static functions - move checks into matchers - remove duplicated diag call CHANGES SINCE LAST ACTI

[PATCH] D55433: [clang-tidy] Adding a new modernize use nodiscard checker

MyDeveloperDay marked 3 inline comments as done. MyDeveloperDay added inline comments. Comment at: clang-tidy/modernize/UseNodiscardCheck.cpp:23 +static bool isAliasedTemplateParamType(QualType ParamType) { + return (ParamType.getCanonicalType()->isTemplateTypeParmType() || +

[PATCH] D55433: [clang-tidy] Adding a new modernize use nodiscard checker

MyDeveloperDay added a comment. libprotobuf still builds cleanly with just the one expected warning..despite adding over 500 [[nodiscards]] F7800873: image.png I'll continue to look at other projects, but I'm happy at present that this gives us a good starti

[PATCH] D55433: [clang-tidy] Adding a new modernize use nodiscard checker

MyDeveloperDay updated this revision to Diff 180625. MyDeveloperDay added a comment. Sorry to add another revision but, rerunning no-discard checker on LLVM code base threw up other [[nodiscard]] placements which whilst they MAY be correct, I don't think we should handle in this first pass. - c

[PATCH] D56424: [clang-tidy] Add check for underscores in googletest names.

MyDeveloperDay added inline comments. Comment at: clang-tidy/google/AvoidUnderscoreInGoogletestNameCheck.cpp:1 +//===--- AvoidUnderscoreInGoogletestNameCheck.cpp - clang-tidy-===// +// nit: space after tidy and before --- Comment a

[PATCH] D56160: [clang-tidy] modernize-use-trailing-return check

MyDeveloperDay added inline comments. Comment at: test/clang-tidy/modernize-use-trailing-return.cpp:1 +// RUN: %check_clang_tidy %s modernize-use-trailing-return %t -- -- --std=c++14 + nit: is there a reason here why you say C++14 when the code checks for C++11?

[PATCH] D56160: [clang-tidy] modernize-use-trailing-return check

MyDeveloperDay added inline comments. Comment at: test/clang-tidy/modernize-use-trailing-return.cpp:1 +// RUN: %check_clang_tidy %s modernize-use-trailing-return %t -- -- --std=c++14 + bernhardmgruber wrote: > MyDeveloperDay wrote: > > nit: is there a reason here

[PATCH] D56160: [clang-tidy] modernize-use-trailing-return check

MyDeveloperDay added inline comments. Comment at: clang-tidy/modernize/UseTrailingReturnCheck.cpp:45 +std::string(Message) + +" (no FixIt provided, function argument list end is inside macro)"); +return {}; bernhardmgruber wrote: > JonasT

[PATCH] D55433: [clang-tidy] Adding a new modernize use nodiscard checker

MyDeveloperDay added a comment. In D55433#1349709 , @JonasToth wrote: > No problem, thats why we test on real projects first, because there is always > something hidden in C++ :) > > Is there a test for the lambdas? test/clang-tidy/modernize-use-nodis

[PATCH] D55433: [clang-tidy] Adding a new modernize use nodiscard checker

MyDeveloperDay marked an inline comment as done. MyDeveloperDay added a comment. In D55433#1349709 , @JonasToth wrote: > No problem, thats why we test on real projects first, because there is always > something hidden in C++ :) > > Is there a test for t

[PATCH] D55433: [clang-tidy] Adding a new modernize use nodiscard checker

MyDeveloperDay updated this revision to Diff 180687. MyDeveloperDay marked 3 inline comments as done. MyDeveloperDay added a comment. Address review comments - add more lambda tests - add nested lambda test - remove hasParent() call, (seems isConversionOperator()) seems to detect the CXXMethods

[PATCH] D55433: [clang-tidy] Adding a new modernize use nodiscard checker

MyDeveloperDay added a comment. In D55433#1349709 , @JonasToth wrote: > No problem, thats why we test on real projects first, because there is always > something hidden in C++ :) > > Is there a test for the lambdas? test/clang-tidy/modernize-use-nodis

[PATCH] D56424: [clang-tidy] Add check for underscores in googletest names.

MyDeveloperDay added inline comments. Comment at: clang-tidy/google/AvoidUnderscoreInGoogletestNameCheck.cpp:27 +static bool isGoogletestTestMacro(StringRef MacroName) { + static const llvm::StringSet<> MacroNames = {"TEST", "TEST_F", "TEST_P", +

[PATCH] D56424: [clang-tidy] Add check for underscores in googletest names.

MyDeveloperDay added inline comments. Comment at: clang-tidy/google/AvoidUnderscoreInGoogletestNameCheck.cpp:27 +static bool isGoogletestTestMacro(StringRef MacroName) { + static const llvm::StringSet<> MacroNames = {"TEST", "TEST_F", "TEST_P", +

[PATCH] D56424: [clang-tidy] Add check for underscores in googletest names.

MyDeveloperDay added inline comments. Comment at: clang-tidy/google/AvoidUnderscoreInGoogletestNameCheck.cpp:64 + Check->diag(TestCaseNameToken->getLocation(), + "avoid using \"_\" in test case name \"%0\" according to " + "Googletest FAQ")

[PATCH] D55433: [clang-tidy] Adding a new modernize use nodiscard checker

MyDeveloperDay added a comment. In D55433#1350999 , @JonasToth wrote: > LGTM! > You verified that your fixes, fix the issues in LLVM? But it looks good to > go. They look good, you asked before... > P.S. did you request commit rights already? I do no

[PATCH] D56532: [clang-tidy] Add the abseil-duration-conversion-cast check

MyDeveloperDay added a comment. nit's come from running running validate_check.py on the rst file from D55523: [clang-tidy] Linting .rst documentation $ ../clang-tidy/validate_check.py --rst abseil-duration-conversion-cast.rst Checking abseil-duration-conversion

[PATCH] D67395: [clang-format] Apply BAS_AlwaysBreak to C++11 braced lists

MyDeveloperDay accepted this revision. MyDeveloperDay added a comment. This revision is now accepted and ready to land. Thank you for this patch, i've seen a number of PRs raise to this effect.. this looks to be a great start in this area LGTM Repository: rC Clang CHANGES SINCE LAST ACTION

[PATCH] D64695: [clang-format] Modified SortIncludes and IncludeCategories to priority for sorting #includes within the Group Category.

MyDeveloperDay added a comment. Does this need landing? given that you have a number of patches in flight perhaps it would be good to request commit access Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64695/new/ https://reviews.llvm.org/D64695 __

[PATCH] D64695: [clang-format] Modified SortIncludes and IncludeCategories to priority for sorting #includes within the Group Category.

MyDeveloperDay added a comment. > Yes, it would be good if it is landed. And can I know the procedure for > getting commit access https://llvm.org/docs/DeveloperPolicy.html#obtaining-commit-access Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64695/new/ https

[PATCH] D67629: [clang-format] Fix C# breaking before function name when using Attributes

MyDeveloperDay created this revision. MyDeveloperDay added reviewers: owenpan, klimek. MyDeveloperDay added a project: clang-tools-extra. Herald added a project: clang. This is a fix for https://bugs.llvm.org/show_bug.cgi?id=4 This comes with 3 main parts - C# attributes cause function name

[PATCH] D67627: Clang-format: Add Whitesmiths indentation style

MyDeveloperDay added a comment. Thank you for this patch This LGTM, ensure you comment out the failing unit tests and add a FIXME to the test as to why they don't currently work (or fix them if possible), the tests need to keep passing or others will not know they haven't broken things. Normal

[PATCH] D67627: Clang-format: Add Whitesmiths indentation style

MyDeveloperDay added a comment. when/if you commit can you abandon D6833: Clang-format: Braces Indent Style Whitesmith Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67627/new/ https://reviews.llvm.org/D67627 ___

[PATCH] D67660: [clang-format]

MyDeveloperDay created this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Repository: rC Clang https://reviews.llvm.org/D67660 Files: clang/lib/Format/FormatTokenLexer.cpp clang/lib/Format/FormatTokenLexer.h clang/lib/Format/TokenAnnotator.cpp clang/u

[PATCH] D67670: [clang-format][PR41964] Fix crash with SIGFPE when TabWidth is set to 0 and line starts with tab

MyDeveloperDay created this revision. MyDeveloperDay added reviewers: owenpan, klimek, russellmcc, timwoj. MyDeveloperDay added a project: clang-tools-extra. Herald added a project: clang. clang-format 8.0 crashes with SIGFPE (floating point exception) when formatting following file: app.cpp: voi

[PATCH] D67670: [clang-format][PR41964] Fix crash with SIGFPE when TabWidth is set to 0 and line starts with tab

MyDeveloperDay marked an inline comment as done. MyDeveloperDay added inline comments. Comment at: clang/lib/Format/FormatTokenLexer.cpp:660 case '\t': -Column += Style.TabWidth - Column % Style.TabWidth; +Column += Style.TabWidth - (Column ? Column % Style

[PATCH] D67670: [clang-format][PR41964] Fix crash with SIGFPE when TabWidth is set to 0 and line starts with tab

MyDeveloperDay updated this revision to Diff 220571. MyDeveloperDay added a comment. I should have known it was more involved. v % 0 and v / 0 are both undefined behavior CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67670/new/ https://reviews.llvm.org/D67670 Files: clang/lib/Format

[PATCH] D67627: Clang-format: Add Whitesmiths indentation style

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

[PATCH] D67718: [clang-format][PR41899] PointerAlignment: Left leads to useless space in lambda intializer expression

MyDeveloperDay created this revision. MyDeveloperDay added reviewers: klimek, owenpan, krasimir, timwoj. MyDeveloperDay added a project: clang-tools-extra. Herald added a project: clang. https://bugs.llvm.org/show_bug.cgi?id=41899 auto lambda = [&a = a]() { a = 2; }; is formatted as auto la

[PATCH] D67773: [clang-format[PR43144] Add support for SpaceAroundBraces style

MyDeveloperDay created this revision. MyDeveloperDay added reviewers: klimek, owenpan. MyDeveloperDay added a project: clang-tools-extra. Herald added a project: clang. This is to address a request made in https://bugs.llvm.org/show_bug.cgi?id=43144 Add the ability to not have a space before and

[PATCH] D67501: [clang-tidy] Fix relative path in header-filter.

MyDeveloperDay added inline comments. Comment at: clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp:555 + StringRef FileName = File->tryGetRealPathName(); + if (FileName.empty()) { +FileName = File->getName(); elide the {} Repository: rCTE Cl

[PATCH] D67843: DisableFormat also now disables SortIncludes

MyDeveloperDay added a comment. I assume the intention was that users could have DisableFormat=true and SortIncludes=true when they want to sort the includes but not perform any additional formatting in the code. I think by making this change you make it impossible to run clang-format through

[PATCH] D67888: [clang-format] NFC clang-format the clang-format unit tests

MyDeveloperDay created this revision. MyDeveloperDay added reviewers: klimek, owenpan, timwoj. MyDeveloperDay added a project: clang-tools-extra. Herald added a project: clang. It is annoying that the clang-format tests aren't themselves clang-formatted, if you use a format on save option in VS o

[PATCH] D67888: [clang-format] NFC clang-format the clang-format unit tests

MyDeveloperDay updated this revision to Diff 221214. MyDeveloperDay added a comment. wrong patch file CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67888/new/ https://reviews.llvm.org/D67888 Files: clang/unittests/Format/FormatTest.cpp Index: clang/unittests/Format/FormatTest.cpp ==

[PATCH] D67888: [clang-format] NFC clang-format the clang-format unit tests

MyDeveloperDay added a comment. In D67888#1678465 , @owenpan wrote: > I suppose the .clang-format file you used only had `BasedOnStyle: LLVM` in > it. Did you run clang-format the second time to ensure that the formatted > file was stable? it used the

[PATCH] D67843: DisableFormat also now disables SortIncludes

MyDeveloperDay added a subscriber: klimek. MyDeveloperDay added a comment. I basically agree with all the comments, I agree with you that I doubt its ever used in SortIncludes:true and DisableFormat:true, I just saw this as a hole that probably based on Myrums Law (https://www.hyrumslaw.com/) m

[PATCH] D38446: update comments in clang-format.py for python3 compatibility

MyDeveloperDay accepted this revision. MyDeveloperDay added a comment. This revision is now accepted and ready to land. I'm not python expert but as this is a comment I don't see any issue. LGTM Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D38446/new/ https://rev

[PATCH] D67949: [clang-format] [PR36858] Add missing .hh and .cs extensions from python support utilities

MyDeveloperDay created this revision. MyDeveloperDay added reviewers: pseyfert, klimek, owenpan. MyDeveloperDay added a project: clang-tools-extra. Herald added a project: clang. https://bugs.llvm.org/show_bug.cgi?id=36858 identifies .hh as a missing C++ header extension file while making this ch

[PATCH] D44609: [Clang-Format] New option BeforeLambdaBody to manage lambda line break inside function parameter call (in Allman style)

MyDeveloperDay added a comment. Thanks for the patch, so sorry its taking a long time, its patches like this why I'm about to make a phabricator project for #clang-format alone so that we can ensure all clang-format patches can be see in isolation. I think we need to get this patches rebased to

<    1   2   3   4   5   6   7   8   9   10   >