[PATCH] D51258: Extract parseBindID method

klimek added inline comments. Comment at: lib/ASTMatchers/Dynamic/Parser.cpp:362 +bool Parser::parseBindID(std::string , TokenInfo ) { + // Parse .bind("foo") CloseToken seems to not be used afterwards either here or in the follow-up patch? Repository:

[PATCH] D51259: Allow binding to NamedValue resulting from let expression

klimek accepted this revision. klimek added a comment. This revision is now accepted and ready to land. LG Repository: rC Clang https://reviews.llvm.org/D51259 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D51261: Add preload option to clang-query

klimek accepted this revision. klimek added a comment. This revision is now accepted and ready to land. LG Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D51261 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[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 = klimek

[PATCH] D50697: [clang-format] fix PR38557 - comments between "default" and ':' causes the case label to be treated as an identifier

klimek accepted this revision. klimek added a comment. This revision is now accepted and ready to land. lg Repository: rC Clang https://reviews.llvm.org/D50697 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D45719: [clang-Format] Fix indentation of member call after block

klimek added a comment. Sorry that I lost track of that, but feel free to ping once / week - unfortunately the patch doesn't apply cleanly, can you rebase? Repository: rC Clang https://reviews.llvm.org/D45719 ___ cfe-commits mailing list

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

klimek added a comment. 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 should behave like namespaces, you want them to be formatted differently. Given that you want full control over the formatting of

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

klimek added a comment. Yea, if we go down this route I'd go with this by default: some ? thing : else ? otherthing : unrelated ? but : finally; Theoretically we could even use: some ? thing : else; by default ;) Repository: rC Clang https://reviews.llvm.org/D50078

[PATCH] D49840: [AST] Add MatchFinder::matchSubtree

klimek added a comment. Usually we use match(anyOf(node), hasDescendant(node)). Or did I misunderstand what you want? Repository: rC Clang https://reviews.llvm.org/D49840 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D49724: [VFS] Cleanups to VFS interfaces.

klimek accepted this revision. klimek added a comment. This revision is now accepted and ready to land. lg Repository: rC Clang https://reviews.llvm.org/D49724 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[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 = I think

[PATCH] D44609: [Clang-Format] New option BreakBeforeLambdaBody to manage lambda line break inside function parameter call

klimek added inline comments. Comment at: lib/Format/TokenAnnotator.cpp:627 } +if(Style.BraceWrapping.BeforeLambdaBody && Current->is(TT_LambdaLSquare)) { +++Left->BlockParameterCount; Wawha wrote: > klimek wrote: > > Why do we want to increase

[PATCH] D44609: [Clang-Format] New option BreakBeforeLambdaBody to manage lambda line break inside function parameter call

klimek added inline comments. Herald added a subscriber: acoomans. Comment at: lib/Format/TokenAnnotator.cpp:627 } +if(Style.BraceWrapping.BeforeLambdaBody && Current->is(TT_LambdaLSquare)) { +++Left->BlockParameterCount; Why do we want to

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

klimek added a comment. In https://reviews.llvm.org/D28462#1148732, @enyquist wrote: > @klimek having gotten that out of the way, I do occasionally drink too much > and have sudden urges to re-implement things from scratch. Close it if you > need to, since I can't commit to anything, but

[PATCH] D45094: [LibTooling] Make interface of VFS injection into ClangTool more user-friendly

klimek added inline comments. Comment at: lib/Basic/VirtualFileSystem.cpp:328 void OverlayFileSystem::pushOverlay(IntrusiveRefCntPtr FS) { + // FIXME: OverlayFS containing another one in its stack could be flattened. FSList.push_back(FS); ilya-biryukov

[PATCH] D48159: [clangd] Implement hover for "auto" and "decltype"

klimek accepted this revision. klimek added inline comments. Comment at: clangd/XRefs.cpp:559 + //- auto& i = 1; + bool VisitDeclaratorDecl(DeclaratorDecl *D) { +if (!D->getTypeSourceInfo() || klimek wrote: > malaperle wrote: > > klimek wrote: > > >

[PATCH] D48159: [clangd] Implement hover for "auto" and "decltype"

klimek added a reviewer: rsmith. klimek added inline comments. Comment at: clangd/XRefs.cpp:559 + //- auto& i = 1; + bool VisitDeclaratorDecl(DeclaratorDecl *D) { +if (!D->getTypeSourceInfo() || malaperle wrote: > klimek wrote: > > sammccall wrote: > > >

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

klimek added a comment. In https://reviews.llvm.org/D28462#1147019, @enyquist wrote: > @klimek fair point. To be honest, I've pretty much lost interest / momentum > on this feature, I very much doubt I will ever go back and re-implement from > scratch as you suggest. > Not meaning to sound

[PATCH] D48159: [clangd] Implement hover for "auto" and "decltype"

klimek added inline comments. Comment at: clangd/XRefs.cpp:559 + //- auto& i = 1; + bool VisitDeclaratorDecl(DeclaratorDecl *D) { +if (!D->getTypeSourceInfo() || sammccall wrote: > malaperle wrote: > > sammccall wrote: > > > out of curiosity, why not

[PATCH] D44609: [Clang-Format] New option BreakBeforeLambdaBody to manage lambda line break inside function parameter call

klimek added a comment. In https://reviews.llvm.org/D44609#1143895, @Wawha wrote: > Hello, > > after my last modification (require by previous comment), I do not see any > feedback or validation for this patch. > Is their something special to do in order to go forward on this patch? > >

[PATCH] D45719: [clang-Format] Fix indentation of member call after block

klimek accepted this revision. klimek added a comment. This revision is now accepted and ready to land. LG. Repository: rC Clang https://reviews.llvm.org/D45719 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D45719: [clang-Format] Fix indentation of member call after block

klimek added inline comments. Comment at: unittests/Format/FormatTest.cpp:4449-4450 + " })\n" + " .foo(\"aaa\"\n" + " \"bb\");\n" +

[PATCH] D45719: [clang-Format] Fix indentation of member call after block

klimek added inline comments. Comment at: unittests/Format/FormatTest.cpp:4359 + "return 3;\n" + " }).as("");\n" + "}"); ank wrote: > klimek wrote: > > ank wrote: > > > klimek wrote: > > > > What would be

[PATCH] D45719: [clang-Format] Fix indentation of member call after block

klimek added inline comments. Comment at: unittests/Format/FormatTest.cpp:4359 + "return 3;\n" + " }).as("");\n" + "}"); ank wrote: > klimek wrote: > > What would be interesting is tests that: > > a) have another

