[PATCH] D60199: [clang-format] Do not emit replacements while regrouping if Cpp includes are OK

MyDeveloperDay added a comment. Possible fix for PR43372 (https://bugs.llvm.org/show_bug.cgi?id=43372) Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60199/new/ https://reviews.llvm.org/D60199 ___ cfe-commits mailing

[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)); Manikishan wrote: > MyDeveloperDay wrot

[PATCH] D68072: Reference qualifiers in member templates causing extra indentation.

MyDeveloperDay added a comment. As the number of clauses is vast, I think sometimes its worth just putting a one-line comment in to explain what construct you are matching, just to help future maintainers Comment at: clang/lib/Format/TokenAnnotator.cpp:1375 +} + // L

[PATCH] D68072: [clang-format] Reference qualifiers in member templates causing extra indentation.

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

[PATCH] D68227: [clang-format] [PR43372] - clang-format shows replacements in DOS files when no replacement is needed

MyDeveloperDay created this revision. MyDeveloperDay added reviewers: krasimir, klimek, owenpan, ioeric. MyDeveloperDay added a project: clang-format. Herald added a project: clang. MyDeveloperDay edited the summary of this revision. This is a patch to fix PR43372 (https://bugs.llvm.org/show_bug.c

[PATCH] D68227: [clang-format] [PR43372] - clang-format shows replacements in DOS files when no replacement is needed

MyDeveloperDay added a comment. Through experiments, I notice that both result and the Code contains "\r\n", because although we construct the results with "\n" the actual text of the include has I believe trailing "\r" or "\r\n", I notice this when I thought one fix I tried was to trim the inc

[PATCH] D68242: [clang-format] [PR42417] clang-format inserts a space after '->' for operator->() overloading

MyDeveloperDay created this revision. MyDeveloperDay added reviewers: klimek, owenpan, byoungyoung. MyDeveloperDay added a project: clang-format. Herald added a project: clang. https://bugs.llvm.org/show_bug.cgi?id=42417 This revision removes the extra space between the opertor-> and the parens (

[PATCH] D68227: [clang-format] [PR43372] - clang-format shows replacements in DOS files when no replacement is needed

MyDeveloperDay updated this revision to Diff 222498. MyDeveloperDay added a comment. Being specific about replacing "\r\n"'s with "\n"'s rather than removing just \r's CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68227/new/ https://reviews.llvm.org/D68227 Files: clang/lib/Format/Fo

[PATCH] D14484: [clang-format] Formatting constructor initializer lists by putting them always on different lines

MyDeveloperDay requested changes to this revision. MyDeveloperDay added a comment. This revision now requires changes to proceed. Looking at this I'm wondering if this Isn't at least partially handled by the `BreakConstructorInitializersStyle` in combination with `ConstructorInitializerAllOnOne

[PATCH] D50078: clang-format: support aligned nested conditionals formatting

MyDeveloperDay accepted this revision. MyDeveloperDay added a comment. This revision is now accepted and ready to land. Thank you for this patch, (and by your patience ;-) ) This LGTM, Were there any objections from anyone else? otherwise I'd say this was ok. Repository: rC Clang CHANGES S

[PATCH] D68227: [clang-format] [PR43372] - clang-format shows replacements in DOS files when no replacement is needed

MyDeveloperDay updated this revision to Diff 222665. MyDeveloperDay set the repository for this revision to rC Clang. MyDeveloperDay added a project: clang-tools-extra. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68227/new/ https://reviews.llvm.org/D68227 Files:

[PATCH] D68296: clang-format: Add ability to wrap braces after multi-line control statements

MyDeveloperDay requested changes to this revision. MyDeveloperDay added a comment. This revision now requires changes to proceed. Firstly thank you for the patch.. Prior authors have pointed out that new styles need to be supported by a large public style guide as evidence as to why clang-format

[PATCH] D68332: [clang-format] [PR43531] clang-format damages "alternative representations" for operators

MyDeveloperDay created this revision. MyDeveloperDay added reviewers: klimek, reuk, owenpan, mitchell-stellar, STL_MSFT. MyDeveloperDay added projects: clang-format, clang-tools-extra. Herald added a project: clang. https://bugs.llvm.org/show_bug.cgi?id=43531 Fix for clang-format incorrectly han

[PATCH] D68296: clang-format: Add ability to wrap braces after multi-line control statements

MyDeveloperDay added a comment. As I mentioned, its worth getting evidence from a large public project... https://wiki.blender.org/index.php/Dev:Doc/Code_Style Blender public repo on github.com has 90 Forks, 299 * and 90,000 commits From their style guide Exceptions to Braces on Same Line

[PATCH] D68346: [clang-format] Add new option to add spaces around conditions

MyDeveloperDay requested changes to this revision. MyDeveloperDay added inline comments. This revision now requires changes to proceed. Comment at: clang/include/clang/Format/Format.h:1950 + /// similar) conditions. + bool SpacesAroundConditions; + its position

[PATCH] D68296: [clang-format] Add ability to wrap braces after multi-line control statements

MyDeveloperDay added a comment. @klimek thanks for offering for me to make the decision. I think given the fact that blender has it in their style guide and that they are clearly working around clang-formats inability to handle this case, I'm minded to LGTM this revision I hope you are ok with

[PATCH] D68296: [clang-format] Add ability to wrap braces after multi-line control statements

MyDeveloperDay requested changes to this revision. MyDeveloperDay added a comment. This revision now requires changes to proceed. Looks good but I think the option would be better as just `MultiLine`, remember to mark the comments "Done" when you are complete Comment at: clang

[PATCH] D68296: [clang-format] Add ability to wrap braces after multi-line control statements

MyDeveloperDay accepted this revision. MyDeveloperDay added a comment. This revision is now accepted and ready to land. LGTM and Thank you. Comment at: clang/lib/Format/Format.cpp:643 +true}; switch (Style.BreakBeforeBraces) { case FormatStyle:

[PATCH] D68415: [clang-format] C++11 braced lists should respect the SpacesInParentheses setting

MyDeveloperDay added a comment. Should we have `SpacesInBraces`? as a first class formatting option, given that we have? SpaceBetweenBraces.SpacesInAngles = true; SpaceBetweenBraces.SpacesInParentheses = true; SpaceBetweenBraces.SpacesInSquareBrackets = true; Repository: rC Clang CH

[PATCH] D68415: [clang-format] C++11 braced lists should respect the SpacesInParentheses setting

MyDeveloperDay added a comment. what if you have this vector{1, 2, 3}; new int[3]{1, 2, 3}; foo(int f); but want this: vector{ 1, 2, 3 }; new int[3]{ 1, 2, 3 }; foo(int f); wouldn't turning on `SpacesInParentheses: true` now mean you get vector{ 1, 2, 3 }; new int[3]{ 1, 2, 3

[PATCH] D68415: [clang-format] C++11 braced lists should respect the SpacesInParentheses setting

MyDeveloperDay added a comment. understood, ok LGTM Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68415/new/ https://reviews.llvm.org/D68415 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.

[PATCH] D68415: [clang-format] C++11 braced lists should respect the SpacesInParentheses setting

MyDeveloperDay added a comment. @mitchell-stellar I pulled the patch to commit it, but the unit tests fail, could you double-check them for me? [ OK ] FormatTest.LayoutBraceInitializersInReturnStatement (13 ms) [ RUN ] FormatTest.LayoutCxx11BraceInitializers C:/cygwin64/buildar

[PATCH] D68332: [clang-format] [PR43531] clang-format damages "alternative representations" for operators

MyDeveloperDay updated this revision to Diff 223215. MyDeveloperDay marked 3 inline comments as done. MyDeveloperDay added a comment. - add additional compl(5) and not(5) unit tests - improve the comments around the condition and explaining the tests - remove the use of .equals(...) CHANGES SINC

[PATCH] D68332: [clang-format] [PR43531] clang-format damages "alternative representations" for operators

This revision was automatically updated to reflect the committed changes. Closed by commit rGba12cec21f55: [clang-format] [PR43531] clang-format damages "alternative representations" for… (authored by MyDeveloperDay). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://revi

[PATCH] D68415: [clang-format] C++11 braced lists should respect the SpacesInParentheses setting

MyDeveloperDay added a comment. LGTM and passes [--] Global test environment tear-down [==] 700 tests from 21 test cases ran. (67039 ms total) [ PASSED ] 700 tests. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68415/new/ https://reviews.

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

MyDeveloperDay added a comment. In D67773#1694797 , @mitchell-stellar wrote: > This needs to be more thought out. At the very least it needs to cover more > keywords like `do`, `switch`, `try`, and `catch`. There may be more. > Consideration also needs

[PATCH] D68473: [clang-format] SpacesInSquareBrackets should affect lambdas with parameters too

MyDeveloperDay accepted this revision. MyDeveloperDay added a comment. This revision is now accepted and ready to land. Thanks for the patch. LGTM Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68473/new/ https://reviews.llvm.org/D68473 ___

[PATCH] D68481: [clang-format] [PR27004] omits leading space for noexcept when formatting operator delete()

MyDeveloperDay created this revision. MyDeveloperDay added reviewers: klimek, owenpan, mitchell-stellar. MyDeveloperDay added a project: clang-format. Herald added a project: clang. clang-format is incorrectly thinking the parameter parens are part of a cast operation, this is resulting in there

[PATCH] D68473: [clang-format] SpacesInSquareBrackets should affect lambdas with parameters too

MyDeveloperDay added inline comments. Comment at: clang/lib/Format/TokenAnnotator.cpp:2613 + TT_StructuredBindingLSquare, + TT_LambdaLSquare) && Style.SpacesInSquareBrackets && Right.isNot(tok::r_square)); -

[PATCH] D68481: [clang-format] [PR27004] omits leading space for noexcept when formatting operator delete()

MyDeveloperDay updated this revision to Diff 223369. MyDeveloperDay marked 2 inline comments as done. MyDeveloperDay added a comment. Add additional override and final keywords CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68481/new/ https://reviews.llvm.org/D68481 Files: clang/lib/F

[PATCH] D68481: [clang-format] [PR27004] omits leading space for noexcept when formatting operator delete()

MyDeveloperDay added inline comments. Comment at: clang/lib/Format/TokenAnnotator.cpp:1617 +if (Tok.Next->isOneOf(tok::kw_noexcept, tok::kw_volatile, tok::kw_const, + tok::kw_throw, tok::l_square, tok::arrow)) + return false; mit

[PATCH] D68551: [clang-format] [NFC] Ensure clang-format is itself clang-formatted.

MyDeveloperDay created this revision. MyDeveloperDay added a reviewer: mitchell-stellar. MyDeveloperDay added a project: clang-format. Herald added a project: clang. Before making a proposed change, ensure ClangFormat.cpp is fully clang-formatted, no functional change just clang-formatting using

[PATCH] D29039: Proposal for clang-format -r option

MyDeveloperDay added a comment. When I first saw this issue some time ago I tended to agree with the comment regarding the "its not clang-formats" job to do that... However, over the last couple of months I've seen more and more twitter and stack-overflow comments regarding "how do I check my w

[PATCH] D68539: [clang-tidy] fix for readability-identifier-naming incorrectly fixes variables which become keywords

MyDeveloperDay added a comment. This should really be a full context diff -U9 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68539/new/ https://reviews.llvm.org/D68539 ___ cfe-commits mailing li

[PATCH] D68554: [clang-format] Proposal for clang-format to give compiler style warnings

MyDeveloperDay created this revision. MyDeveloperDay added reviewers: mitchell-stellar, klimek, owenpan. MyDeveloperDay added projects: clang-format, clang-tools-extra. Herald added a subscriber: mgorny. Herald added a project: clang. Related somewhat to D29039: [clang-format] Proposal for clang-f

[PATCH] D68554: [clang-format] Proposal for clang-format to give compiler style warnings

MyDeveloperDay added a comment. In D68554#1696675 , @klimek wrote: > Why not build a tool that takes the diff output of clang-format and changes > it to what you want? Wouldn't that be a couple of lines of python? Thanks for taking the time to look at t

[PATCH] D33029: [clang-format] add option for dangling parenthesis

MyDeveloperDay added a comment. @stringham you seemingly have marked this revision so that no one else can edit it, so I cannot add it to the clang-format project or reassign reviewers, could you please change it to be public, it also means others cannot request changes or approve it (if they

[PATCH] D31574: [clang-format] update documentation

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

[PATCH] D68551: [clang-format] [NFC] Ensure clang-format is itself clang-formatted.

MyDeveloperDay added a comment. In D68551#1696598 , @Eugene.Zelenko wrote: > If clang-format if clang-formatted now completely, I think will be good idea > to add rule to check this during build, like Polly does. This is a great idea... Do you know if

[PATCH] D68539: [clang-tidy] fix for readability-identifier-naming incorrectly fixes variables which become keywords

MyDeveloperDay added a comment. I logged the original bug and I like it! I think the warning is better than mutating with a prefix, Thank you. I'll let the code owners approve it, but you have my vote! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/

[PATCH] D68551: [clang-format] [NFC] Ensure clang-format is itself clang-formatted.

MyDeveloperDay added a comment. I think when polly's script detects a change it's just showing the diff, this might be so much nicer if we could implement D68554: [clang-format] Proposal for clang-format to give compiler style warnings Repository: rC Clang

[PATCH] D68554: [clang-format] Proposal for clang-format to give compiler style warnings

MyDeveloperDay added a comment. Using a suggestion from D68551: [clang-format] [NFC] Ensure clang-format is itself clang-formatted. as an example of its usefulness, I modified my clang-format CMakeList.txt to do as Polly does and add clang-format check rules, c

[PATCH] D68569: [clang-format] Also look for .{ext}.clang-format file

MyDeveloperDay added a comment. Thanks for the patch, I think I understand what you are trying to do, but I have a few questions. Is the premise here that you need slightly different styles for .cpp than for .h? could you explain why? whilst I can see there is value in having such fine control

[PATCH] D68568: [clang-format] Make '.clang-format' variants finding a loop

MyDeveloperDay added a comment. Thank you for the patch, I think this looks cleaner and generally LGTM, I think its always good to put a little description of the change along with the NFC in the title to let people know that functionally there isn't any change without having to work though the

[PATCH] D68569: [clang-format] Also look for .{ext}.clang-format file

MyDeveloperDay added a comment. Comment at: clang/lib/Format/Format.cpp:2603 llvm::SmallVector FilesToLookFor; + Nit: almost every file has an extension should this be 3? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews

[PATCH] D68554: [clang-format] Proposal for clang-format to give compiler style warnings

MyDeveloperDay added a comment. In D68554#1697857 , @mitchell-stellar wrote: > I don't care for the command line switches that try to emulate compiler > flags. I am also of the opinion that external scripts should be used for this > functionality. git f

[PATCH] D68539: [clang-tidy] fix for readability-identifier-naming incorrectly fixes variables which become keywords

MyDeveloperDay added a comment. > I saw this issue when I submitted mine > (https://bugs.llvm.org/show_bug.cgi?id=43306) which for now seems to be a > harder fix, still looking for a solution there. oh boy!...are you gonna have to look at every variable/macro in scope and compare? Reposito

[PATCH] D33029: [clang-format] add option for dangling parenthesis

MyDeveloperDay added a comment. I'd like to assess where we are at with this revision, despite our concerns over additional complexity of clang-format, I don't think this is adding too much (I've seen far worse patches) It appears to me that these changes are mainly in mustBreak,canBreak etc...

[PATCH] D33029: [clang-format] add option for dangling parenthesis

MyDeveloperDay added a comment. I tried the patch and rebased it locally (as there were conflicts with the current trunk) The change caused other tests to fail (which doesn't completely surprise me) most tests failures look associated with positioning of trailing brace for example @@ -1,4 +

[PATCH] D68682: Clang-tidy fixes should avoid creating new blank lines

MyDeveloperDay added a comment. Sorry, I'm going to give a drive-by comment (which you can ignore), mainly by because you mentioned `clang-format`. This seems like a good idea as obviously it solves this problem, however, isn't rather like trying to fix it after the fact? what if (I'm sure the

[PATCH] D68695: [clang-format] Update noexcept reference qualifiers detection

MyDeveloperDay added a comment. Only because I had problem finding the commit, this is as a consequence of D68072: [clang-format] Reference qualifiers in member templates causing extra indentation. Repository: rC Clang CHANGES SINCE LAST ACTION https://re

[PATCH] D68695: [clang-format] Update noexcept reference qualifiers detection

MyDeveloperDay accepted this revision. MyDeveloperDay added a comment. This revision is now accepted and ready to land. Thank you for fixing this, I'm sorry my review probably wasn't thorough enough in the first place, this LGTM and thank you for the very thorough description as this really help

[PATCH] D68695: [clang-format] Update noexcept reference qualifiers detection

MyDeveloperDay added inline comments. Comment at: lib/Format/TokenAnnotator.cpp:1499 +} else if (Current.isOneOf(tok::identifier, tok::kw_const, + tok::kw_noexcept) && Current.Previous && krasimir wrote: > MyDevel

[PATCH] D68707: [clang-format] throws an incorrect assertion in consumeToken() formatting the MSVC stl

MyDeveloperDay created this revision. MyDeveloperDay added reviewers: mitchell-stellar, STL_MSFT, klimek, krasimir. MyDeveloperDay added projects: clang-format, clang-tools-extra. Herald added a project: clang. MyDeveloperDay edited the summary of this revision. An incorrect assertion is thrown wh

[PATCH] D68707: [clang-format] throws an incorrect assertion in consumeToken() formatting the MSVC stl

MyDeveloperDay updated this revision to Diff 224075. MyDeveloperDay marked an inline comment as done. MyDeveloperDay added a comment. I'm not even sure if the assertion is valid? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68707/new/ https://reviews.llvm.org/D68707 Files: clang/lib

[PATCH] D68707: [clang-format] throws an incorrect assertion in consumeToken() formatting the MSVC stl

MyDeveloperDay added inline comments. Comment at: clang/lib/Format/TokenAnnotator.cpp:847 case tok::kw_while: - assert(!Line.startsWith(tok::hash)); - if (Tok->is(tok::kw_if) && CurrentToken && - CurrentToken->isOneOf(tok::kw_constexpr, tok::identifier)) -

[PATCH] D68551: [clang-format] [NFC] Ensure clang-format is itself clang-formatted.

MyDeveloperDay added a comment. > Btw, I am the author of the CMakeLists snippet quoted by @MyDeveloperDay. > Before that, it was a shell script that didn't run on Windows. Making it part > of the regression test basically eliminated all discussion about code > formatting, but we had to run l

[PATCH] D68554: [clang-format] Proposal for clang-format to give compiler style warnings

MyDeveloperDay added a comment. As this behaviour is not something clang-format should do because there are external tool that can do this for us, I thought I'd set myself a challenge of trying to write this with external tools, to prove to myself that it was indeed easy, what I want to achieve

[PATCH] D68682: Clang-tidy fix removals removing all non-blank text from a line should remove the line

MyDeveloperDay added a comment. @poelmanc thank you for the very detailed explanation to what was probably my very naive point, I have to say with more information I am inclined to agree with you that perhaps you need to see all the replacements as a whole to make reasonable assumptions. But I

[PATCH] D29039: [clang-format] Proposal for clang-format -r option

MyDeveloperDay added a comment. Comment at: tools/clang-format/ClangFormat.cpp:345 + + Regex regex(FileNames[i]); + I could imagine the path you supplying being just a directory . or ../../include/Format as much as a wild card, once in that directory I

[PATCH] D68767: [clang-format] NFC - Move functionality into functions to help code structure

MyDeveloperDay created this revision. MyDeveloperDay added reviewers: klimek, mitchell-stellar, owenpan. MyDeveloperDay added a project: clang-format. Herald added a project: clang. I don't think its the main()'s responsibility to be working out how to dump the config, or the format()'s responsi

[PATCH] D68554: [clang-format] Proposal for clang-format to give compiler style warnings

MyDeveloperDay marked 2 inline comments as done. MyDeveloperDay added inline comments. Comment at: clang/tools/clang-format/ClangFormat.cpp:371 + if (!Replaces.empty()) { +IntrusiveRefCntPtr DiagOpts = +new DiagnosticOptions(); klimek wro

[PATCH] D68554: [clang-format] Proposal for clang-format to give compiler style warnings

MyDeveloperDay marked an inline comment as done. MyDeveloperDay added a comment. > I think it's easy enough to do as a kind of driver, either in python or C++, > but then again, at that point putting it into ClangFormat seems fine. One > thing that I find on the one side kinda nice, but on the o

[PATCH] D68554: [clang-format] Proposal for clang-format to give compiler style warnings

MyDeveloperDay updated this revision to Diff 224403. MyDeveloperDay added a comment. builds ontop of D68767: [clang-format] NFC - Move functionality into functions to help code structure move replacement warnings into a separate function and move outputXML out i

[PATCH] D68554: [clang-format] Proposal for clang-format to give compiler style warnings

MyDeveloperDay added a comment. In D68554#1703596 , @klimek wrote: > I ran through the same line of reasoning in my head when writing my last > comment :) The argument about the readability of error messages by tools is > definitely a good one, so I'd sa

[PATCH] D68707: [clang-format] throws an incorrect assertion in consumeToken() formatting the MSVC stl

This revision was automatically updated to reflect the committed changes. Closed by commit rG3b4c8f680712: [clang-format] throws an incorrect assertion in consumeToken() formatting the… (authored by MyDeveloperDay). Changed prior to commit: https://reviews.llvm.org/D68707?vs=224075&id=224415#to

[PATCH] D29039: [clang-format] Proposal for clang-format -r option

MyDeveloperDay added a comment. In D29039#1704166 , @zturner wrote: > In Windows you just write a Python script to do this, and this works > everywhere, not just on one platform or the other, so bash isn't even > necessary and Python is easy to write so

[PATCH] D61663: [clang-format] Fix a JavaScript import order bug.

MyDeveloperDay added a comment. Are you waiting for someone to land this? Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61663/new/ https://reviews.llvm.org/D61663 ___ cfe-commits mailing list cfe-commits@lists.llvm.o

[PATCH] D16066: [clang-format] Add BeforeWhileInDoWhile BraceWrapping option

MyDeveloperDay added a comment. This change need unit tests CHANGES SINCE LAST ACTION https://reviews.llvm.org/D16066/new/ https://reviews.llvm.org/D16066 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mail

[PATCH] D14927: clang-format: Add SpaceBeforeTemplateParameterList option

MyDeveloperDay added a comment. This revision would require unit tests to proceed CHANGES SINCE LAST ACTION https://reviews.llvm.org/D14927/new/ https://reviews.llvm.org/D14927 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists

[PATCH] D56345: [clang-format] Assert that filenames are not empty

MyDeveloperDay accepted this revision. MyDeveloperDay added a comment. This revision is now accepted and ready to land. Thanks for the patch, I'm sorry it's taken so long sometimes items get lost...This looks to still be relevant, I think you'll need to rebase but it LGTM $ clang-format --ass

[PATCH] D27377: clang-format: Support the Java 8 'default' method modifier

MyDeveloperDay added a comment. While reviewing old revisions either not landed or unreviewed I stumbled across this accepted review. Looking at the current code I think its not needed anymore. Perhaps if we merge your test in with the current trunk we can determine if this is still needed C

[PATCH] D68554: [clang-format] Proposal for clang-format to give compiler style warnings

MyDeveloperDay updated this revision to Diff 224742. MyDeveloperDay added a comment. I'm going to commit this as it was approved, but I've added some simple lit tests, which exposed a double free, I fixed that up, and for the lit tests not to fail with an exit code I now return WarningAsError fr

[PATCH] D68767: [clang-format] NFC - Move functionality into functions to help code structure

MyDeveloperDay marked 6 inline comments as done. MyDeveloperDay added a comment. I'll likely abandon this review when D68554: [clang-format] Proposal for clang-format to give compiler style warnings lands Comment at: clang/tools/clang-format/

[PATCH] D68554: [clang-format] Proposal for clang-format to give compiler style warnings

MyDeveloperDay updated this revision to Diff 224743. MyDeveloperDay added a comment. Incorporate feedback from @owenpan on D68767: [clang-format] NFC - Move functionality into functions to help code structure which this revision is reliant upon. CHANGES SINCE

[PATCH] D68554: [clang-format] Proposal for clang-format to give compiler style warnings

This revision was automatically updated to reflect the committed changes. Closed by commit rG1f20bc17d005: [clang-format] Proposal for clang-format to give compiler style warnings (authored by MyDeveloperDay). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm

[PATCH] D68914: [clang-format] Remove duplciate code from Invalid BOM detection

MyDeveloperDay created this revision. MyDeveloperDay added reviewers: bruno, arphaman, klimek, owenpan, mitchell-stellar, dexonsmith. MyDeveloperDay added projects: clang, clang-format, clang-tools-extra. Review comments on D68767: [clang-format] NFC - Move functionality into functions to help c

[PATCH] D68767: [clang-format] NFC - Move functionality into functions to help code structure

MyDeveloperDay abandoned this revision. MyDeveloperDay marked 3 inline comments as done. MyDeveloperDay added a comment. Review comments regarding movign getInvalidBOM are addressed in D68914: [clang-format] Remove duplciate code from Invalid BOM detection Rep

[PATCH] D68914: [clang-format] Remove duplciate code from Invalid BOM detection

MyDeveloperDay updated this revision to Diff 224776. MyDeveloperDay marked 2 inline comments as done. MyDeveloperDay added a comment. Updating with review comments CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68914/new/ https://reviews.llvm.org/D68914 Files: clang/include/clang/Basi

[PATCH] D68554: [clang-format] Proposal for clang-format to give compiler style warnings

MyDeveloperDay added a comment. > I've reverted this for now to green up the bots. > > For the reland, did y'all consider the impact of this on clang-format build > time (it now depends on Frontend and hence on Driver and Sema, and Sema is > slow to build) and binary size (it's now basically im

[PATCH] D68554: [clang-format] Proposal for clang-format to give compiler style warnings

MyDeveloperDay added a comment. In D68554#1707537 , @thakis wrote: > > Thanks for doing this, I am struggling to find the MacOS bot log that > > failed, are any available via Buildbot? (I notice the log looks like your > > own machine) > > The mac bots a

[PATCH] D68914: [clang-format] Remove duplciate code from Invalid BOM detection

MyDeveloperDay added a comment. I tend to agree mainly given that StringRef is used without const elsewhere. I'll bow to better judgement. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68914/new/ https://reviews.llvm.org/D68914 ___ cfe-com

[PATCH] D68969: [clang-format] Remove the dependency on frontend

MyDeveloperDay created this revision. MyDeveloperDay added reviewers: thakis, klimek, mitchell-stellar. MyDeveloperDay added projects: clang, clang-format. Herald added a subscriber: mgorny. Address review comments from D68554: [clang-format] Proposal for clang-format to give compiler style warni

[PATCH] D68554: [clang-format] Proposal for clang-format to give compiler style warnings

MyDeveloperDay added a comment. In D68554#1709316 , @sylvestre.ledru wrote: > @MyDeveloperDay I think it should be added to the release notes. it is a > great new changes for clang format (it would have made my life at Mozilla > much easier ;) I agree

[PATCH] D68969: [clang-format] Remove the dependency on frontend

MyDeveloperDay added a comment. In D68969#1709321 , @klimek wrote: > My intuitive solution would have been to get the char* for the start and > end-location and then search forward and backwards for \n. I may need to, it feels slow for large files which

[PATCH] D68554: [clang-format] Proposal for clang-format to give compiler style warnings

MyDeveloperDay added a subscriber: hans. MyDeveloperDay added a comment. In D68554#1709316 , @sylvestre.ledru wrote: > @MyDeveloperDay I think it should be added to the release notes. it is a > great new changes for clang format (it would have made my li

[PATCH] D68969: [clang-format] Remove the dependency on frontend

MyDeveloperDay updated this revision to Diff 225320. MyDeveloperDay added a comment. Remove need to split lines or search for `\n` General Algorithm 1. take Location of Replacement line and column 2. make a new SourceLocation of line and column =0 (beginning of that replacement line) 3. make a

[PATCH] D68969: [clang-format] Remove the dependency on frontend

MyDeveloperDay marked 2 inline comments as done. MyDeveloperDay added a comment. Comment at: clang/tools/clang-format/ClangFormat.cpp:345 if (WarnFormat && !NoWarnFormat) { +ArrayRef> Ranges; for (const auto &R : Replaces) { klimek wrote: > Looks u

[PATCH] D68969: [clang-format] Remove the dependency on frontend

MyDeveloperDay marked an inline comment as done. MyDeveloperDay added inline comments. Comment at: clang/tools/clang-format/ClangFormat.cpp:351 + SourceLocation LineBegin = Sources.translateFileLineCol( + FileEntryPtr.get(), PLoc.getLine() - 1, 0); + SourceLoca

[PATCH] D68969: [clang-format] Remove the dependency on frontend

MyDeveloperDay updated this revision to Diff 225455. MyDeveloperDay added a comment. Update out by one errors and unused variables CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68969/new/ https://reviews.llvm.org/D68969 Files: clang/tools/clang-format/CMakeLists.txt clang/tools/cla

[PATCH] D68554: [clang-format] Proposal for clang-format to give compiler style warnings

MyDeveloperDay added a comment. In D68554#1712863 , @hans wrote: > > I can't wait for @hans next Windows Snapshot, this is my gate for rolling > > into our builds and CI system, after that, I'm gonna catch every single > > person who tries to build/check

[PATCH] D68554: [clang-format] Proposal for clang-format to give compiler style warnings

MyDeveloperDay marked an inline comment as done. MyDeveloperDay added inline comments. Comment at: clang/test/Format/dry-run-alias.cpp:1-2 +// RUN: clang-format -style=LLVM -i -n %s 2> %t.stderr +// RUN: grep -E "*code should be clang-formatted*" %t.stderr + hlia

[PATCH] D69164: [clang-format] fix regression recognizing casts in Obj-C calls

MyDeveloperDay added a comment. LGTM, thanks for the patch Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69164/new/ https://reviews.llvm.org/D69164 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://list

[PATCH] D68346: [clang-format] Add new option to add spaces around conditions

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

[PATCH] D68346: [clang-format] Add new option to add spaces around conditions

MyDeveloperDay added a comment. Thank you for the patch. If you plan to continue to submit patches I think it would be worthwhile if you applied for commit access, This is not the first revision from you and we need more people who are willing to help with clang-format to fix bug and do code r

[PATCH] D69164: [clang-format] fix regression recognizing casts in Obj-C calls

MyDeveloperDay added a comment. rC373922: [clang-format] [PR27004] omits leading space for noexcept when formatting… was to fix https://bugs.llvm.org/show_bug.cgi?id=27004 which wasn't just related to `delete` it occurred in other cases. (operator++) and ther

[PATCH] D68914: [clang-format] Remove duplciate code from Invalid BOM detection

MyDeveloperDay updated this revision to Diff 225779. MyDeveloperDay marked 2 inline comments as done. MyDeveloperDay added a comment. I think we agree it should be (StringRef ), no const,no reference CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68914/new/ https://reviews.llvm.org/D6891

[PATCH] D68969: [clang-format] Remove the dependency on frontend

MyDeveloperDay marked 2 inline comments as done. MyDeveloperDay added inline comments. Comment at: clang/tools/clang-format/ClangFormat.cpp:356 + + StringRef Line(StartBuf, (EndBuf - StartBuf) - 1); + klimek wrote: > - 1 is to exclude the \n I'd assume? corr

[PATCH] D50078: clang-format: support aligned nested conditionals formatting

MyDeveloperDay added a comment. You know that feeling when you are doing a review and you think... I'm just not a compiler... I feel given the previous discussion and the level of extra tests maybe this is just worth going ahead with as long as you are prepared to support it in the interim.

[PATCH] D50078: clang-format: support aligned nested conditionals formatting

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

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