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

klimek added inline comments. Comment at: lib/Tooling/Refactoring/ASTSelection.cpp:164 + unsigned NumMatches = 0; + for (Decl *D : Context.getTranslationUnitDecl()->decls()) { +if (ObjCImplEndLoc.isValid() && arphaman wrote: > klimek wrote: > > arphaman wro

[PATCH] D34440: [Clang] Expand response files before loading compilation database

klimek added a comment. In https://reviews.llvm.org/D34440#812938, @vladimir.plyashkun wrote: > > If you want one unified format, the compilation database is it. > > Is the `clang-tidy ... -- ` meant to be more or less a drop-in > replacement for `clang ` (arguments-wise)? > If yes, this expan

[PATCH] D35783: Ignore shadowing for declarations coming from within macros.

klimek added a comment. Your patch description sounds like you're only switching the warning off when the scope ends in a macro, but the patch looks different. For example: #define M int x int x; void f() { M; x = 42; // the user probably meant ::x here } would also be suppre

[PATCH] D35783: Ignore shadowing for declarations coming from within macros.

klimek added a comment. Could we perhaps only suppress the warning when we shadow a variable within the same declcontext? Generally, with macros, shadowing local variables is more surprising. https://reviews.llvm.org/D35783 ___ cfe-commits mailing

[PATCH] D35794: [clang-format] Fix comment levels between '} else {' and PPDirective.

klimek accepted this revision. klimek added a comment. This revision is now accepted and ready to land. lg Comment at: lib/Format/UnwrappedLineParser.h:126 bool eof() const; - void nextToken(); + // LevelDifference is the difference of levels after and before this token. +

[PATCH] D35828: Fix incorrect use of current directory to find moved paths in ASTReader.

klimek created this revision. CurrentDir was set as the path of the current module, but that can change as part of a chain of loaded modules. When we try to locate a file mentioned in a module that does not exist, we use a heuristic to look at the relative path between the original location of th

[PATCH] D35828: Fix incorrect use of current directory to find moved paths in ASTReader.

This revision was automatically updated to reflect the committed changes. Closed by commit rL308962: Fix incorrect use of current directory to find moved paths in ASTReader. (authored by klimek). Repository: rL LLVM https://reviews.llvm.org/D35828 Files: cfe/trunk/include/clang/Serializatio

[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

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

This revision was automatically updated to reflect the committed changes. Closed by commit rL309810: Adapt clang-tidy checks to changing semantics of hasDeclaration. (authored by klimek). Repository: rL LLVM https://reviews.llvm.org/D36154 Files: clang-tools-extra/trunk/clang-tidy/google/St

[PATCH] D36261: [clangd] Use multiple working threads in clangd.

klimek added inline comments. Comment at: clangd/ClangdServer.h:105 +/// A helper class to pass concurrency parameters to ClangdScheduler. +class SchedulingParams { +public: I think calling it "Options" is more idiomatic. Comment at: clangd/Cla

[PATCH] D36308: Add handling for DeducedType to HasDeclarationMatcher

klimek accepted this revision. klimek added a comment. This revision is now accepted and ready to land. Thanks! LG after comment change. Also, should we add some tests to clang-tidy? :) Comment at: include/clang/ASTMatchers/ASTMatchersInternal.h:745 + +// For deduced types

[PATCH] D35355: Fix templated type alias completion when using global completion cache

klimek added a comment. Sorry for missing this - can you add a test? https://reviews.llvm.org/D35355 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D36398: [clangd] Check if CompileCommand has changed on forceReparse.

klimek added inline comments. Comment at: clangd/ClangdUnitStore.cpp:45 + .first; +Result.RemovedFile = nullptr; + } else if (!compileCommandsAreEqual(It->second->getCompileCommand(), Just say RemovedFile = nullptr in the struct? ==

[PATCH] D36397: [clangd] Fixed a data race.

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

[PATCH] D36397: [clangd] Fixed a data race.

klimek added a comment. In https://reviews.llvm.org/D36397#833890, @ilya-biryukov wrote: > In https://reviews.llvm.org/D36397#833883, @klimek wrote: > > > Tests? > > > TSAN does not catch this (as it's a logical error) and it requires a rather > bizarre timing of file reparses to trigger. > I c

[PATCH] D36159: clang-format: [JS] handle single lines comments ending in `\\`.

klimek accepted this revision. klimek added a comment. This revision is now accepted and ready to land. lg Comment at: lib/Format/FormatTokenLexer.cpp:544-545 +while (BackslashPos != StringRef::npos) { + if (BackslashPos + 1 < FormatTok->TokenText.size() && +

[PATCH] D36261: [clangd] Use multiple working threads in clangd.

klimek added a comment. High-level question: Why can't we use llvm::ThreadPool? https://reviews.llvm.org/D36261 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D36398: [clangd] Check if CompileCommand has changed on forceReparse.

klimek added a comment. Also missing tests :) Comment at: clangd/ClangdUnitStore.cpp:45 + .first; +Result.RemovedFile = nullptr; + } else if (!compileCommandsAreEqual(It->second->getCompileCommand(), ilya-biryukov wrote: > klimek wrote: > > Jus

[PATCH] D35955: clang-format: Add preprocessor directive indentation

klimek added a comment. In https://reviews.llvm.org/D35955#834914, @djasper wrote: > Manuel: Can you take a look at the last comment here? Why does PPBranchLevel > start at -1? It's a perhaps too-clever implementation to make sure that we can have a per-branch data structure (indexed from 0)

[PATCH] D35955: clang-format: Add preprocessor directive indentation

klimek added a comment. In https://reviews.llvm.org/D35955#835994, @euhlmann wrote: > In https://reviews.llvm.org/D35955#835439, @klimek wrote: > > > I think if we need this info, we can just make it count down to -1 again > > (or, but that's isomorphic, let it run from 0 and make sure we never

[PATCH] D35355: Fix templated type alias completion when using global completion cache

klimek accepted this revision. klimek added a comment. This revision is now accepted and ready to land. LG https://reviews.llvm.org/D35355 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-com

[PATCH] D36529: Fixed a race condition in PrecompiledPreamble.

klimek added inline comments. Comment at: lib/Frontend/PrecompiledPreamble.cpp:470-471 + int FD; + auto EC = llvm::sys::fs::createTemporaryFile(Prefix, Suffix, /*ref*/ FD, + /*ref*/ File); if (EC) I don't underst

[PATCH] D36529: Fixed a race condition in PrecompiledPreamble.

klimek accepted this revision. klimek added a comment. This revision is now accepted and ready to land. LG, as discussed in person, it's probably a good idea to try to get rid of the non-file-creating version, if possible, or at least fix the comments on the functions to make this behavior clear

[PATCH] D36261: [clangd] Use multiple working threads in clangd.

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

[PATCH] D36398: [clangd] Check if CompileCommand has changed on forceReparse.

klimek accepted this revision. klimek added a comment. This revision is now accepted and ready to land. lg Comment at: unittests/clangd/ClangdTests.cpp:509-510 + /*RunSynchronously=*/true); + // No need to sync reparses, because RunSynchronously is set +

[PATCH] D35955: clang-format: Add preprocessor directive indentation

klimek added a comment. LG from my side. https://reviews.llvm.org/D35955 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D36397: [clangd] Fixed a data race.

klimek added inline comments. Comment at: clangd/ClangdUnit.cpp:714 -void CppFile::cancelRebuilds() { +void CppFile::cancelRebuild() { deferCancelRebuild().get(); } + Somewhat orthogonal question - why CppFile? We do support other languages in Clangd, right? (

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

[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] D54952: [clangd] DO NOT SUBMIT. Draft interfaces for build system integration.

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

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

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

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

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

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

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

[PATCH] 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 http://lists.llvm.org/cgi-bin/

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

[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 http://lists.llv

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

[PATCH] D51258: Extract parseBindID method

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

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

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

[PATCH] D54077: [clangd] Implemented DraftFileSystem

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

[PATCH] D54077: [clangd] Implemented DraftFileSystem

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

[PATCH] D54077: [clangd] Implemented DraftFileSystem

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

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

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

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

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

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

klimek added a comment. In D28462#1405360 , @MyDeveloperDay wrote: > I'm not a code owner here but we seem to be lacking people who are either > available, able, willing or allowed to approve code reviews for some of the > code in the clang-tooling (not

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

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

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

[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 cfe-com

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

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

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

[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 = Tokens->set

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

<    1   2   3   4   5   6   >