[PATCH] D48259: [clang-format] Fix bug with UT_Always when there is less than one full tab

klimek added inline comments. Comment at: lib/Format/WhitespaceManager.cpp:678 // Indent with tabs only when there's at least one full tab. -if (FirstTabWidth + Style.TabWidth <= Spaces) { +if (Style.TabWidth <= Spaces) { Spaces -= FirstTabWidth;

[PATCH] D48242: [ASTMatchers] Add support for matching the type of a friend decl.

klimek accepted this revision. klimek added a comment. lg, thx Repository: rC Clang https://reviews.llvm.org/D48242 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D48269: [ASTMatchers] Don't assert-fail in specifiesTypeLoc().

klimek accepted this revision. klimek added a comment. This revision is now accepted and ready to land. lg, thanks! Repository: rC Clang https://reviews.llvm.org/D48269 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D48259: [clang-format] Fix bug with UT_Always when there is less than one full tab

klimek added inline comments. Comment at: lib/Format/WhitespaceManager.cpp:678 // Indent with tabs only when there's at least one full tab. -if (FirstTabWidth + Style.TabWidth <= Spaces) { +if (Style.TabWidth <= Spaces) { Spaces -= FirstTabWidth;

[PATCH] D48242: [ASTMatchers] Add support for matching the type of a friend decl.

