[PATCH] D59309: [clang-format] BreakAfterReturnType ignored on functions with numeric template parameters

MyDeveloperDay added a comment. In D59309#1428799 , @klimek wrote: > Why did the numeric constant break this? Which path would the function run > through? as its looking though the parameter list is doesn't recognize a numeric constant as being a valid

[PATCH] D59309: [clang-format] BreakAfterReturnType ignored on functions with numeric template parameters

MyDeveloperDay added a comment. That works because the argument list is just tok::identifier and TT_StartOfName so if we see StartOfName we are going to return true identifier will simply be skipped round the loop `` int Test(A a) {} AnnotatedTokens(L=0): M=0 C=0 T=Unknown S=1 B=0 BK=0 P=

[PATCH] D59309: [clang-format] BreakAfterReturnType ignored on functions with numeric template parameters

MyDeveloperDay added a comment. of course, this isn't legal int TestFn(A=1) { return a; } and, would see the TT_StartOfName for (a) and would break int TestFn(A a = 1) { return a; } CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59309/new/ https://reviews.llvm.or

[PATCH] D59309: [clang-format] BreakAfterReturnType ignored on functions with numeric template parameters

MyDeveloperDay added a comment. In D59309#1428970 , @klimek wrote: > In D59309#1428969 , @MyDeveloperDay > wrote: > > > That works because the argument list is just tok::identifier and > > TT_StartOfName > > > Sho

[PATCH] D59309: [clang-format] BreakAfterReturnType ignored on functions with numeric template parameters

MyDeveloperDay added a comment. In D59309#1428970 , @klimek wrote: > In D59309#1428969 , @MyDeveloperDay > wrote: > > > That works because the argument list is just tok::identifier and > > TT_StartOfName > > > Sho

[PATCH] D55170: [clang-format]: Add NonEmptyParentheses spacing option

MyDeveloperDay requested changes to this revision. MyDeveloperDay added a comment. This revision now requires changes to proceed. Pulling this revision down to build it caused build issues with missing SBBLO_Always, could you investigate before committing? Comment at: lib/Form

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

MyDeveloperDay added a comment. @to-miz do you have permission to commit? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D52150/new/ https://reviews.llvm.org/D52150 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.or

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

