[PATCH] D146310: [clang-format] Fix dropped 'else' in 398cddf6acec

klimek added a comment. Submitted. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146310/new/ https://reviews.llvm.org/D146310 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D146310: [clang-format] Fix dropped 'else' in 398cddf6acec

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG9e9e096ae95b: [clang-format] Fix dropped else. (authored by klimek). Herald added reviewers: rymiel, HazardyKnusperkeks, owenpan, MyDeveloperDay.

[PATCH] D147176: [clang-format] NFC ensure Style operator== remains sorted for ease of editing

klimek added a comment. And thanks a lot for cleaning up, really appreciate it! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D147176/new/ https://reviews.llvm.org/D147176 ___ cfe-commits mailing list

[PATCH] D147176: [clang-format] NFC ensure Style operator== remains sorted for ease of editing

klimek added a comment. I apologize for not tagging appropriately - is that process documented somewhere? Herald added a comment. NOTE: Clang-Format Team Automated Review Comment It looks like your clang-format review does not contain any unit tests, please try to ensure all code changes

[PATCH] D146310: [clang-format] Fix dropped 'else' in 398cddf6acec

klimek added a comment. Thanks, yes, I did not intend to delete the else. This only triggers with fuzzing with rather involved inputs, thus I wasn't able to create a nice enough unit test. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146310/new/

[PATCH] D146704: [clang-format] NFC Format.h and ClangFormatStyleOptions.rst are out of date

klimek accepted this revision. klimek added a comment. This revision is now accepted and ready to land. Thank you!! Sorry for forgetting that I needed to do this, CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146704/new/ https://reviews.llvm.org/D146704

[PATCH] D145244: [clang-format] Add ability to trace tokens.