klimek added inline comments. Comment at: include/clang/ASTMatchers/ASTMatchers.h:2904 +internal::Matcher, InnerMatcher, 1) { + QualType QT = internal::getUnderlyingType(Node); + if (!QT.isNull()) In which cases can getUnderlyingType return a null type?

[PATCH] D47759: [Format] Do not use a global static value for EOF within ScopedMacroState.

klimek accepted this revision. klimek added a comment. This revision is now accepted and ready to land. LG. Repository: rC Clang https://reviews.llvm.org/D47759 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D46024: [clang-format] Add SpaceBeforeCpp11BracedList option.

klimek accepted this revision. klimek added a comment. In https://reviews.llvm.org/D46024#1129350, @hans wrote: > In https://reviews.llvm.org/D46024#1121242, @rkirsling wrote: > > > FWIW, please note that this space-before-brace style is not specific to > > WebKit; CppCoreGuidelines exhibits it

[PATCH] D47095: [clang-format/ObjC] Correctly parse Objective-C methods with 'class' in name

klimek accepted this revision. klimek added a comment. LG Repository: rC Clang https://reviews.llvm.org/D47095 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D36610: [Tooling] Add option to getFullyQualifiedName using a custom PritingPolicy

klimek added a comment. I believe this was accepted by rnk - are you waiting for specific further feedback? https://reviews.llvm.org/D36610 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D46024: [clang-format] Add SpaceBeforeCpp11BracedList option.

klimek added a comment. Is this written down somewhere? https://webkit.org/code-style-guidelines/ doesn't seem to mention it. Repository: rC Clang https://reviews.llvm.org/D46024 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D44609: [Clang-Format] New option BreakBeforeLambdaBody to manage lambda line break inside function parameter call

klimek added inline comments. Comment at: include/clang/Format/Format.h:891 + /// \endcode + bool BreakBeforeLambdaBody; + I'd just make that default for Allman style. Comment at: lib/Format/TokenAnnotator.cpp:2844 if (isAllmanBrace(Left)

[PATCH] D46062: [clang-format] Start formatting cpp code in raw strings in google style

klimek accepted this revision. klimek added a comment. This revision is now accepted and ready to land. LG Repository: rC Clang https://reviews.llvm.org/D46062 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D45719: [clang-Format] Fix indentation of member call after block

klimek added inline comments. Comment at: unittests/Format/FormatTest.cpp:4359 + "return 3;\n" + " }).as("");\n" + "}"); What would be interesting is tests that: a) have another value after the closing }; doesn't

[PATCH] D45726: Format closing braces when reformatting the line containing theopening brace.

