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

klimek added inline comments. Comment at: clang/tools/clang-format/ClangFormat.cpp:345 if (WarnFormat && !NoWarnFormat) { +ArrayRef> Ranges; for (const auto &R : Replaces) { Looks unused? Comment at: clang/tools/clang-format/ClangFo

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

klimek added a comment. In D68969#1709696 , @MyDeveloperDay wrote: > In D68969#1709321 , @klimek wrote: > > > My intuitive solution would have been to get the char* for the start and > > end-location and then sear

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

klimek added a comment. My intuitive solution would have been to get the char* for the start and end-location and then search forward and backwards for \n. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68969/new/ https://reviews.llvm.org/D68969 __

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

klimek added a comment. In D68914#1709002 , @owenpan wrote: > Here are some `const StringRef &` examples > . StringRef already is a const pointe

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

klimek added a comment. In D68554#1707540 , @thakis wrote: > > Could you or anyone else point me to a good example, I'd definitely like > > to take a look. > > http://llvm-cs.pcc.me.uk/include/llvm/Support/SourceMgr.h/rSMDiagnostic -- > > > `via clang

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

klimek added inline comments. Comment at: clang/include/clang/Basic/SourceManager.h:232 +// nullptr +static const char *getInvalidBOM(StringRef BufStr); }; owenpan wrote: > Can you make the parameter `const`? StringRef is inherently const though, right

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

klimek accepted this revision. klimek added a comment. This revision is now accepted and ready to land. lg Comment at: clang/tools/clang-format/ClangFormat.cpp:293 +// Returns an invalid BOM +static const char *hasInValidBOM(StringRef BufStr) { I'd name this

[PATCH] D40221: [clang-format] Parse blocks in braced lists

klimek added inline comments. Comment at: lib/Format/UnwrappedLineParser.cpp:1324 + nextToken(); + if (!Style.isCpp()) { +// Blocks are only supported in C++ and Objective-C. benhamilton wrote: > Style: Remove curly braces for one-line if blocks. > Isn't t

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

klimek added a comment. In D68554#1703358 , @MyDeveloperDay wrote: > > 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 th

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

klimek added a comment. In D68554#1702090 , @MyDeveloperDay wrote: > 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

[PATCH] D68660: [tooling] Teach Tooling to understand compilation with offloading.

klimek added inline comments. Comment at: clang/lib/Tooling/Tooling.cpp:117 // The one job we find should be to invoke clang again. const auto &Cmd = cast(*Jobs.begin()); if (StringRef(Cmd.getCreator().getName()) != "clang") { tra wrote: > hliao wrote: >

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

klimek added a comment. 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? Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68554/new/ https://reviews.llvm.org/D68554 ___

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

klimek added a comment. In D68296#1691393 , @mitchell-stellar wrote: > Thanks for the review. I have added documentation updates. > > I do not have a public style guide to reference. My company just switched to > auto-clang-formatting all of our code, an

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

klimek accepted this revision. klimek added a comment. This revision is now accepted and ready to land. LG, nice patch, thanks! Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68072/new/ https://reviews.llvm.org/D68072 _

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

klimek accepted this revision. klimek added a comment. This revision is now accepted and ready to land. lg Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67949/new/ https://reviews.llvm.org/D67949 ___ cfe-commits mail

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

klimek accepted this revision. klimek added a comment. This revision is now accepted and ready to land. LG Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67718/new/ https://reviews.llvm.org/D67718 ___ cfe-commits mail

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

klimek accepted this revision. klimek added a comment. This revision is now accepted and ready to land. LG. That makes lots of sense now :) Thanks! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67670/new/ https://reviews.llvm.org/D67670 ___

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

klimek 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.TabWidth : 0); break; Shouldn'

[PATCH] D67541: [ClangFormat] Future-proof Standard option, allow floating or pinning to arbitrary lang version

klimek accepted this revision. klimek added a comment. This revision is now accepted and ready to land. LG Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67541/new/ https://reviews.llvm.org/D67541 ___ c

[PATCH] D67405: Make FormatToken::Type private.

klimek created this revision. klimek added a reviewer: sammccall. Herald added a project: clang. This enables us to intercept changes to the token type via setType(), which is a precondition for being able to use multi-pass formatting for macro arguments. Repository: rG LLVM Github Monorepo h

[PATCH] D66462: Removed the 'id' AST matcher, which is superseded by '.bind()'

klimek accepted this revision. klimek added a comment. This revision is now accepted and ready to land. Yay, thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66462/new/ https://reviews.llvm.org/D66462 ___

[PATCH] D65227: clang-format: Support `if CONSTEXPR` if CONSTEXPR is a macro.

klimek accepted this revision. klimek added a comment. This revision is now accepted and ready to land. I am surprised this works as well as it does, but tests seem to indicate it does, so... LG (minus potentially reducing some duplication :) Comment at: clang/lib/Format/Token

[PATCH] D65092: [clang] Add isDirectlyDerivedFrom AST Matcher.

klimek added a comment. So, I did like the more exhaustive doc, I thought you'd move it to the code so it also shows up in the generated doc page :) (sorry for not being clearer here) Comment at: clang/include/clang/ASTMatchers/ASTMatchers.h:2637 internal::Match

[PATCH] D65125: clang-format: Fix namespace end comments for namespaces with attributes and macros

klimek accepted this revision. klimek added a comment. This revision is now accepted and ready to land. LG, thx! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65125/new/ https://reviews.llvm.org/D65125 ___ cfe-commits mailing list cfe-commi

[PATCH] D65092: [clang] Add isDirectlyDerivedFrom AST Matcher.

klimek requested changes to this revision. klimek added inline comments. This revision now requires changes to proceed. Comment at: clang/docs/LibASTMatchersReference.html:5277 +MatcherCXXRecordDecl>isDirectlyDe

[PATCH] D65092: [clang] Add isDirectlyDerivedFrom AST Matcher.

klimek added inline comments. Comment at: clang/include/clang/ASTMatchers/ASTMatchers.h:2637 internal::Matcher, Base) { - return Finder->classIsDerivedFrom(&Node, Base, Builder); + return Finder->classIsDerivedFrom(&Node, Base, Builder, false); } ---