klimek added a comment. Answering the fundamental design question first. Comment at: clang/lib/Format/ContinuationIndenter.cpp:289 Current.closesBlockOrBlockTypeList(Style))) { +DEBUG_FORMAT_TRACE_TOKEN(Current, "!canBreak");

[PATCH] D145244: [clang-format] Add ability to trace tokens.

klimek updated this revision to Diff 502123. klimek added a comment. Remove superfluous include. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D145244/new/ https://reviews.llvm.org/D145244 Files: clang/lib/Format/CMakeLists.txt

[PATCH] D145244: [clang-format] Add ability to trace tokens.

klimek created this revision. klimek added a reviewer: sammccall. Herald added a project: All. klimek requested review of this revision. Herald added a project: clang. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D145244 Files: clang/lib/Format/CMakeLists.txt

[PATCH] D144170: [clang-format] Add simple macro replacements in formatting.

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG01402831aaae: [clang-format] Add simple macro replacements in formatting. (authored by klimek). Repository: rG LLVM Github Monorepo CHANGES

[PATCH] D144170: [clang-format] Add simple macro replacements in formatting.

klimek added inline comments. Comment at: clang/lib/Format/UnwrappedLineParser.cpp:4592 + !Macros.hasArity(ID->TokenText, Args->size())) { +// The macro is overloaded to be both object-like and function-like, +// but none of the function-like arities

[PATCH] D144170: [clang-format] Add simple macro replacements in formatting.

klimek updated this revision to Diff 500191. klimek added a comment. Address review comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D144170/new/ https://reviews.llvm.org/D144170 Files: clang/include/clang/Format/Format.h

[PATCH] D144170: [clang-format] Add simple macro replacements in formatting.

klimek added inline comments. Comment at: clang/lib/Format/MacroExpander.cpp:172 assert(defined(ID->TokenText)); + assert( + (!OptionalArgs && ObjectLike.find(ID->TokenText) != ObjectLike.end()) || sammccall wrote: > this assertion failure isn't going

[PATCH] D144170: [clang-format] Add simple macro replacements in formatting.

klimek updated this revision to Diff 500181. klimek marked 5 inline comments as done. klimek added a comment. Address review comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D144170/new/ https://reviews.llvm.org/D144170 Files:

[PATCH] D144170: [clang-format] Add simple macro replacements in formatting.

klimek added inline comments. Comment at: clang/unittests/Format/FormatTest.cpp:22584 + Style.Macros.push_back("CALL(x)=f([] { x })"); + Style.Macros.push_back("ASSIGN_OR_RETURN(a, b, c)=a = (b) || (c)"); + sammccall wrote: > klimek wrote: > > sammccall wrote:

[PATCH] D144170: [clang-format] Add simple macro replacements in formatting.

klimek updated this revision to Diff 500121. klimek marked 6 inline comments as done. klimek added a comment. Address review comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D144170/new/ https://reviews.llvm.org/D144170 Files:

[PATCH] D144170: [clang-format] Add simple macro replacements in formatting.

klimek updated this revision to Diff 499777. klimek added a comment. Add comment. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D144170/new/ https://reviews.llvm.org/D144170 Files: clang/include/clang/Format/Format.h

[PATCH] D144170: [clang-format] Add simple macro replacements in formatting.

klimek added inline comments. Comment at: clang/lib/Format/ContinuationIndenter.cpp:744 + // Align following lines within parenthesis / brackets if configured. + // For a line of macro parents, the commas that follow the opening parenthesis + // in the line come after the

[PATCH] D144170: [clang-format] Add simple macro replacements in formatting.

klimek updated this revision to Diff 499771. klimek marked 9 inline comments as done. klimek added a comment. Address reviewer comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D144170/new/ https://reviews.llvm.org/D144170 Files:

[PATCH] D144170: [clang-format] Add simple macro replacements in formatting.

klimek created this revision. klimek added a reviewer: sammccall. Herald added a project: All. klimek requested review of this revision. Herald added a project: clang. Add configuration to specify macros. Macros will be expanded, and the code will be parsed and annotated in the expanded state. In

[PATCH] D143070: [clang-format] Enable FormatTokenSource to insert tokens.

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG1995d4424505: [clang-format] Enable FormatTokenSource to insert tokens. (authored by klimek). Repository: rG LLVM Github Monorepo CHANGES SINCE

[PATCH] D143070: [clang-format] Enable FormatTokenSource to insert tokens.

klimek added inline comments. Comment at: clang/lib/Format/UnwrappedLineParser.h:287 // owned outside of and handed into the UnwrappedLineParser. + // FIXME: The above fixme doesn't work if we need to create tokens while + // parsing. sammccall wrote: > I'm

[PATCH] D143070: [clang-format] Enable FormatTokenSource to insert tokens.

klimek updated this revision to Diff 497637. klimek marked an inline comment as done. klimek added a comment. Address review comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143070/new/ https://reviews.llvm.org/D143070 Files:

[PATCH] D143070: [clang-format] Enable FormatTokenSource to insert tokens.

klimek updated this revision to Diff 497416. klimek added a comment. Undo changes to ownership of initial set of FormatTokens. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143070/new/ https://reviews.llvm.org/D143070 Files:

[PATCH] D143070: [clang-format] Enable FormatTokenSource to insert tokens.

klimek added inline comments. Comment at: clang/lib/Format/FormatTokenSource.h:74 public: - IndexedTokenSource(ArrayRef Tokens) + IndexedTokenSource(SmallVectorImpl ) : Tokens(Tokens), Position(-1) {} sammccall wrote: > As I understand it, this

[PATCH] D143070: [clang-format] Enable FormatTokenSource to insert tokens.

klimek updated this revision to Diff 497415. klimek marked 3 inline comments as done. klimek added a comment. Address reviewer comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143070/new/ https://reviews.llvm.org/D143070 Files:

[PATCH] D143070: [clang-format] Enable FormatTokenSource to insert tokens.

klimek created this revision. klimek added a reviewer: sammccall. Herald added a project: All. klimek requested review of this revision. Herald added a project: clang. In preparation for configured macro replacements in formatting, add the ability to insert tokens to FormatTokenSource, and

[PATCH] D119138: [clang-format] Further improve support for requires expressions

klimek added a comment. For non-functional clean-ups generally llvm doesn't require pre-commit review - I did communicate here so people involved in the original change wouldn't miss the clean-up. I do agree that what commits to pre-review is a fine line, and usually try to err on the side of

[PATCH] D119138: [clang-format] Further improve support for requires expressions

klimek added a comment. I changed it in 49aca00d63e14df8bc68fc4329e6cbc9c9805eb8 . "We" is the people working on clang-format :) I hope that we have a common goal of making clang-format as easy to maintain as we can. FWIW, I

[PATCH] D119138: [clang-format] Further improve support for requires expressions

klimek added a comment. Generally, why do we need to have that much information? I.e. why do we need to know the exact type of the "requires" keyword? I do understand we need to know the brace type, but that seems like it would be easier to figure out in the TokenAnnotator (where we already

[PATCH] D119138: [clang-format] Further improve support for requires expressions

klimek added a comment. Herald added a project: All. Hey ho, sorry for the late comment here, but adding peekNextToken(n) is problematic, as this gets in the way of future changes we want to do to handle macros better. Usually we want to use X = Tokens->getPosition() and FormatTok =

[PATCH] D88299: [clang-format] Add MacroUnexpander.

klimek added a comment. In D88299#3804910 , @owenpan wrote: > @sstwcw thanks for pointing it out. See D134329 > . Thanks Owen, really appreciate this! And sorry for not getting to it yet myself :( Repository: rG

[PATCH] D88299: [clang-format] Add MacroUnexpander.

klimek added a comment. In D88299#3660779 , @nridge wrote: > Thanks! (I was intrigued by Sam's "solves a whole class of clang-format's > biggest problems" comment :-)) The end-result hopefully will :) Repository: rG LLVM Github Monorepo CHANGES

[PATCH] D88299: [clang-format] Add MacroUnexpander.

klimek added a comment. In D88299#3660772 , @nridge wrote: > Does this patch change the formatting behaviour of clang-format? > > If so, are there any test cases that show before/after formatting? The > MacroCallReconstructorTest unit test seems like

[PATCH] D88299: [clang-format] Add MacroUnexpander.

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGd6d0dc1f4537: [clang-format] Add MacroUnexpander. (authored by klimek). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D88299: [clang-format] Add MacroUnexpander.

klimek updated this revision to Diff 443729. klimek added a comment. Address review comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D88299/new/ https://reviews.llvm.org/D88299 Files: clang/lib/Format/CMakeLists.txt

[PATCH] D88299: [clang-format] Add MacroUnexpander.

klimek marked 7 inline comments as done. klimek added inline comments. Comment at: clang/lib/Format/Macros.h:201 + /// Generally, this line tries to have the same structure as the expanded, + /// formatted unwrapped lines handed in via \c addLine(), with the exception + ///

[PATCH] D88299: [clang-format] Add MacroUnexpander.

klimek added a comment. Noticed I should have waiting with the renaming of the file until the review is done :( Sorry for the extra confusion. Comment at: clang/lib/Format/MacroUnexpander.cpp:77 + // stream, we need to continue the unexpansion until we find the right token +

[PATCH] D88299: [clang-format] Add MacroUnexpander.

klimek updated this revision to Diff 390057. klimek marked 49 inline comments as done. klimek added a comment. Work in review comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D88299/new/ https://reviews.llvm.org/D88299 Files:

[PATCH] D114430: [clang-format] NFC - recent changes caused clang-format to no longer be clang-formatted.

klimek added a comment. Thanks for cleaning up after me, and sorry for the mess - do y'all have clang-format set up as a presubmit or do you just remember to format manually? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D114430/new/

[PATCH] D69764: [clang-format] Add East/West Const fixer capability

klimek added a comment. In D69764#2934686 , @aaron.ballman wrote: > In D69764#2934612 , @MyDeveloperDay > wrote: > >>> I was referring to @rsmith and @aaron.ballman (to clarify), both are >>> maintainers in

[PATCH] D69764: [clang-format] Add East/West Const fixer capability

klimek added a comment. In D69764#2934032 , @MyDeveloperDay wrote: > In D69764#2932929 , @steveire wrote: > >> > > @steveire, thanks for your comments, I also agree that a second tool > shouldn't be needed,

[PATCH] D96114: [ASTMatchers] Fix parent-child traversal between functions and parms

klimek added a comment. Can you explain the change and the before/after a bit more? Thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D96114/new/ https://reviews.llvm.org/D96114 ___ cfe-commits

[PATCH] D91996: [clang-format] Remove double trim

klimek added a comment. Isn't the comment incorrect after this patch? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91996/new/ https://reviews.llvm.org/D91996 ___ cfe-commits mailing list

[PATCH] D88299: [clang-format] Add MacroUnexpander.

klimek added inline comments. Comment at: clang/lib/Format/FormatToken.h:449 + /// When macro expansion introduces parents, those are marked as + /// \c MacroParent, so formatting knows their children need to be formatted. sammccall wrote: > I can't really

[PATCH] D88299: [clang-format] Add MacroUnexpander.

klimek updated this revision to Diff 301253. klimek marked 3 inline comments as done. klimek added a comment. Adapted based on review comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D88299/new/ https://reviews.llvm.org/D88299 Files:

[PATCH] D89920: Export TemplateArgumentMatcher so clients defining custom matchers don't need to use the internal namespace

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

[PATCH] D88952: [clang-format][tests] Fix MacroExpander lexer not parsing C++ keywords

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

[PATCH] D88956: [clang-format] Fix misformatted macro definitions after D86959

klimek added inline comments. Comment at: clang/unittests/Format/FormatTest.cpp:8333 +TEST_F(FormatTest, UnderstandsUsesOfStarAndAmpInMacroDefinition) { + // This is a regression test for mis-parsing the & after decltype as a binary arichardson wrote: >

[PATCH] D88956: [clang-format] Fix misformatted macro definitions after D86959

klimek added inline comments. Comment at: clang/unittests/Format/FormatTest.cpp:8333 +TEST_F(FormatTest, UnderstandsUsesOfStarAndAmpInMacroDefinition) { + // This is a regression test for mis-parsing the & after decltype as a binary I'd put this into a new

[PATCH] D83296: [clang-format] Add a MacroExpander.

klimek added a comment. In D83296#2299062 , @nridge wrote: > What does this change mean for users of clang-format -- better formatting of > complicated (e.g. multi-line) macro invocations? In addition to what Sam said, this also attempts to be an

[PATCH] D84306: [clang-format][NFC] Be more careful about the layout of FormatToken.

klimek added a comment. In D84306#2294952 , @MyDeveloperDay wrote: > Which bit do you find more complex? adding something to the FormatToken or > the use of the `is()` calls? I think mainly that a) the language doesn't support bitfields well yet, as

[PATCH] D88299: [clang-format] Add MacroUnexpander.

klimek created this revision. klimek added a reviewer: sammccall. Herald added subscribers: cfe-commits, mgorny. Herald added a project: clang. klimek requested review of this revision. MacroUnexpander applies the structural formatting of expanded lines into UnwrappedLines to the corresponding

[PATCH] D83296: [clang-format] Add a MacroExpander.

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. klimek marked 5 inline comments as done. Closed by commit rGe336b74c995d: [clang-format] Add a MacroExpander. (authored by klimek). Changed prior to commit:

[PATCH] D84306: [clang-format][NFC] Be more careful about the layout of FormatToken.

klimek added a comment. FWIW, finding this due to seeing the added complexity. Do you have data on what the difference is on clang-format for either overall memory use or performance? Thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D83296: [clang-format] Add a MacroExpander.

klimek updated this revision to Diff 292970. klimek marked 7 inline comments as done. klimek added a comment. Worked in review comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83296/new/ https://reviews.llvm.org/D83296 Files:

[PATCH] D83296: [clang-format] Add a MacroExpander.

klimek added inline comments. Comment at: clang/lib/Format/FormatToken.h:177 +/// EndOfExpansion: 0 0 0 21 0 1 +struct MacroContext { + MacroContext(MacroRole Role) : Role(Role) {} sammccall wrote: > "context" is often pretty vague - "MacroSource"

[PATCH] D83296: [clang-format] Add a MacroExpander.

klimek added inline comments. Comment at: clang/lib/Format/MacroExpander.cpp:186 +Tok->MacroCtx = MacroContext(MR_ExpandedArg); + pushToken(Tok); +} sammccall wrote: > klimek wrote: > > sammccall wrote: > > > you're pushing here without copying.

[PATCH] D83296: [clang-format] Add a MacroExpander.

klimek added inline comments. Comment at: clang/lib/Format/MacroExpander.cpp:186 +Tok->MacroCtx = MacroContext(MR_ExpandedArg); + pushToken(Tok); +} sammccall wrote: > you're pushing here without copying. This means the original tokens from the

[PATCH] D83296: [clang-format] Add a MacroExpander.

klimek added inline comments. Comment at: clang/lib/Format/MacroExpander.cpp:35 + SmallVector Params; + SmallVector Tokens; +}; sammccall wrote: > Tokens -> Expansion? (semantics rather than type) Changed to "Body". Comment at:

[PATCH] D83296: [clang-format] Add a MacroExpander.

klimek updated this revision to Diff 281611. klimek marked 27 inline comments as done. klimek added a comment. Addressed code review comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83296/new/ https://reviews.llvm.org/D83296 Files:

[PATCH] D83296: [clang-format] Add a MacroExpander.

klimek added a comment. In D83296#2146970 , @sammccall wrote: > In D83296#2146870 , @klimek wrote: > > > Monday-morning ping. > > > Thanks for the reminder here... however this is taking me a bit to get my > head

[PATCH] D83296: [clang-format] Add a MacroExpander.

klimek added a comment. Monday-morning ping. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83296/new/ https://reviews.llvm.org/D83296 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D83621: [clang][Tooling] Try to avoid file system access if there is no record for the file in compile_commads.json

klimek added a subscriber: djasper. klimek added a comment. @djasper wrote this iirc, but I doubt he'll have much time to look into this :) IIRC the symlink checking was there because some part of the system on linux tends to give us realpaths (with CMake builds), and that leads to not finding

[PATCH] D79773: [clang-format] Improve clang-formats handling of concepts

klimek added inline comments. Comment at: clang/lib/Format/UnwrappedLineParser.cpp:631-636 + else if (FormatTok->is(tok::arrow)) { +// Following the } we can find a trailing return type arrow +// as part of an implicit conversion constraint. +nextToken(); +

[PATCH] D83296: [clang-format] Add a MacroExpander.

klimek created this revision. klimek added a reviewer: sammccall. Herald added subscribers: cfe-commits, mgorny. Herald added a project: clang. The MacroExpander allows to expand simple (non-resursive) macro definitions from a macro identifier token and macro arguments. It annotates the tokens

[PATCH] D83218: Hand Allocator and IdentifierTable into FormatTokenLexer.

This revision was automatically updated to reflect the committed changes. Closed by commit rG8c2a61397607: Hand Allocator and IdentifierTable into FormatTokenLexer. (authored by klimek). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83218/new/

[PATCH] D83218: Hand Allocator and IdentifierTable into FormatTokenLexer.

klimek updated this revision to Diff 275964. klimek added a comment. Address review comment. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83218/new/ https://reviews.llvm.org/D83218 Files: clang/lib/Format/FormatTokenLexer.cpp

[PATCH] D83218: Hand Allocator and IdentifierTable into FormatTokenLexer.

klimek created this revision. klimek added a reviewer: sammccall. Herald added a project: clang. Herald added a subscriber: cfe-commits. This allows us to share the allocator in the future so we can create tokens while parsing. Repository: rG LLVM Github Monorepo

[PATCH] D83076: Revert AST Matchers default to AsIs mode

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

[PATCH] D82771: [ASTMatcher] Fix a performance regression: memorize the child match.

klimek accepted this revision. klimek added a comment. This revision is now accepted and ready to land. LG Comment at: clang/lib/ASTMatchers/ASTMatchFinder.cpp:468 +// Memoize result even doing a single-level match, it might be expensive. +Key.Type = MaxDepth == 1 ?

[PATCH] D82771: [ASTMatcher] Fix a performance regression: memorize the child match.

klimek added a comment. In D82771#2121924 , @hokein wrote: > In D82771#2120214 , @klimek wrote: > > > In what situation are we calling child matchers repeatedly with the same > > matcher on the same node? > > > I

[PATCH] D82771: [ASTMatcher] Fix a performance regression: memorize the child match.

klimek added inline comments. Comment at: clang/lib/ASTMatchers/ASTMatchFinder.cpp:467 Key.Traversal = Ctx.getParentMapContext().getTraversalKind(); -Key.Type = MatchType::Descendants; +// Memorize result even doing a single-level match, it might be expensive. +

[PATCH] D82771: [ASTMatcher] Fix a performance regression: memorize the child match.

klimek added a comment. In what situation are we calling child matchers repeatedly with the same matcher on the same node? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D82771/new/ https://reviews.llvm.org/D82771

[PATCH] D80202: [ASTMatchers] Performance optimization for memoization

klimek added a reviewer: sammccall. klimek added a comment. +Sam for additional sanity checking. Comment at: clang/lib/ASTMatchers/ASTMatchFinder.cpp:902 // Maps (matcher, node) -> the match result for memoization. - typedef std::map MemoizationMap; + typedef std::map>

[PATCH] D80961: Ignore template instantiations if not in AsIs mode

klimek added a comment. In D80961#2099262 , @steveire wrote: > In D80961#2095254 , @klimek wrote: > > > 1. the scare quotes around "standing objections" reads like you're not > > respecting the opinions of others

[PATCH] D80961: Ignore template instantiations if not in AsIs mode

klimek added a comment. In D80961#2090216 , @steveire wrote: > In D80961#2079419 , @aaron.ballman > wrote: > > > In D80961#2079044 , @klimek wrote: > > > > > In

[PATCH] D80025: [ASTMatcher] Correct memoization bug ignoring direction (descendants or ancestors)

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

[PATCH] D81552: [ASTMatchers] Added hasDirectBase and hasClass Matchers

klimek added a comment. We have some precedent for overloading has* matchers, but I'll defer to Sam's judgement whether that's a good idea here. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D81552/new/ https://reviews.llvm.org/D81552

[PATCH] D80961: Ignore template instantiations if not in AsIs mode

klimek added a comment. In D80961#2076242 , @aaron.ballman wrote: > In D80961#2073049 , @klimek wrote: > > > Without jumping into the discussion whether it should be the default, I > > think we should be able to

[PATCH] D80961: WIP: Ignore template instantiations if not in AsIs mode

klimek added a comment. Without jumping into the discussion whether it should be the default, I think we should be able to control template instantiation visitation separately from other implicit nodes. Having to put AsIs on a matcher every time you need to match template instantiations is a

[PATCH] D80961: WIP: Ignore template instantiations if not in AsIs mode

klimek added a comment. In D80961#2068865 , @ymandel wrote: > Thank you for bringing up this issue. I think it highlights an underlying > problem -- editing templates is quite difficult -- that neither setting will > address, as Dmitri expanded on

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

klimek added a comment. For the policy question: clang-format does intentionally not try to be stable - the "how to" suggestion for clang-format has always been to format changes lines and live with divergence (divergence from people manually formatting things is larger). Repository: rG

[PATCH] D69764: [clang-format] Add East/West Const fixer capability

klimek added inline comments. Comment at: clang/docs/ClangFormatStyleOptions.rst:1378 +**ConstStyle** (``ConstAlignmentStyle``) + Different ways to arrange const. MyDeveloperDay wrote: > aaron.ballman wrote: > > MyDeveloperDay wrote: > > > klimek wrote: > > >

[PATCH] D80025: [ASTMatcher] Correct memoization bug ignoring direction (descendants or ancestors)

klimek added inline comments. Comment at: clang/lib/ASTMatchers/ASTMatchFinder.cpp:46-49 +enum class MatchDirection { + Ancestors, + Descendants +}; loic-joly-sonarsource wrote: > klimek wrote: > > loic-joly-sonarsource wrote: > > > klimek wrote: > > > > Nice

[PATCH] D80202: [ASTMatchers] Performance optimization for memoization

klimek added a comment. In D80202#2046321 , @njames93 wrote: > In D80202#2046266 , @klimek wrote: > > > Given the extra complexity I'd like to see that it matters - bound nodes > > tend to be small. > > > I put

[PATCH] D80202: [ASTMatchers] Performance optimization for memoization

klimek added a comment. Given the extra complexity I'd like to see that it matters - bound nodes tend to be small. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80202/new/ https://reviews.llvm.org/D80202

[PATCH] D80025: [ASTMatcher] Correct memoization bug ignoring direction (descendants or ancestors)

klimek added inline comments. Comment at: clang/lib/ASTMatchers/ASTMatchFinder.cpp:46-49 +enum class MatchDirection { + Ancestors, + Descendants +}; loic-joly-sonarsource wrote: > klimek wrote: > > Nice find! Why don't we need more states though? > > 1.

[PATCH] D80025: [ASTMatcher] Correct memoization bug ignoring direction (descendants or ancestors)

klimek added inline comments. Comment at: clang/lib/ASTMatchers/ASTMatchFinder.cpp:46-49 +enum class MatchDirection { + Ancestors, + Descendants +}; Nice find! Why don't we need more states though? 1. wouldn't hasParent() followed by a hasAncestor() also

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

klimek updated this revision to Diff 263712. klimek added a comment. Update docs. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67405/new/ https://reviews.llvm.org/D67405 Files: clang/lib/Format/Format.cpp clang/lib/Format/FormatToken.cpp

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

klimek updated this revision to Diff 263707. klimek added a comment. Rebase. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67405/new/ https://reviews.llvm.org/D67405 Files: clang/lib/Format/Format.cpp clang/lib/Format/FormatToken.cpp

[PATCH] D73537: [clangd] add CODE_OWNERS

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

[PATCH] D69764: [clang-format] Add Left/Right Const (East/West , Before/After) fixer capability

klimek added a comment. I'm not generally opposed to this, given that a) clang-format already changes code; I think by now we're not fixing double semicolon mainly for workflow reasons, would be fine to add b) the implementation is very self contained Comment at:

[PATCH] D70664: update string comparison in clang-format.py

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

[PATCH] D70633: clang-format-vs : Fix Unicode formatting

klimek added a comment. generally makes sense Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70633/new/ https://reviews.llvm.org/D70633 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D69577: [clang-format] [PR35518] C++17 deduction guides are wrongly formatted

klimek added inline comments. Comment at: clang/lib/Format/TokenAnnotator.cpp:1400-1403 +} else if (Current.Previous && Current.Previous->is(tok::r_paren) && + Current.startsSequence(tok::arrow, tok::identifier, tok::less)) { + // Deduction guides

[PATCH] D69122: Add support to find out resource dir and add it as compilation args

klimek added a comment. > since resource directory should be picked relative > to the path of the clang-compiler in the compilation command. The resource dir should be the resource dir that shipped with the clang source code that the *tool* was built with. We can think about the resource dir

[PATCH] D69351: Improve Clang's getting involved document and make it more inclusive in wording.

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

[PATCH] D69351: Improve Clang's getting involved document and make it more inclusive in wording.

klimek added inline comments. Comment at: clang/www/get_involved.html:94 + + A complete specification: The specification must be sufficient to + understand the design of the feature as well as interpret the meaning of Remove "complete" - the explanation

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

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:356 + + StringRef Line(StartBuf, (EndBuf - StartBuf) - 1); + - 1 is to exclude the \n I'd

  1   2   3   4   5   6   >