This revision was automatically updated to reflect the committed changes. Closed by commit rL330573: Format closing braces when reformatting the line containing the opening brace. (authored by klimek, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM

[PATCH] D45726: Format closing braces when reformatting the line containing theopening brace.

klimek updated this revision to Diff 143274. klimek marked 2 inline comments as done. klimek added a comment. Address comments. Repository: rC Clang https://reviews.llvm.org/D45726 Files: lib/Format/AffectedRangeManager.cpp lib/Format/AffectedRangeManager.h lib/Format/Format.cpp

[PATCH] D45726: Format closing braces when reformatting the line containing theopening brace.

klimek added a comment. In https://reviews.llvm.org/D45726#1071925, @krasimir wrote: > Another point: for the example in the summary about bailing-out early, is > there a test for this already? If not, we should add one. Yep, there already is one - I regressed that with my change at first ;)

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

klimek added inline comments. Comment at: lib/Format/WhitespaceManager.cpp:438 + + AlignTokens(Style, + [&](const Change ) { I'm not sure whether we should use AlignTokens here, given that we pass in a parameter to basically skip all its

[PATCH] D45726: Format closing braces when reformatting the line containing theopening brace.This required a couple of yaks to be shaved:1. MatchingOpeningBlockLineIndex was misused to also store the

klimek created this revision. klimek added a reviewer: krasimir. ...doesn't work correctly for "} else {". 2. We needed to change the API of AffectedRangeManager to not use iterators; we always passed in begin / end for the whole container before, so there was no mismatch in generality. 3.

[PATCH] D38615: [libclang] Only mark CXCursors for explicit attributes with a type

klimek accepted this revision. klimek added a comment. This revision is now accepted and ready to land. lg https://reviews.llvm.org/D38615 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D43969: Improve completion experience for headers

klimek accepted this revision. klimek added a comment. This revision is now accepted and ready to land. lg Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D43969 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D42036: [clang-format] Keep comments aligned to macros

klimek added a comment. Just from a formatting point of view, why not: //. Comment #.define X Repository: rC Clang https://reviews.llvm.org/D42036 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D41487: [clang-format] Adds a FormatStyleSet

klimek added inline comments. Comment at: lib/Format/Format.cpp:906-907 + } + if (!LanguageFound) +return make_error_code(ParseError::Unsuitable); + *Style = *StyleSet.Get(Language); Optional: I'd probably slightly re-structure the above to: if

[PATCH] D41487: [clang-format] Adds a FormatStyleSet

klimek added inline comments. Comment at: lib/Format/Format.cpp:893 for (int i = Styles.size() - 1; i >= 0; --i) { -if (Styles[i].Language == Language || -Styles[i].Language == FormatStyle::LK_None) { +if (!LanguageFound && (Styles[i].Language == Language || +

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

klimek added a comment. In https://reviews.llvm.org/D37813#958116, @Typz wrote: > OK. > > So you mean a solution like the one discussed earlier would be the way to go? > > > I mean that we can configure macros in the format style, like "define A(X) > > class X {". I'm not 100% sure whether we

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

klimek added a comment. I've talked with Daniel and we both believe this patch is not the right way to go forward for clang-format. It is adding configuration mechanisms that we need to maintain going forward, without addressing a general problem; specifically for how to handle macros, I

[PATCH] D41130: git-clang-format: cleanup: Use run() when possible.

klimek accepted this revision. klimek added a comment. This revision is now accepted and ready to land. lg https://reviews.llvm.org/D41130 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D41147: git-clang-format: Add new --staged option.

klimek accepted this revision. klimek added a comment. This revision is now accepted and ready to land. lg https://reviews.llvm.org/D41147 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D41145: git-clang-format: refactor to support upcoming --staged flag

klimek accepted this revision. klimek added a comment. This revision is now accepted and ready to land. lg Comment at: google3/third_party/llvm/llvm/tools/clang/tools/clang-format/git-clang-format:252 + assert len(commits) != 0 + if len(commits) >= 2: +return

[PATCH] D40485: [clangd] Introduced a Context that stores implicit data

klimek added inline comments. Comment at: clangd/Context.h:65 + Context *Parent; + TypedValueMap Data; +}; sammccall wrote: > ilya-biryukov wrote: > > sammccall wrote: > > > ilya-biryukov wrote: > > > > sammccall wrote: > > > > > We add complexity here

[PATCH] D40909: [clang-format] Reorganize raw string delimiters

klimek added inline comments. Comment at: include/clang/Format/Format.h:1375 +std::vector EnclosingFunctionNames; +/// \brief The canonical delimiter for this language. +std::string CanonicalDelimiter; djasper wrote: > krasimir wrote: > > djasper

[PATCH] D39027: [docs][refactor] Add a new tutorial that talks about how one can implement refactoring actions

klimek added inline comments. Comment at: docs/RefactoringActionTutorial.rst:7 + + This tutorial talks about a work-in-progress library in Clang. + Some of the described features might not be available yet in trunk, but should hokein wrote: > I'm a bit

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

klimek added a comment. In https://reviews.llvm.org/D37813#945125, @Typz wrote: > I don't think this is really relevant for this tool: if someone changes the > implementation of the macro, then *they* must indeed if it should not be > formatted like a namespace (and keep the clang-format

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

klimek added a comment. In https://reviews.llvm.org/D37813#942137, @Typz wrote: > As far as "parsing" and formatting inside the block is concerned, this is > indeed unrelated (and would totally work if all macros where specified with > some preprocessor definitions). > > But identifying the

[PATCH] D33589: clang-format: consider not splitting tokens in optimization

klimek added a comment. In https://reviews.llvm.org/D33589#942128, @Typz wrote: > Indeed, looks good, thanks! > > Though that exacerbates the alignment issue, I now get things like this: > > enum { > SomeLongerEnum, // comment > SomeThing, // comment > Foo, // something > }

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

klimek added a comment. In https://reviews.llvm.org/D37813#941987, @Typz wrote: > Definitely that would be much more expressive. But this approach is also > incomplete: in case of namespace (and possibly others?), we also need to > perform the reverse operation, e.g. to "generate" a macro call

[PATCH] D33589: clang-format: consider not splitting tokens in optimization

klimek added a comment. In https://reviews.llvm.org/D33589#941979, @Typz wrote: > I think the difference between code and comments is that code "words" are > easily 10 characters or more, whereas actual words (in comments) are very > often less than 10 characters: so code overflowing by 10

[PATCH] D40605: Better trade-off for excess characters vs. staying within the column limits.

This revision was automatically updated to reflect the committed changes. Closed by commit rL319541: Better trade-off for excess characters vs. staying within the column limits. (authored by klimek). Repository: rL LLVM https://reviews.llvm.org/D40605 Files:

[PATCH] D40605: Better trade-off for excess characters vs. staying within the column limits.

klimek updated this revision to Diff 125094. klimek marked an inline comment as done. klimek added a comment. Add test. Repository: rC Clang https://reviews.llvm.org/D40605 Files: lib/Format/ContinuationIndenter.cpp lib/Format/ContinuationIndenter.h unittests/Format/FormatTest.cpp

[PATCH] D40605: Better trade-off for excess characters vs. staying within the column limits.

klimek updated this revision to Diff 124940. klimek added a comment. Fix incorrect return value leading to unnecessary computation. Repository: rC Clang https://reviews.llvm.org/D40605 Files: lib/Format/ContinuationIndenter.cpp lib/Format/ContinuationIndenter.h

[PATCH] D40605: Better trade-off for excess characters vs. staying within the column limits.

klimek added inline comments. Comment at: lib/Format/ContinuationIndenter.cpp:1422 + Strict); +} } krasimir wrote: > The problem here is that we're calling breakProtrudingToken 3 times more than > we used to. Seems like such a

[PATCH] D40605: Better trade-off for excess characters vs. staying within the column limits.

klimek created this revision. When we break a long line like: Column limit: 21 | // foo foo foo foo foo foo foo foo foo foo foo foo The local decision when to allow protruding vs. breaking can lead to this outcome (2 excess characters, 2 breaks): // foo foo foo foo

[PATCH] D40310: Restructure how we break tokens.

This revision was automatically updated to reflect the committed changes. Closed by commit rL319314: Restructure how we break tokens. (authored by klimek). Repository: rL LLVM https://reviews.llvm.org/D40310 Files: cfe/trunk/lib/Format/BreakableToken.cpp

[PATCH] D40310: Restructure how we break tokens.

klimek added inline comments. Comment at: lib/Format/ContinuationIndenter.cpp:1749 + } + if (!Reflow) { +// If we didn't reflow into the next line, the only space to consider is krasimir wrote: > nit: Maybe change this to `if (Reflow)` and

[PATCH] D40310: Restructure how we break tokens.

klimek added inline comments. Comment at: lib/Format/ContinuationIndenter.cpp:1707 + RemainingTokenColumns = Token->getRemainingLength( + NextLineIndex, TailOffset, ContentStartColumn); + Reflow = true; krasimir wrote: > When we're

[PATCH] D40310: Restructure how we break tokens.

klimek updated this revision to Diff 124709. klimek marked 4 inline comments as done. klimek added a comment. Address review comments. https://reviews.llvm.org/D40310 Files: lib/Format/BreakableToken.cpp lib/Format/BreakableToken.h lib/Format/ContinuationIndenter.cpp

[PATCH] D40310: Restructure how we break tokens.

klimek added inline comments. Comment at: lib/Format/ContinuationIndenter.cpp:1525 + if (!DryRun) +Token->adaptStartOfLine(0, Whitespaces); + krasimir wrote: > If we indent here, shouldn't that also change ContentStartColumn? Nope, this will exactly adapt

[PATCH] D40310: Restructure how we break tokens.

klimek updated this revision to Diff 124581. klimek marked 3 inline comments as done. klimek added a comment. Address review comments. https://reviews.llvm.org/D40310 Files: lib/Format/BreakableToken.cpp lib/Format/BreakableToken.h lib/Format/ContinuationIndenter.cpp

[PATCH] D40072: [libclang] Support querying whether a declaration is invalid

klimek accepted this revision. klimek added a comment. This revision is now accepted and ready to land. LG Comment at: include/clang-c/Index.h:2622 + * + * A declaration is invalid if it could not be parsed successfully. + */ Perhaps add that we need to have

[PATCH] D40310: Restructure how we break tokens.

klimek added a comment. Restructured to make the invariants clearer based on a chat with Krasimir. https://reviews.llvm.org/D40310 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D40310: Restructure how we break tokens.

klimek updated this revision to Diff 124381. klimek added a comment. Restructure based on code review feedback. https://reviews.llvm.org/D40310 Files: lib/Format/BreakableToken.cpp lib/Format/BreakableToken.h lib/Format/ContinuationIndenter.cpp unittests/Format/FormatTest.cpp

[PATCH] D40310: Restructure how we break tokens.

klimek added inline comments. Comment at: lib/Format/ContinuationIndenter.cpp:1518 + unsigned RemainingTokenColumns = 0; + // The column number we're currently at. + unsigned ContentStartColumn = 0; krasimir wrote: > Could you please spell out the invariants

[PATCH] D40310: Restructure how we break tokens.

klimek marked an inline comment as done. klimek added inline comments. Comment at: lib/Format/ContinuationIndenter.cpp:1504 : Style.PenaltyBreakComment; - unsigned RemainingSpace = ColumnLimit - Current.UnbreakableTailLength; + // Stores

[PATCH] D40310: Restructure how we break tokens.

klimek updated this revision to Diff 124095. klimek added a comment. Pull out getRemainingLength. https://reviews.llvm.org/D40310 Files: lib/Format/BreakableToken.cpp lib/Format/BreakableToken.h lib/Format/ContinuationIndenter.cpp unittests/Format/FormatTest.cpp

[PATCH] D33589: clang-format: consider not splitting tokens in optimization

klimek added a comment. In https://reviews.llvm.org/D33589#933746, @Typz wrote: > with this setting, a "non wrapped" comment will actually be reflown to > ColumnLimit+10... Isn't the same true for code then, though? Generally, code will protrude by 10 columns before being broken?

[PATCH] D33589: clang-format: consider not splitting tokens in optimization

klimek added a comment. In https://reviews.llvm.org/D33589#933746, @Typz wrote: > > @klimek wrote: > > In the above example, we add 3 line breaks, and we'd add 1 (or more) > > additional line breaks when reflowing below the column limit. > > I agree that that can lead to different overall

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

klimek added a comment. In https://reviews.llvm.org/D37813#930065, @Typz wrote: > ping? Argh, very sorry for the delay in response. In https://reviews.llvm.org/D37813#905257, @Typz wrote: > In https://reviews.llvm.org/D37813#876227, @klimek wrote: > > > I think instead of introducing more

[PATCH] D40310: Restructure how we break tokens.

klimek added inline comments. Comment at: lib/Format/BreakableToken.cpp:198 + "Getting the length of a part of the string literal indicates that " + "the code tries to reflow it."); + return UnbreakableTailLength + Postfix.size() + krasimir

[PATCH] D40310: Restructure how we break tokens.

klimek updated this revision to Diff 124075. klimek marked 10 inline comments as done. klimek added a comment. Address review comments. https://reviews.llvm.org/D40310 Files: lib/Format/BreakableToken.cpp lib/Format/BreakableToken.h lib/Format/ContinuationIndenter.cpp

[PATCH] D33589: clang-format: consider not splitting tokens in optimization

klimek added a comment. In https://reviews.llvm.org/D33589#933568, @klimek wrote: > In https://reviews.llvm.org/D33589#931723, @Typz wrote: > > > In https://reviews.llvm.org/D33589#925903, @klimek wrote: > > > > > I think this patch doesn't handle a couple of cases that I'd like to see > > >

[PATCH] D40068: Implement more accurate penalty & trade-offs while breaking protruding tokens.

klimek added a comment. In https://reviews.llvm.org/D40068#931679, @Typz wrote: > Generally, this indeed improves the situation (though I cannot say much about > the code itself, it is still too subtle for my shallow knowledge of > clang-format). > > But it seems to give some strange looking

[PATCH] D33589: clang-format: consider not splitting tokens in optimization

klimek added a comment. In https://reviews.llvm.org/D33589#931802, @Typz wrote: > Btw, another issue I am having is that reflowing does not respect the > alignment. For exemple: > > enum { > Foo,///< This is a very long comment > Bar,///< This is shorter > BarBar,

[PATCH] D33589: clang-format: consider not splitting tokens in optimization

klimek added a comment. In https://reviews.llvm.org/D33589#931723, @Typz wrote: > In https://reviews.llvm.org/D33589#925903, @klimek wrote: > > > I think this patch doesn't handle a couple of cases that I'd like to see > > handled. A counter-proposal with different trade-offs is in > >

[PATCH] D40060: [clangd] Fuzzy match scorer

klimek added inline comments. Comment at: clangd/FuzzyMatch.cpp:69 +: NPat(std::min(MaxPat, Pattern.size())), NWord(0), + ScoreScale(0.5f / NPat) { + memcpy(Pat, Pattern.data(), NPat); Why .5? Comment at: clangd/FuzzyMatch.cpp:88 +

[PATCH] D40310: Restructure how we break tokens.

klimek created this revision. This fixes some bugs in the reflowing logic and splits out the concerns of reflowing from BreakableToken. Things to do after this patch: - Refactor the breakProtrudingToken function possibly into a class, so we can split it up into methods that operate on the

[PATCH] D40068: Implement more accurate penalty & trade-offs while breaking protruding tokens.

klimek updated this revision to Diff 123359. klimek added a comment. Just exporting https://reviews.llvm.org/D40068 Files: lib/Format/BreakableToken.cpp lib/Format/BreakableToken.h lib/Format/ContinuationIndenter.cpp unittests/Format/FormatTest.cpp

[PATCH] D40120: [clang-format] Add text proto filename detection

klimek accepted this revision. klimek added a comment. This revision is now accepted and ready to land. LG https://reviews.llvm.org/D40120 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D40068: Implement more accurate penalty & trade-offs while breaking protruding tokens.

klimek updated this revision to Diff 123309. klimek marked an inline comment as done. klimek added a comment. Address review comments. https://reviews.llvm.org/D40068 Files: lib/Format/BreakableToken.cpp lib/Format/BreakableToken.h lib/Format/ContinuationIndenter.cpp

[PATCH] D40068: Implement more accurate penalty & trade-offs while breaking protruding tokens.

klimek added inline comments. Comment at: unittests/Format/FormatTest.cpp:9951 + "\n*/", + Style)); + krasimir wrote: > Why didn't this get reformatted as: > ``` > EXPECT_EQ("int a; /* first line\n" > "*