MyDeveloperDay updated this revision to Diff 190737. MyDeveloperDay added a comment. Remove non related clang-format changes run git clang-format Add a Microsoft style (trying to match most closely the more traditional Microsoft style used for C#) This will allow, CSharp to be added to the confi

[PATCH] D54881: [clang-format] Prevent Clang-Format from editing leading whitespace on lines outside of the format range

MyDeveloperDay accepted this revision. MyDeveloperDay added a comment. This revision is now accepted and ready to land. From what I can tell this looks OK as you seem to be mainly only passing down the boolean, my guess is that while most people don't use the -lines directly that often but it pr

[PATCH] D40988: Clang-format: add finer-grained options for putting all arguments on one line

MyDeveloperDay accepted this revision. MyDeveloperDay added a comment. This revision is now accepted and ready to land. So @russellmcc you've been bumping along this road nicely for 6 months, doing what people say... pinging every week or so in order to get your code reviewed, and you are getti

[PATCH] D59408: [clang-format] [PR25010] Extend AllowShortIfStatementsOnASingleLine not working if an "else" statement is present

MyDeveloperDay created this revision. MyDeveloperDay added reviewers: djasper, klimek, reuk, JonasToth, alexfh, krasimir, russellmcc. MyDeveloperDay added a project: clang-tools-extra. Herald added a subscriber: jdoerfert. An addendum to D59087: [clang-format] [PR25010] AllowShortIfStatementsOn

[PATCH] D40988: Clang-format: add finer-grained options for putting all arguments on one line

MyDeveloperDay added a comment. > nobody being able to make changes Nobody IS able to make changes, but not because of complexity! We discourage away potential contributors/maintainers by leaving their reviews for weeks/months/years, not just not letting them in, not even discussing them.. M

[PATCH] D55170: [clang-format]: Add NonEmptyParentheses spacing option

MyDeveloperDay added a comment. I'm happy to be wrong, but In current master, SpaceBeforeCpp11BracedList is a boolean not an enum (but I think I've seen a review changing it somewhere which maybe isn't landed) https://reviews.llvm.org/source/llvm-github/browse/master/clang/include/clang/Format

[PATCH] D55170: [clang-format]: Add NonEmptyParentheses spacing option

MyDeveloperDay added a comment. > I'm sorry for not having a positive answer here, but I'm not in favor of this > change. The style rule looks like it introduces arbitrary complexity at a > point where I don't understand at all how it matters. We cannot possibly > support all style guides that

[PATCH] D55170: [clang-format]: Add NonEmptyParentheses spacing option

MyDeveloperDay added inline comments. Comment at: include/clang/Format/Format.h:1578 /// \endcode bool SpaceBeforeCpp11BracedList; boolean here not enum see comment below Comment at: lib/Format/TokenAnnotator.cpp:2608 +(Style.Sp

[PATCH] D55170: [clang-format]: Add NonEmptyParentheses spacing option

MyDeveloperDay added a comment. > I don't think risk is what matters - in the end it's about cost, and cost is > a very technical reason I'm really sorry I'm not trying to be difficult its just over the years I keep seeing this being said in reviews? but what is the cost? I assume you don't

[PATCH] D59540: [clang-tidy] [PR41119] readability-identifier-naming incorrectly fixes lambda capture

MyDeveloperDay created this revision. MyDeveloperDay added reviewers: JonasToth, aaron.ballman, alexfh, michaelplatings. MyDeveloperDay added a project: clang-tools-extra. Herald added subscribers: jdoerfert, xazax.hun. While testing clang-tidy for D59251: [Documentation] Proposal for plan to ch

[PATCH] D59292: [clang-format] messes up indentation when using JavaScript private fields and methods

MyDeveloperDay added subscribers: mitchellwills, mprobst. MyDeveloperDay added a comment. ping @mprobst,@mitchellwills noticed you just reviewed another clang-format-js commit, wondered if you were interested in reviewing this for me?, finding it hard to get any review traction. CHANGES SINCE

[PATCH] D55170: [clang-format]: Add NonEmptyParentheses spacing option

MyDeveloperDay accepted this revision. MyDeveloperDay added a comment. This revision is now accepted and ready to land. LGTM (landed here too https://github.com/mydeveloperday/clang-experimental/releases) CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55170/new/ https://reviews.llvm.org

[PATCH] D59546: [clang-format] structured binding in range for detected as Objective C

MyDeveloperDay created this revision. MyDeveloperDay added reviewers: djasper, klimek, JonasToth, reuk. MyDeveloperDay added a project: clang-tools-extra. Sometime after 6.0.0 and the current trunk 9.0.0 the following code would be considered as objective C and not C++ Reported by: https://twitt

[PATCH] D59540: [clang-tidy] [PR41119] readability-identifier-naming incorrectly fixes lambda capture

MyDeveloperDay planned changes to this revision. MyDeveloperDay added a comment. In D59540#1434837 , @JonasToth wrote: > What happens on `[=]() ...`, direct capture `[&Columns]()...` and > `[Columns]()...`? Thanks for the review... in answer to your que

[PATCH] D59540: [clang-tidy] [PR41119] readability-identifier-naming incorrectly fixes lambda capture

MyDeveloperDay updated this revision to Diff 191311. MyDeveloperDay added a comment. Minor modification to improve the `[=]` case `[&Columns]` and `[Columns]` are not yet fixed and will not be correctly renamed to `[&columns]` and `[columns]` But at least they are left unaltered CHANGES SINCE

[PATCH] D59540: [clang-tidy] [PR41119] readability-identifier-naming incorrectly fixes lambda capture

MyDeveloperDay updated this revision to Diff 191365. MyDeveloperDay added a comment. Address review comments This may not be a more satisfactory solution but it at least now covers the other cases raised by @JonasToth Add more tests around this area. CHANGES SINCE LAST ACTION https://review

[PATCH] D59540: [clang-tidy] [PR41119] readability-identifier-naming incorrectly fixes lambda capture

MyDeveloperDay marked 5 inline comments as done. MyDeveloperDay added inline comments. Comment at: test/clang-tidy/readability-identifier-naming.cpp:509 +// CHECK-FIXES: {{^}} bool columns=false; + auto ptr=[&]{return Columns;}(); +// CHECK-FIXES: {{^}} auto ptr=[&]{return col

[PATCH] D59408: [clang-format] [PR25010] Extend AllowShortIfStatementsOnASingleLine not working if an "else" statement is present

MyDeveloperDay marked 2 inline comments as done. MyDeveloperDay added inline comments. Comment at: clang/docs/ClangFormatStyleOptions.rst:401 + * ``SIS_AlwaysNoElse`` (in configuration: ``AlwaysNoElse``) +Allow short if/else if statements even if the else is a compound state

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

MyDeveloperDay marked an inline comment as done. MyDeveloperDay added inline comments. Comment at: include/clang/Basic/TokenKinds.h:80 K == tok::utf8_string_literal || K == tok::utf16_string_literal || - K == tok::utf32_string_literal; } klimek

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

MyDeveloperDay added a comment. In D52150#1436423 , @to-miz wrote: > I checked out the git repository and applied the patch to head and reran all > tests. > The diff produced by git is basically the same, should I upload the new diff? I think you might

[PATCH] D59309: [clang-format] BreakAfterReturnType ignored on functions with numeric template parameters

MyDeveloperDay updated this revision to Diff 191530. MyDeveloperDay added a comment. Addressing code review comment @klimek I think is what you meant, a much smaller and neater version of what I had before. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59309/new/ https://reviews.llv

[PATCH] D59309: [clang-format] BreakAfterReturnType ignored on functions with numeric template parameters

MyDeveloperDay updated this revision to Diff 191536. MyDeveloperDay added a comment. Sorry for the spam: - write this a little better CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59309/new/ https://reviews.llvm.org/D59309 Files: clang/lib/Format/TokenAnnotator.cpp clang/unittests

[PATCH] D55170: [clang-format]: Add NonEmptyParentheses spacing option

MyDeveloperDay added a comment. > The cost is financial, as it's developer time, which costs real money to > companies. In the end, to support this, people like myself who are doing this > as part of their job spend time that they'd otherwise spend to make other > things better that might be mo

[PATCH] D42034: [clang-format] In tests, expected code should be format-stable

MyDeveloperDay added inline comments. Herald added a project: LLVM. Comment at: cfe/trunk/unittests/Format/FormatTest.cpp:78 EXPECT_EQ(Expected.str(), format(Code, Style)); if (Style.Language == FormatStyle::LK_Cpp) { // Objective-C++ is a superset of C++, so eve

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

MyDeveloperDay updated this revision to Diff 191649. MyDeveloperDay added a comment. Addressing Review Comments - Remove all non Format only code (i.e. changes to clang/Basic and clang/Lexer to support verbatim interpolated strings C# 6) - For the most part the need to for them is removed by mer

[PATCH] D59629: [clang-format] correctly format protobuf fields named "enum".

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

[PATCH] D59627: [clang-format] Keep protobuf "package" statement on one line

MyDeveloperDay added inline comments. Comment at: lib/Format/TokenAnnotator.cpp:1079 if (Style.Language == FormatStyle::LK_Proto && Line.Level == 0 && -CurrentToken->is(Keywords.kw_option)) { +(CurrentToken->is(Keywords.kw_option) || + CurrentToken->i

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

MyDeveloperDay updated this revision to Diff 191654. MyDeveloperDay marked 3 inline comments as done. MyDeveloperDay added a comment. Address comment nits CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58404/new/ https://reviews.llvm.org/D58404 Files: clang/docs/ClangFormat.rst clan

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

MyDeveloperDay added inline comments. Comment at: clang/unittests/Format/FormatTestCSharp.cpp:19 + +class FormatTestCSharp : public ::testing::Test { +protected: klimek wrote: > If everything's static, you don't need a test fixture at all, you can just > make it

[PATCH] D59684: [clang-format] [PR41187] moves Java import statements to the wrong location if code contains statements that start with the word import

MyDeveloperDay created this revision. MyDeveloperDay added reviewers: djasper, klimek, mprobst, reuk, JonasToth. MyDeveloperDay added a project: clang-tools-extra. Import sorting of java file, incorrectly move import statement to after a function beginning with the word import. Make 1 character

[PATCH] D55170: [clang-format]: Add NonEmptyParentheses spacing option

MyDeveloperDay added a comment. @lebedev.ri Are we talking about a general ideology of the long term cost to allow any new things in? or to not allow things in this specific case? because in this specific case all the changes are based on what is really a single clause that was already there be

[PATCH] D59627: [clang-format] Keep protobuf "package" statement on one line

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

[PATCH] D59859: [clang-tidy] FIXIT for implicit bool conversion now obeys upper case suffixes if enforced.

MyDeveloperDay added a comment. We normally add something to the documentation about the checker and/or the release notes to say what had changed Comment at: clang-tools-extra/clang-tidy/readability/ImplicitBoolConversionCheck.cpp:48 case CK_IntegralToBoolean: -return

[PATCH] D59932: [clang-tidy] **Prototype**: Add fix description to clang-tidy checks.

MyDeveloperDay added a comment. In D59932#1446533 , @JonasToth wrote: > I think the idea is good and implementation, too. If we iterate all checks > anyway (probably?) we could think about adding a severity to the checks, too? In a similar vein, I often

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

MyDeveloperDay created this revision. MyDeveloperDay added reviewers: klimek, djasper, JonasToth, russellmcc, reuk. MyDeveloperDay added a project: clang-tools-extra. Code::Blocks and Qt Creator code uses a style guide which recommends indenting the next line after the access modifier e.g. cla

[PATCH] D59309: [clang-format] BreakAfterReturnType ignored on functions with numeric template parameters

MyDeveloperDay marked 2 inline comments as done. MyDeveloperDay added inline comments. Comment at: clang/lib/Format/TokenAnnotator.cpp:3177-3178 return false; // Don't split tagged template literal so there is a break between the tag // identifier and template st

[PATCH] D59309: [clang-format] BreakAfterReturnType ignored on functions with numeric template parameters

MyDeveloperDay marked an inline comment as done. MyDeveloperDay added a comment. friendly ping CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59309/new/ https://reviews.llvm.org/D59309 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D59540: [clang-tidy] [PR41119] readability-identifier-naming incorrectly fixes lambda capture

MyDeveloperDay marked an inline comment as done. MyDeveloperDay added a comment. friendly ping CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59540/new/ https://reviews.llvm.org/D59540 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D60320: [clang-format]: Add option to insert space after locical not operator

MyDeveloperDay added a comment. just a few nits, apart form that it LG Comment at: clang/include/clang/Format/Format.h:1721 + /// Defines whether a space should be placed after a logical `!` + bool SpaceAfterLogicalNot; nit: end the sentence with a "." ==

[PATCH] D60320: [clang-format]: Add option to insert space after locical not operator

MyDeveloperDay added a comment. I actually also tend to change doc/ClangFormatStyleOptions.rst too CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60320/new/ https://reviews.llvm.org/D60320 ___ cfe-commits mailing list cfe-commits@lists.llvm.

[PATCH] D60359: [clang-format] Fix incorrect indentation of C++ constructor initializer list

MyDeveloperDay accepted this revision. MyDeveloperDay added a comment. This revision is now accepted and ready to land. Thanks for doing this, it LGTM, I like your solution better ;-) I will abandon D60361: [clang-format] [PR41407] Constructor initializer list indented incorrectly

[PATCH] D52527: [clang-format] fix Bug 38686: add AfterCaseLabel to BraceWrapping

MyDeveloperDay added a comment. Herald added a project: clang. @owenpan can I help you bring this back up to date and review, I think I saw another issue in the bugzilla requesting this. I know the code owners like new option to have a style guide used by a large code base, but to me this seem

[PATCH] D60320: [clang-format]: Add option to insert space after locical not operator

MyDeveloperDay added a comment. In D60320#1457568 , @reuk wrote: > I updated `ClangFormatStyleOptions.rst` by hand, is there a way to do that > automatically instead (for future patches)? I do it by hand, I know there is a python script out there, go wi

[PATCH] D60320: [clang-format]: Add option to insert space after locical not operator

MyDeveloperDay accepted this revision. MyDeveloperDay added a comment. This revision is now accepted and ready to land. LGTM (minor nit) Comment at: clang/lib/Format/TokenAnnotator.cpp:2834 } - if (Left.is(TT_UnaryOperator)) -return Right.is(TT_BinaryOperator); + if (L

[PATCH] D60374: clang-format incorrectly indents wrapped closing parenthesis

MyDeveloperDay added inline comments. Comment at: clang/lib/Format/ContinuationIndenter.cpp:968 if (Current.is(tok::r_paren) && State.Stack.size() > 1 && - (!Current.Next || Current.Next->isOneOf(tok::semi, tok::l_brace))) + (!Current.Next || Current.Next->isOneOf(to

[PATCH] D60374: clang-format incorrectly indents wrapped closing parenthesis

MyDeveloperDay added a comment. maybe add the following as a test because I think it shows the inconsistency void Foo::bar( // some comment ) {} void Foo::bar( // some comment ) const {} Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60374/new/ https:/

[PATCH] D60374: clang-format incorrectly indents wrapped closing parenthesis

MyDeveloperDay accepted this revision. MyDeveloperDay added a comment. This revision is now accepted and ready to land. LGTM , if you also think the test will help show the use case then please add it, otherwise this revision notes might be information enough Repository: rC Clang CHANGES SIN

[PATCH] D60374: clang-format incorrectly indents wrapped closing parenthesis

MyDeveloperDay added a comment. In D60374#1457705 , @owenpan wrote: > In D60374#1457693 , @MyDeveloperDay > wrote: > > > LGTM , if you also think the test will help show the use case then please > > add it, otherw

[PATCH] D59087: [clang-format] [PR25010] AllowShortIfStatementsOnASingleLine not working if an "else" statement is present

MyDeveloperDay marked 2 inline comments as done. MyDeveloperDay added a comment. In D59087#1456129 , @krasimir wrote: > I believe there is no such thing as an "short else statement". The `else` is > part of the `if` statement and if it is present, I don't

[PATCH] D52527: [clang-format] fix Bug 38686: add AfterCaseLabel to BraceWrapping

MyDeveloperDay added a subscriber: reuk. MyDeveloperDay added a comment. @ownenpan might be worth checking with @reuk who has been adding some options for the JUCE style guide and looking at the JUCE code it seems this style might match their style. Comment at: include/clang/

[PATCH] D60363: [clang-format] [PR41170] Break after return type ignored with certain comments positions

MyDeveloperDay added a comment. In D60363#1459297 , @lebedev.ri wrote: > Please either subscribe the correct lists or set the correct repo in > differential params. Thanks for the correction, I was never really clear which one to use for the project as

[PATCH] D60151: [clang-tidy] Rename llvm checkers to llvm-project

MyDeveloperDay added a comment. are we supporting "-llvm-*" in existing .clang-tidy files? If people selectively turn checkers off, won't all of a sudden everyone start getting llvm-project checks and fixes turned back on? https://github.com/search?q=-llvm-%2A&type=Code maybe we need to add so

[PATCH] D54881: [clang-format] Prevent Clang-Format from editing leading whitespace on lines outside of the format range

MyDeveloperDay added a comment. > I agree and would be happy with the change if it would only change the > line-filtered workflow, but this afaict (unless I'm missing something :) will > also affect the workflow where the provided range is 0-length range, which > has an implicit "format stuff

[PATCH] D60151: [clang-tidy] Rename llvm checkers to llvm-project

MyDeveloperDay added a comment. >> I suppose we could keep the names and directory structure and just change >> the namespace. That would just be a special case in the scripts. Haven't >> looked into it yet, but will do so as soon as I can. > > Isn't that matching done on strings? I.e. is t

[PATCH] D60363: [clang-format] [PR41170] Break after return type ignored with certain comments positions

MyDeveloperDay updated this revision to Diff 195057. MyDeveloperDay added a comment. use endsWith() as it ignored comments CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60363/new/ https://reviews.llvm.org/D60363 Files: clang/lib/Format/TokenAnnotator.h clang/unittests/Format/Format

[PATCH] D60362: [clang-format] [PR39719] clang-format converting object-like macro to function-like macro

MyDeveloperDay added a comment. @klimek one possible solution to this might be to replace the "keyword" back to an identifier in a '#define ' scenario Maybe something like this? bool FormatTokenLexer::tryConvertKeyWordDefines() { // ensure #define keyword x = tok::hash,tok::identifier,to

[PATCH] D60853: clang-format converts a keyword macro definition to a macro function

MyDeveloperDay added a comment. LGTM Comment at: clang/lib/Format/UnwrappedLineParser.cpp:808 - if (FormatTok->Tok.getKind() != tok::identifier) { + if (!FormatTok->Tok.getIdentifierInfo()) { IncludeGuard = IG_Rejected; Is this equivalent to saying som

[PATCH] D60362: [clang-format] [PR39719] clang-format converting object-like macro to function-like macro

MyDeveloperDay abandoned this revision. MyDeveloperDay added a comment. Abandoning in favor of D60853: clang-format converts a keyword macro definition to a macro function Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60362/new/

[PATCH] D60996: [clang-format] Fix bug in reflow of block comments containing CR/LF

MyDeveloperDay accepted this revision. MyDeveloperDay added a comment. This revision is now accepted and ready to land. This looks logical to me, seems to fit with what ``WhitespaceManager::appendNewlineText`` is doing LGTM Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.ll

[PATCH] D61222: [clang-format] Fix bug in determineTokenType() for TT_StartOfName

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

[PATCH] D61297: [clang-format] Fix bug that misses some function-like macro usages

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

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

MyDeveloperDay added a comment. Did this case some issue? Does this fix something if so can we add a test, because maybe the line isn't needed Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61281/new/ https://reviews.llvm.org/D61281 ___

[PATCH] D61276: [clang-format] Fix bug in block comment reflow that joins * and /

MyDeveloperDay added a comment. Thanks for the patch, is this case only limited to * Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61276/new/ https://reviews.llvm.org/D61276 ___ cfe-commits mailing list cfe-commits@l

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

MyDeveloperDay accepted this revision. MyDeveloperDay added a comment. This revision is now accepted and ready to land. Thanks for the patch, thanks also for changing to https as I believe this was a change suggested in clang tidy docs too Repository: rG LLVM Github Monorepo CHANGES SINCE LA

[PATCH] D61222: [clang-format] Fix a bug in AlignConsecutiveDeclarations

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

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

MyDeveloperDay marked an inline comment as done. MyDeveloperDay added inline comments. Comment at: lib/Format/FormatTokenLexer.cpp:249 + Identifier->ColumnWidth += Question->ColumnWidth; + Identifier->Type = Identifier->Type; + Tokens.erase(Tokens.end() - 1);

[PATCH] D61276: [clang-format] Fix bug in block comment reflow that joins * 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/D61276/new/ https://reviews.llvm.org/D61276 __

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

MyDeveloperDay added a comment. In D61281#1488022 , @RKSimon wrote: > In D61281#1485833 , @MyDeveloperDay > wrote: > > > Did this cause some issue? Does this fix something if so can we add a test, > > because mayb

[PATCH] D57674: [clang-tidy] Add options to bugprone-argument-comment to add missing argument comments to literals

MyDeveloperDay created this revision. MyDeveloperDay added reviewers: JonasToth, alexfh, hokein, aaron.ballman, Eugene.Zelenko. MyDeveloperDay added a project: clang-tools-extra. Herald added a subscriber: xazax.hun. Herald added a project: clang. bugprone-argument-comment only supports identifyi

[PATCH] D57674: [clang-tidy] Add options to bugprone-argument-comment to add missing argument comments to literals

MyDeveloperDay marked an inline comment as done. MyDeveloperDay added inline comments. Comment at: docs/ReleaseNotes.rst:89 +- The :doc:`bugprone-argument-comment + ` now supports note to self, I will reorganize this alphabetically, on next revision Reposit

[PATCH] D57674: [clang-tidy] Add options to bugprone-argument-comment to add missing argument comments to literals

MyDeveloperDay added a comment. An example of this checker run over clang-tidy/modernize is given here D57675: [clang-tidy] DO NOT SUBMIT.. example diff of applying bugprone-argument-comments with AddCommentsToXXXLiterals options Repository: rCTE Clang Tool

[PATCH] D57655: clang-format with UseTab: Always sometimes doesn't insert the right amount of tabs.

MyDeveloperDay added a comment. There guys set a high barrier of entry, you need to at least start by adding some unit tests (or someone will break your code in the future), if you think it was a bug it might be worth logging that in bugzilla too! Repository: rC Clang CHANGES SINCE LAST ACT

[PATCH] D57674: [clang-tidy] Add options to bugprone-argument-comment to add missing argument comments to literals

MyDeveloperDay updated this revision to Diff 185014. MyDeveloperDay added a comment. Fix release note alphabetic order Add examples of Fixit into the documentation CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57674/new/ https://reviews.llvm.org/D57674 Files: clang-tidy/bugprone/Argu

[PATCH] D57674: [clang-tidy] Add options to bugprone-argument-comment to add missing argument comments to literals

MyDeveloperDay updated this revision to Diff 185148. MyDeveloperDay edited the summary of this revision. MyDeveloperDay added a comment. Address review comments Add support for additional StringLiterals,CharacterLiterals,UserDefinedLiterals and NullPtrs Simplify the Options names from "AddCommen

[PATCH] D57674: [clang-tidy] Add options to bugprone-argument-comment to add missing argument comments to literals

MyDeveloperDay added inline comments. Comment at: clang-tidy/bugprone/ArgumentCommentCheck.cpp:228-236 +static bool isStringLiteral(const Expr *Arg) { + const auto *Cast = dyn_cast(Arg); + return Cast ? isa(Cast->getSubExpr()) : false; +} + +static bool isNullPtrLiteral(const E

[PATCH] D57674: [clang-tidy] Add options to bugprone-argument-comment to add missing argument comments to literals

MyDeveloperDay updated this revision to Diff 185385. MyDeveloperDay marked 6 inline comments as done. MyDeveloperDay added a comment. Address review comments CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57674/new/ https://reviews.llvm.org/D57674 Files: clang-tidy/bugprone/ArgumentCo

[PATCH] D57674: [clang-tidy] Add options to bugprone-argument-comment to add missing argument comments to literals

MyDeveloperDay marked an inline comment as done. MyDeveloperDay added inline comments. Comment at: clang-tidy/bugprone/ArgumentCommentCheck.cpp:257 Ctx->getLangOpts()); }; @aaron.ballman, slight aside (and not in the code

[PATCH] D57674: [clang-tidy] Add options to bugprone-argument-comment to add missing argument comments to literals

MyDeveloperDay updated this revision to Diff 185632. MyDeveloperDay marked 5 inline comments as done. MyDeveloperDay added a comment. -Address review comments -Ignore argument comment addition for macro inputs -Add unit tests to cover the above -Fix clang-tidy suggestion (from readability-identifi

[PATCH] D57674: [clang-tidy] Add options to bugprone-argument-comment to add missing argument comments to literals

MyDeveloperDay marked 3 inline comments as done. MyDeveloperDay added inline comments. Comment at: clang-tidy/bugprone/ArgumentCommentCheck.cpp:1 -//===--- ArgumentCommentCheck.cpp - clang-tidy ===// // aaron.ballman wrote: > Why did

[PATCH] D57896: Variable names rule

MyDeveloperDay added a comment. does the readability-identifier-naming check need to be changed to support multiple allowed case types? - key: readability-identifier-naming.VariableCase value:camelBack,CamelBack Repository: rG LLVM Github Monorepo CHANGES SINCE L

[PATCH] D57674: [clang-tidy] Add options to bugprone-argument-comment to add missing argument comments to literals

MyDeveloperDay updated this revision to Diff 185947. MyDeveloperDay marked 2 inline comments as done. MyDeveloperDay added a comment. Pre-Commit Changes - add FIXME wording to test - move local header include after libraries - rebase CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57674/n

[PATCH] D57896: Variable names rule

MyDeveloperDay added a comment. > I personally would be happy to change the settings from `camelBack` to > `aNy_CasE`. Should we come up with a new style? UpperOrLowerCamelCase, I don't mind going and doing that in the readability-identifier-naming check, given that I just wrote up all the

[PATCH] D57966: [clang-tidy] add camelBackOrCase casing style to readability-identifier-naming to support change to variable naming policy (if adopted)

MyDeveloperDay created this revision. MyDeveloperDay added reviewers: michaelplatings, aaron.ballman, JonasToth, hokein. MyDeveloperDay added a project: clang-tools-extra. Herald added a subscriber: xazax.hun. Herald added a project: clang. Introduction of a new variable policy (as outlined in D5

[PATCH] D40854: [clang-tidy] WIP implement cppcoreguidelines check for mixed integer arithmetic

MyDeveloperDay added inline comments. Herald added a project: clang. Comment at: clang-tidy/cppcoreguidelines/MixedIntArithmeticCheck.h:1 +//===--- MixedIntArithmeticCheck.h - clang-tidy--*- C++ -*-===// +// I guess the license has to be updated

[PATCH] D33841: [clang-tidy] redundant keyword check

MyDeveloperDay added inline comments. Comment at: clang-tidy/readability/RedundantExternCheck.cpp:45 + + int offset = Text.find("extern"); + current convention would be that this should be Offset CHANGES SINCE LAST ACTION https://reviews.llvm.org/D33841/new/

[PATCH] D57662: [clang-tidy] Parallelise clang-tidy-diff.py

MyDeveloperDay added a comment. Just a question.. If clang tidy is running with -fix in parallel, what stops each clang-tidy invocation altering a common header at the same time? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57662/new/ https://reviews.llvm.org/D57662 ___

[PATCH] D57896: Variable names rule

MyDeveloperDay added a comment. > Sounds good to me. I see that you've made D57966 > a child of this issue, but we could swap > the dependency around so that once your patch is applied I can update this > patch to use `camelBackOrCase`. I'm OK if we want to do

[PATCH] D57662: [clang-tidy] Parallelise clang-tidy-diff.py

MyDeveloperDay added a comment. > You are right. May be it worth disabling `-fix` for `j != 1`. I only say this because I think I might have seen it happen when I was running `run-clang-tidy.py` over a large code base with a fairly aggressive check/fixit, but frankly I was too new to LLVM to kn

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

MyDeveloperDay added a comment. Rather than sending a review, probably the reproducible case as to why it crash is more important, it might be better to write up your bug at https://bugs.llvm.org/ I tried to repoduce this and I can't see how I could make a string literal without a double quote

[PATCH] D57087: [clang-tidy] add OverrideMacro to modernize-use-override check

MyDeveloperDay updated this revision to Diff 187155. MyDeveloperDay marked 10 inline comments as done. MyDeveloperDay added a comment. Address review comments - change OverrideMacro to OverrideSpelling - change FinalMacro to FinalSpelling - fix unit tests - show warning without fix-it if Macros a

[PATCH] D57087: [clang-tidy] add OverrideMacro to modernize-use-override check

MyDeveloperDay added inline comments. Comment at: clang-tidy/modernize/UseOverrideCheck.cpp:32 +: ClangTidyCheck(Name, Context), + OverrideMacro(Options.get("OverrideMacro", "override")), + FinalMacro(Options.get("FinalMacro", "final")) {} alexfh wr

[PATCH] D57087: [clang-tidy] add OverrideMacro to modernize-use-override check

MyDeveloperDay added a comment. In D57087#1400578 , @malcolm.parsons wrote: > Some Clang warnings use PP.getLastMacroWithSpelling() to determine the user's > macro automatically. This would be a neat trick, but mainly I think users would either be usin

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

MyDeveloperDay added a comment. @djasper and @klimek you both look relatively inactive in the last couple of weeks/month, is there anyone else who acts as a gate keeper for clang-format or can anyone add a LGTM? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D28462/new/ https://reviews.

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