[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] 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] 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 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] 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 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 a comment. In D68914#1709002 , @owenpan wrote: > Here are some `const StringRef &` examples > . StringRef already is a const pointe

[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] 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 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 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 assume

[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 afterwa

[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 ___ c

[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 as

[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] 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 tha

[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 f

[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] 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 LLV

[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: clang/

[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: clang/lib/Form

[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 above.

[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 ra

[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 c

[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] 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 cfe-co

[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 D80961#20762

[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 h

[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 ___ c

[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 https://reviews.llvm.org/D83

[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 clang/lib/Format

[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/ https

[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 wit

[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(); +parseS

[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] 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 https://lis

[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 a

[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> Mem

[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] 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. +K

[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 g

[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 ? Mat

[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 clang/l

[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 cl

[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 trigge

[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. wouldn'

[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] 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 > > https://

[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] 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 re

[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 > > > han

[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 wrote

[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 unittests/Format/F

[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 and

[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 outc

[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? https://re

[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 unittests/Format/Forma

[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 whethe

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

klimek accepted this revision. klimek added a comment. Self-accepting and closing, as the underlying change has landed, and this diff is incorrect now. I also have an idea to solve the problem Typz has brought up. https://reviews.llvm.org/D40068 __

[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 t

[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 unittest

[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] 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 id

[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 unittests/Format/Fo

[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 th

[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 unittests/Format/Fo

[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 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 swit

[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 cfe/trunk/lib/Format/BreakableToke

[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 foo

[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 wa

[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 unittests/Format/FormatT

[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 Ind

[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: cfe/trunk/lib/Format/Continuatio

[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 char

[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#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#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 'op

[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 config

[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 concern

[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 wrot

[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 (impleme

[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 Revision(co

[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 http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-com

[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 http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-com

[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 + /// th

[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 clang/lib/Format/Form

[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 https://r

[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 it's

[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 SINC

[PATCH] D27104: Unify and simplify the behavior of the hasDeclaration matcher.

klimek updated this revision to Diff 108608. klimek added a comment. Update to handle hasDeclaration for type alias template decls. https://reviews.llvm.org/D27104 Files: include/clang/ASTMatchers/ASTMatchers.h include/clang/ASTMatchers/ASTMatchersInternal.h unittests/AST/ASTImporterTest.

[PATCH] D27104: Unify and simplify the behavior of the hasDeclaration matcher.

klimek added a reviewer: bkramer. klimek added a comment. Adding Ben as reviewer. https://reviews.llvm.org/D27104 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D35012: [refactor] Add the AST source selection component

klimek added a comment. Apart from nits, looks OK to me - Eric, are all your concerns addressed? Comment at: include/clang/Tooling/Refactoring/ASTSelection.h:51-53 + ast_type_traits::DynTypedNode Node; + SourceSelectionKind SelectionKind; + std::vector Children;

[PATCH] D36154: Adapt clang-tidy checks to changing semantics of hasDeclaration.

klimek created this revision. Herald added a subscriber: JDevlieghere. https://reviews.llvm.org/D36154 Files: clang-tidy/google/StringReferenceMemberCheck.cpp clang-tidy/misc/DanglingHandleCheck.cpp clang-tidy/misc/InaccurateEraseCheck.cpp clang-tidy/misc/UseAfterMoveCheck.cpp clang-tid

[PATCH] D36155: Use VFS operations in FileManager::makeAbsolutePath.

klimek accepted this revision. klimek added a comment. This revision is now accepted and ready to land. Nice catch! lg https://reviews.llvm.org/D36155 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/list

[PATCH] D36154: Adapt clang-tidy checks to changing semantics of hasDeclaration.

klimek updated this revision to Diff 109287. klimek marked an inline comment as done. klimek added a comment. Address review comment. https://reviews.llvm.org/D36154 Files: clang-tidy/google/StringReferenceMemberCheck.cpp clang-tidy/misc/DanglingHandleCheck.cpp clang-tidy/misc/InaccurateE

[PATCH] D36154: Adapt clang-tidy checks to changing semantics of hasDeclaration.

klimek added a comment. ptal https://reviews.llvm.org/D36154 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D36156: [rename] Introduce symbol occurrences

klimek added inline comments. Comment at: include/clang/Tooling/Refactoring/Rename/SymbolOccurrences.h:32 +/// a macro expansion. +class SymbolOccurrence { +public: I understand the exact vs. non-exact idea, but can you expand a bit on how Obj-C code looks where

[PATCH] D27104: Unify and simplify the behavior of the hasDeclaration matcher.

This revision was automatically updated to reflect the committed changes. Closed by commit rL309809: Unify and simplify the behavior of the hasDeclaration matcher. (authored by klimek). Changed prior to commit: https://reviews.llvm.org/D27104?vs=108608&id=109325#toc Repository: rL LLVM http

[PATCH] D36184: [clang-diff] Filter AST nodes

klimek added a comment. Just as a general note: adding cfe-commits after the fact is usually not a good idea, as we lose the history of the review in the email list (which is the source of truth). https://reviews.llvm.org/D36184 ___ cfe-commits ma

<    1   2   3   4   5   6   >