[PATCH] D39842: Allow to store precompiled preambles in memory.

klimek added inline comments. Comment at: lib/Frontend/PrecompiledPreamble.cpp:44 +SmallString<64> Path; +llvm::sys::path::system_temp_directory(/*erasedOnReboot=*/true, Path); +llvm::sys::path::append(Path, "___clang_inmemory_preamble___");

[PATCH] D40120: [clang-format] Add text proto filename detection

klimek added a comment. Should that be configured? METADATA seems pretty domain specific ;) https://reviews.llvm.org/D40120 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D39842: Allow to store precompiled preambles in memory.

klimek added inline comments. Comment at: lib/Frontend/PrecompiledPreamble.cpp:44 +SmallString<64> Path; +llvm::sys::path::system_temp_directory(/*erasedOnReboot=*/true, Path); +llvm::sys::path::append(Path, "___clang_inmemory_preamble___");

[PATCH] D40068: Implement more accurate penalty & trade-offs while breaking protruding tokens.

klimek updated this revision to Diff 123139. klimek marked 4 inline comments as done. klimek added a comment. Address review comments. https://reviews.llvm.org/D40068 Files: lib/Format/BreakableToken.cpp lib/Format/BreakableToken.h lib/Format/ContinuationIndenter.cpp

[PATCH] D40068: Implement more accurate penalty & trade-offs while breaking protruding tokens.