[PATCH] D63734: Update CODE_OWNERS.txt for clang-doc

klimek accepted this revision. klimek added a comment. This revision is now accepted and ready to land. LG CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63734/new/ https://reviews.llvm.org/D63734 ___ cfe-commits mailing list cfe-commits@lis

[PATCH] D37813: clang-format: better handle namespace macros

klimek accepted this revision. klimek added a comment. This revision is now accepted and ready to land. LG Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D37813/new/ https://reviews.llvm.org/D37813 ___ cfe-commits mail

[PATCH] D37813: clang-format: better handle namespace macros

klimek added inline comments. Comment at: unittests/Format/NamespaceEndCommentsFixerTest.cpp:526 EXPECT_EQ("namespace A {\n" " int i;\n" "} // namespace A", Typz wrote: > Should I also fix these tests? > > They already existed befor

[PATCH] D37813: clang-format: better handle namespace macros

klimek added inline comments. Comment at: unittests/Format/NamespaceEndCommentsFixerTest.cpp:582 + EXPECT_EQ("TESTSUITE() {\n" +" int i;\n" +"} // TESTSUITE()", All of the fixNamespaceEndComments tests are indented, but standard llvm sty

[PATCH] D37813: clang-format: better handle namespace macros

klimek added a comment. In D37813#1512317 , @Typz wrote: > The patch goal is indeed to indent the content of namespace-macro blocks like > the content of any 'regular' namespace. So it should look like the content of > a namespace, possibly depending on

[PATCH] D37813: clang-format: better handle namespace macros

klimek added a comment. In D37813#1254891 , @Typz wrote: > In D37813#1254865 , @klimek wrote: > > > In D37813#1253813 , @Typz wrote: > > > > > In D37813#1184051

[PATCH] D41911: [clangd] Include scanner that finds compile commands for headers.

klimek added inline comments. Comment at: clangd/IncludeScanner.cpp:75 + bool WasEmpty = Queue.empty(); + for (const auto &Cmd : Cmds) { +QueueEntry E(Cmd, VFS); Usually I'd try to not lock a loop, as a large number of compile commands now blocks other thr

[PATCH] D59746: [CommandLineParser] Add DefaultOption flag

klimek accepted this revision. klimek added a comment. This revision is now accepted and ready to land. lg Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59746/new/ https://reviews.llvm.org/D59746 ___ c

[PATCH] D59746: [CommandLineParser] Add DefaultOption flag

klimek added inline comments. Comment at: llvm/lib/Support/CommandLine.cpp:101 + SmallVector DefaultOptions; + A comment explaining what this contains and how it'll be used would help. Comment at: llvm/lib/Support/CommandLine.cpp:152 +

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

klimek added a comment. In D54881#1462893 , @MyDeveloperDay wrote: > > 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

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

klimek added a comment. In D54881#1462804 , @russellmcc wrote: > Thanks for the explanation! I do understand your philosophy on this, and > agree with the desired behavior case you brought up where you have put in new > braces. > > After thinking about

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

klimek added a comment. In D54881#1449848 , @russellmcc wrote: > klimek: I'm sorry, I don't fully understand your proposed fix. Could you > explain it in more detail? > > Without being able to respond to your proposal in detail, I strongly believe > if

[PATCH] D59746: [LibTooling] Fix '-h' option

klimek added a comment. In D59746#1458539 , @hintonda wrote: > In D59746#1458461 , @hintonda wrote: > > > In D59746#1458432 , @klimek wrote: > > > > > If we make it an alias

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

klimek added inline comments. Comment at: clang/include/clang/Format/Format.h:50 struct FormatStyle { + /// Indents after access modifiers. i.e. + /// \code I think we need to explain this a bit more: What this does is: Indent classes with access modifiers at

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

klimek 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 statement. + MyDeveloperDay wrote: > klimek w

[PATCH] D59332: [clang-format] AlignConsecutiveDeclarations fails with attributes

klimek added inline comments. Comment at: clang/lib/Format/WhitespaceManager.cpp:468 C.Tok->is(TT_FunctionDeclarationName) || + C.Tok->startsSequence( + tok::l_paren, tok::l_paren, tok::identifier, tok::coloncolon,

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

klimek added inline comments. Comment at: clang/lib/Format/TokenAnnotator.cpp:2467-2470 + if (Line.InPPDirective && Right.is(tok::l_paren) && + !Left.is(tok::identifier) && Left.Previous && + Left.Previous->is(tok::identifier) && Left.Previous->Previous && + Left.

[PATCH] D59746: [LibTooling] Fix '-h' option

klimek added a comment. In D59746#1458424 , @hintonda wrote: > In D59746#1458086 , @klimek wrote: > > > In D59746#1440756 , @hintonda > > wrote: > > > > > A better alternati

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

klimek added inline comments. Comment at: cfe/trunk/docs/ClangFormatStyleOptions.rst:384 + + * ``SIS_WithoutElse`` (in configuration: ``WithoutElse``) +Allow short if functions on the same line, as long as else MyDeveloperDay wrote: > klimek wrote: > > Is th

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

klimek added inline comments. Comment at: cfe/trunk/docs/ClangFormatStyleOptions.rst:384 + + * ``SIS_WithoutElse`` (in configuration: ``WithoutElse``) +Allow short if functions on the same line, as long as else Is that actually the status quo or is the curre

[PATCH] D59746: [LibTooling] Fix '-h' option

klimek added a comment. In D59746#1440756 , @hintonda wrote: > A better alternative would have been to add a cl::aliasopt for '-h' in llvm's > CommandLineParser when '-help' was first added. However, that's no longer > possible since some llvm based too

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

klimek accepted this revision. klimek added a comment. Apart from the typo I think this is a simple enough change and a widely enough used style that it LG. Comment at: lib/Format/UnwrappedLineParser.cpp:181 + CompoundStatementIndenter(UnwrappedLineParser *Parser, unsigned &L

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

klimek accepted this revision. klimek added a comment. lg CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60320/new/ https://reviews.llvm.org/D60320 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailma

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

klimek added a comment. The previous behavior looks intentional, and much more regular. I'd be curious why you think the proposed behavior is more readable. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D60374/new/ https://reviews.llvm.org/D60374 _

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

klimek accepted this revision. klimek added a comment. This revision is now accepted and ready to land. lg 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 // iden

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

klimek added inline comments. Comment at: lib/Format/WhitespaceManager.cpp:520 + + AlignMacroSequence(StartOfSequence, EndOfSequence, MinColumn, MaxColumn, + FoundMatchOnLine, AlignMacrosMatches, Changes); Why are we calling AlignMacroSequenc

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

klimek added a comment. I just verified why this doesn't break for our CI: The trick is that if the indent is actually off (as opposed to being correct, but tab vs spaces), we do re-indent until we find an indent that's correct. The fix that would make me happier, I think, is to do the same thing

[PATCH] D59774: [clang-format] Refine structured binding detection

klimek accepted this revision. klimek added a comment. This revision is now accepted and ready to land. lg Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59774/new/ https://reviews.llvm.org/D59774 ___ cfe-commits mail

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

klimek accepted this revision. klimek added a comment. This revision is now accepted and ready to land. LG CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59684/new/ https://reviews.llvm.org/D59684 ___ cfe-commits mailing list cfe-commits@lis

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

klimek added a comment. Minus me understanding the TT_TemplateString change, the rest looks nice now, thanks! 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 //

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

klimek accepted this revision. klimek added a comment. Oh, and LG :) CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55170/new/ https://reviews.llvm.org/D55170 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi

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

klimek added a comment. In D55170#1439864 , @MyDeveloperDay wrote: > @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 a

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

klimek added inline comments. Comment at: lib/Format/WhitespaceManager.cpp:482 + // containing any matching token to be aligned and located after such token. + auto AlignCurrentSequence = [&] { +if (StartOfSequence > 0 && StartOfSequence < EndOfSequence) {

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

klimek accepted this revision. klimek added a comment. This revision is now accepted and ready to land. LG CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57687/new/ https://reviews.llvm.org/D57687 ___ cfe-commits mailing list cfe-commits@lis

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

klimek accepted this revision. klimek added a comment. This revision is now accepted and ready to land. LG. This looks pretty sharp (scnr). Comment at: clang/lib/Format/FormatTokenLexer.cpp:177 +if (Dollar->TokenText == "$") { + // this looks like $@"a" so we need

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

klimek added a comment. Great idea! LG from my side after addressing MyDeveloperDay's comments. Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D42034/new/ https://reviews.llvm.org/D42034 ___ cfe-commits mailing list cfe

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

klimek added inline comments. Comment at: lib/Format/TokenAnnotator.cpp:2546-2560 return Line.Type == LT_ObjCDecl || Left.is(tok::semi) || (Style.SpaceBeforeParens != FormatStyle::SBPO_Never && -(Left.isOneOf(tok::kw_if, tok::pp_elif, tok::kw_for, tok

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

klimek added a comment. I don't understand yet why running clang-format locally would not do the same change? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54881/new/ https://reviews.llvm.org/D54881 ___ cfe-commits mailing list cfe-commits

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

klimek accepted this revision. klimek added a comment. This revision is now accepted and ready to land. lg CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59546/new/ https://reviews.llvm.org/D59546 ___ cfe-commits mailing list cfe-commits@lis

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

klimek added inline comments. Comment at: clang/lib/Format/TokenAnnotator.cpp:2976 + +return Style.AllowShortLambdasOnASingleLine == FormatStyle::SLS_None || + Style.AllowShortLambdasOnASingleLine == FormatStyle::SLS_Inline || If SLS_All is supposed

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

klimek added inline comments. Comment at: lib/Format/WhitespaceManager.cpp:436 +static void +AlignTokenSequenceAll(unsigned Start, unsigned End, unsigned Column, + F &&Matches, VelocityRa wrote: > klimek wrote: > > I don't think the 'All' pos

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

klimek 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 statement. + I'd try to make this either unco

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

klimek 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; } This change looks pretty good, minus the changes outside Format

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

klimek added a comment. In D55170#1432929 , @MyDeveloperDay wrote: > > 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

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

klimek added inline comments. Comment at: lib/Format/TokenAnnotator.cpp:2546-2560 return Line.Type == LT_ObjCDecl || Left.is(tok::semi) || (Style.SpaceBeforeParens != FormatStyle::SBPO_Never && -(Left.isOneOf(tok::kw_if, tok::pp_elif, tok::kw_for, tok

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

klimek added a comment. In D55170#1432832 , @reuk wrote: > @klimek I agree that the rule is somewhat arbitrary. However, it's the style > rule of an established codebase with many users (I don't have a concrete > number, but the project has 1400 stars on

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

klimek added a comment. In D55170#1431854 , @MyDeveloperDay wrote: > > 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 un

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

klimek added a comment. In D40988#1430502 , @MyDeveloperDay wrote: > 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 getting n

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

klimek added a comment. Yea, the a b(c) problem is the olden C++ problem clang-format will never be able to solve, thus all these crazy heuristics. So, inching closer to understanding this - I totally missed the isLiteral() that short-circuits out before. I'm more happy with your current approa

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

klimek added a comment. In D28462#1406716 , @jacob-keller wrote: > In regards to whether clang-format should support this feature, I think it's > pretty clear that a large number of users want it to. The tool already > supports formatting various other

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

klimek added a comment. In D59309#1428969 , @MyDeveloperDay wrote: > That works because the argument list is just tok::identifier and > TT_StartOfName Should we fix isStartOfName then? In A or A<8> A should be TT_StartOfName, too, right? CHANGES SIN

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

klimek accepted this revision. klimek added a comment. Given this doesn't add an extra option (but only an extra enum) and is fairly straight forward, LG. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D52150/new/ https://reviews.llvm.org/D52150 ___

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

klimek added a comment. In D55170#1423941 , @MyDeveloperDay wrote: > In D55170#1423798 , @reuk wrote: > > > Thanks for the review, and for approving this PR. It's very much > > appreciated! > > > > I don't have me

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

klimek added a comment. In D59309#1428807 , @MyDeveloperDay wrote: > In D59309#1428799 , @klimek wrote: > > > Why did the numeric constant break this? Which path would the function run > > through? > > > as its lo

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

klimek added a comment. In D33029#1423949 , @MyDeveloperDay wrote: > In D33029#1423947 , @lebedev.ri > wrote: > > > In D33029#1423944 , > > @MyDeveloperDay wrote: > > > >

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

klimek added a comment. Why did the numeric constant break this? Which path would the function run through? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59309/new/ https://reviews.llvm.org/D59309 ___ cfe-commits mailing list cfe-commits@l

[PATCH] D58345: [clangd] Using symbol name to map includes for STL symbols.

klimek added a comment. In D58345#1406892 , @sammccall wrote: > Sorry to be a pain here, but I'm not sure introducing Javascript is a good > idea unless there's a strong reason for it. > All LLVM developers have python installed, many are comfortable wit

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

klimek added a comment. In D28462#1405360 , @MyDeveloperDay wrote: > 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

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

klimek added a comment. In D28462#1405855 , @MyDeveloperDay wrote: > In D28462#1405711 , @klimek wrote: > > > In D28462#1405360 , > > @MyDeveloperDay wrote: > > > > > I'm n

[PATCH] D56841: [clangd] Filter out plugin related flags.

klimek added a comment. In D56841#1361395 , @kadircet wrote: > It seems like the most relevant place in tooling is ArgumentsAdjuster as > @ioeric pointed out. Happy to move it to there if @sammccall and @klimek > agrees as well. > > As a side note `Argum

[PATCH] D56444: [AST] RecursiveASTVisitor visits lambda classes when implicit visitation is on.

klimek added a comment. Just in general, I'd like to add that my experience over the years dealing with folks trying to do AST matchers is that the inability to express something is much more of a problem than matching too much, simply because the test cases people think of are usually small, a

[PATCH] D56444: [AST] RecursiveASTVisitor visits lambda classes when implicit visitation is on.

klimek added a comment. In D56444#1351056 , @aaron.ballman wrote: > In D56444#1350849 , @JonasToth wrote: > > > In D56444#1350746 , @sammccall > > wrote: > > > > > @klimek:

[PATCH] D56444: [AST] RecursiveASTVisitor visits lambda classes when implicit visitation is on.

klimek added a comment. In D56444#1350746 , @sammccall wrote: > In D56444#1350252 , @JonasToth wrote: > > > I still see the unit-test crashing for `ExprMutAnalyzer` (just apply the > > last two tests > > https://g

[PATCH] D56090: Add a matcher for members of an initializer list expression

klimek added inline comments. Comment at: include/clang/ASTMatchers/ASTMatchers.h:3527 + return N < Node.getNumInits() && + InnerMatcher.matches(*Node.getInit(N)->IgnoreParenImpCasts(), Finder, + Builder); aaron.ballman wro

[PATCH] D54309: [AST] Allow limiting the scope of common AST traversals (getParents, RAV).

klimek added a comment. In D54309#1326852 , @JonasToth wrote: > See PR39949 as well, as it seems to trigger the same or a similar problem. > @ioeric and @klimek maybe could give an opinion, too? Sounds like we might not correctly set the parent map for

[PATCH] D54672: clang-include-fixer.el: support remote files

klimek accepted this revision. klimek added a comment. This revision is now accepted and ready to land. LG Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54672/new/ https://reviews.llvm.org/D54672 ___ cf

[PATCH] D54952: [clangd] DO NOT SUBMIT. Draft interfaces for build system integration.

klimek added inline comments. Comment at: clangd/BuildSystem.h:29 +/// Default compilation database used by clangd, based on the build system. +class Integration : public GlobalCompilationDatabase { +public: ilya-biryukov wrote: > klimek wrote: > > 'Integration'

[PATCH] D54952: [clangd] DO NOT SUBMIT. Draft interfaces for build system integration.

klimek added inline comments. Comment at: clangd/BuildSystem.h:29 +/// Default compilation database used by clangd, based on the build system. +class Integration : public GlobalCompilationDatabase { +public: 'Integration' is a bit of a weird name, as it doesn't t

[PATCH] D54453: Remove myself as owner of clang-query.

klimek added a comment. Thanks Aaron for volunteering, I'm very thankful for all your work on the reviews, it's much appreciated :D Repository: rL LLVM https://reviews.llvm.org/D54453 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http:

[PATCH] D54077: [clangd] Implemented DraftFileSystem

klimek added a comment. In https://reviews.llvm.org/D54077#1288404, @ioeric wrote: > In https://reviews.llvm.org/D54077#1288387, @sammccall wrote: > > > Someone mentioned to me that the interaction-between-features argument > > wasn't clear here: > > > > - we **don't** currently update diagnosti

[PATCH] D54077: [clangd] Implemented DraftFileSystem

klimek added a comment. In https://reviews.llvm.org/D54077#1287282, @LutsenkoDanil wrote: > @klimek If behavior will be configurable, is it ok for you? I have the same concerns as Sam for making this an option. > @sammccall Current behavior may confuse new users, since, other IDEs mostly > (a

[PATCH] D54077: [clangd] Implemented DraftFileSystem

klimek added a comment. In https://reviews.llvm.org/D54077#1287040, @ilya-biryukov wrote: > Thanks for the patch! I believe many people I talked to want this behavior > (myself included). > Some people like what we do now more. It feels like it depends on the > workflow: for people who auto-sa

[PATCH] D37813: clang-format: better handle namespace macros

klimek added a comment. In https://reviews.llvm.org/D37813#1253813, @Typz wrote: > In https://reviews.llvm.org/D37813#1184051, @klimek wrote: > > > The problem here is that we have different opinions on how the formatting > > on namespace macros should behave in the first place- I think they sho

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

klimek added inline comments. Comment at: lib/Format/ContinuationIndenter.cpp:1307 + (Style.BraceWrapping.BeforeLambdaBody && Current.Next != nullptr && +Current.Next->is(TT_LambdaLSquare))); State.Stack.back().IsInsideObjCArrayLiteral = Wawha wr

<    1   2   3   4   5   6   >