klimek added inline comments. Comment at: unittests/Format/FormatTest.cpp:8007 +"\"aaabbbcc ddde \"\n" +"\"efff\");", format("someFunction(\"aaabbbcc ddde efff\");", krasimir wrote: > Why did the string got on a

[PATCH] D39842: Allow to store precompiled preambles in memory.

klimek accepted this revision. klimek added a comment. This revision is now accepted and ready to land. LG Comment at: lib/Frontend/PrecompiledPreamble.cpp:490 PreprocessorOpts.DisablePCHValidation = true; + if (Storage.getKind() == PCHStorage::Kind::TempFile) { +const

[PATCH] D39842: Allow to store precompiled preambles in memory.

klimek added inline comments. Comment at: lib/Frontend/PrecompiledPreamble.cpp:206 + std::unique_ptr Storage; + if (InMemStorage) { +OS = llvm::make_unique(*InMemStorage); ilya-biryukov wrote: > klimek wrote: > > It looks like we should pass in the output

[PATCH] D33589: clang-format: consider not splitting tokens in optimization

klimek added a comment. I think this patch doesn't handle a couple of cases that I'd like to see handled. A counter-proposal with different trade-offs is in https://reviews.llvm.org/D40068. https://reviews.llvm.org/D33589 ___ cfe-commits mailing

<    1   2   3   4   5   6   >