[PATCH] D29300: [clang-format] Refactor WhitespaceManager and friends

klimek added a comment. Generally looks like the right direction, minus that I'm not sure yet what the plan for things broken in BreakableToken are. Comment at: lib/Format/TokenAnnotator.cpp:1843 +Current->MatchingParen->opensBlockOrBlockTypeList(Style)) +

[PATCH] D29322: [clang-format] Fix regression merging comments across newlines.

klimek accepted this revision. klimek added a comment. This revision is now accepted and ready to land. lg minus adding the FIXME to the place where we need the change. Comment at: lib/Format/UnwrappedLineParser.cpp:2127-2129 +// Additional fine-grained breaking of line

[PATCH] D29622: Add a batch query and replace tool based on AST matchers.

klimek added inline comments. Comment at: clang-query/QueryReplace.cpp:50 + +void QueryReplaceTool::addOperation(clang::query::QueryReplaceSpec ) { + ast_matchers::dynamic::Diagnostics Diag; Shouldn't that also just return the error? Comment

[PATCH] D29626: [clang-format] Break before a sequence of line comments aligned with the next line.

klimek added a comment. I think this looks pretty good. More comments would help :) Also, organize is spelled with a 'z' in American. https://reviews.llvm.org/D29626 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D29221: clang-format-vsix: "format on save" feature

klimek added a reviewer: hans. klimek added a comment. +hans +1 to "format only current document but save all" not making much sense :) Comment at: tools/clang-format-vs/ClangFormat/TypeConverterUtils.cs:7 +{ +public sealed class TypeConverterUtils +{

[PATCH] D29626: [clang-format] Break before a sequence of line comments aligned with the next line.

klimek added inline comments. Comment at: lib/Format/UnwrappedLineParser.cpp:2207-2208 +const FormatToken *NextTok) { + // Decides which comment tokens should be added to the current line and which + // should be added as comments before the next token. + //

[PATCH] D29626: [clang-format] Break before a sequence of line comments aligned with the next line.

klimek added inline comments. Comment at: lib/Format/UnwrappedLineParser.h:121-123 + // Comments specifies the sequence of comment tokens to analyze. They get + // either pushed to the current line or added to the comments before the next + // token. Given

[PATCH] D29626: [clang-format] Break before a sequence of line comments aligned with the next line.

klimek added inline comments. Comment at: lib/Format/UnwrappedLineParser.h:121-123 + // Comments specifies the sequence of comment tokens to analyze. They get + // either pushed to the current line or added to the comments before the next + // token. krasimir

[PATCH] D29699: [clang-tidy] Add -extra-arg and -extra-arg-before to clang-tidy-diff.py

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

[PATCH] D29626: [clang-format] Break before a sequence of line comments aligned with the next line.

klimek added inline comments. Comment at: lib/Format/UnwrappedLineParser.h:121-123 + // Comments specifies the sequence of comment tokens to analyze. They get + // either pushed to the current line or added to the comments before the next + // token. krasimir

[PATCH] D29626: [clang-format] Break before a sequence of line comments aligned with the next line.

klimek accepted this revision. klimek added a comment. This revision is now accepted and ready to land. lg. I'd probably still call it consumeComments or something, as we require a flush afterwards for the unconsumed comments, but I don't feel too strongly about that.

[PATCH] D29451: Add a prototype for clangd v0.1

klimek added inline comments. Comment at: clangd/JSONRPCDispatcher.h:25-32 + virtual ~Handler() = default; + + /// Called when the server receives a method call. This is supposed to return + /// a result on Outs. + virtual void handleMethod(llvm::yaml::MappingNode *Params,

[PATCH] D29451: Add a prototype for clangd v0.1

klimek accepted this revision. klimek added a comment. LG. Couple of questions. Comment at: clangd/ClangDMain.cpp:65 +// Now read the JSON. +std::vector JSON; +JSON.resize(Len); Adi wrote: > Avoid unnecessary JSON.resize(Len) & potential

[PATCH] D28764: [clang-format] Implement comment reflowing (v3)

klimek added inline comments. Comment at: lib/Format/BreakableToken.h:40 +/// of the content after a split has been used for breaking, and +/// - insertBreak, for executing the split using a whitespace manager. +/// Do we want to describe how replaceWhitespace

[PATCH] D28764: [clang-format] Implement comment reflowing (v3)

klimek added a comment. This is starting to be pretty awesome. The one thing that would help me review the gist of the implementation a bit more is if that had a bit more comments. Perhaps go over the math in the code and put some comments in why we're doing what at various steps.

[PATCH] D28764: [clang-format] Implement comment reflowing (v3)

klimek added inline comments. Comment at: lib/Format/BreakableToken.h:42-48 +/// There is a pair of operations that are used to compress a long whitespace +/// range with a single space if that will bring the line lenght under the +/// column limit: +/// -

[PATCH] D28764: [clang-format] Implement comment reflowing (v3)

klimek added inline comments. Comment at: lib/Format/ContinuationIndenter.cpp:1158-1159 +CommentPragmasRegex.match(Current.TokenText.substr(2)) || +Current.TokenText.substr(2).ltrim().startswith("clang-format on") || +

[PATCH] D29271: Revert r293455, which breaks v8 with a spurious error. Testcase added.

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

[PATCH] D28764: [clang-format] Implement comment reflowing (v3)

klimek added inline comments. Comment at: lib/Format/BreakableToken.cpp:279-280 + return Content.size() >= 2 && + Content != "clang-format on" && + Content != "clang-format off" && + !Content.endswith("\\") && Can we now use delimitsRegion here?

[PATCH] D28764: [clang-format] Implement comment reflowing (v3)

klimek added inline comments. Comment at: lib/Format/BreakableToken.cpp:747 +Split SplitBefore, WhitespaceManager ) { + // If this is the first line of a token, we need to inform Whitespace Manager + // about it: either adapt the whitespace range preceding it, or mark it

[PATCH] D28764: [clang-format] Implement comment reflowing (v3)

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

[PATCH] D30210: [include-fixer] Add usage count to find-all-symbols.

klimek added inline comments. Comment at: include-fixer/find-all-symbols/FindAllSymbols.cpp:251 + } else { +assert(false && "Must match a NamedDecl!"); + } You can use llvm_unreachable instead. Or do you actually want to fall through in release mode?

[PATCH] D30111: [clang-format] Add a test to check at once all the Mozilla coding style

klimek added inline comments. Comment at: test/Format/check-coding-style-mozilla.cpp:7-9 +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at

[PATCH] D29886: [clangd] Wire up ASTUnit and publish diagnostics with it.

klimek added inline comments. Comment at: clangd/ASTManager.cpp:69 + // our one-element queue is empty. + if (!RequestIsPending && !Done) +ClangRequestCV.wait(Lock); arphaman wrote: > This is just a suggestion based on personal opinion:

[PATCH] D27810: Normalize all filenames before searching FileManager caches

klimek added inline comments. Comment at: lib/Basic/FileManager.cpp:167-168 DirNameStr = DirName.str() + '.'; +llvm::sys::path::native(DirNameStr); +DirName = DirNameStr; + } else { I'd add a canonicalizePath function that: -> on unix just returns

[PATCH] D29943: [clang-format] Align block comment decorations

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

[PATCH] D29943: [clang-format] Align block comment decorations

klimek added inline comments. Comment at: lib/Format/BreakableToken.cpp:397 +// +// Don't try aligning if there are less lines, since some patterns like +// fewer lines Comment at: lib/Format/BreakableToken.cpp:402 +

[PATCH] D29886: [clangd] Wire up ASTUnit and publish diagnostics with it.

klimek added inline comments. Comment at: clangd/ASTManager.h:67 + /// Setting Done to true will make the worker thread terminate. + std::atomic Done; +}; bkramer wrote: > klimek wrote: > > arphaman wrote: > > > It looks like `Done` is always accessed in a

[PATCH] D29886: [clangd] Wire up ASTUnit and publish diagnostics with it.

klimek accepted this revision. klimek added a comment. This revision is now accepted and ready to land. lg Comment at: clangd/ASTManager.cpp:138-139 + // Currently we discard all pending requests and just enqueue the latest one. + while (!RequestQueue.empty()) +

[PATCH] D28764: [clang-format] Implement comment reflowing (v3)

klimek added inline comments. Comment at: lib/Format/BreakableToken.h:55-56 +/// been reformatted, and +/// - replaceWhitespaceBefore, for executing the reflow using a whitespace +/// manager. +/// Shouldn't that be called insertBreakBefore for consistency

[PATCH] D27054: Introducing clang::tooling::AtomicChange for refactoring tools.

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

[PATCH] D30532: Add examples to clang-format configuration

klimek added inline comments. Comment at: docs/ClangFormatStyleOptions.rst:218 + +#define A \ +int ; \ Do we really align these 3 spaces out? Comment at: docs/ClangFormatStyleOptions.rst:447 + SomeClass::Constructor() +

[PATCH] D30385: clang-format: Don't leave behind temp files in -i mode on Windows, PR26125, reloaded

klimek added a comment. Also, do we want to not call ReplaceFile in Path.inc on Win if it potentially leaves temp files lying around? Reading the code, it looks like we currently actually believe it may fail, and retry with MoveFileEx afterwards... https://reviews.llvm.org/D30385

[PATCH] D30385: clang-format: Don't leave behind temp files in -i mode on Windows, PR26125, reloaded

klimek added a comment. Can we open the files with FILE_SHARE_DELETE instead? https://reviews.llvm.org/D30385 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D30385: clang-format: Don't leave behind temp files in -i mode on Windows, PR26125, reloaded

klimek added inline comments. Comment at: include/clang/Rewrite/Core/Rewriter.h:188-197 + /// prewrite(FID) is called after all changes to FID have been written to a + /// temporary file, but before that temporary file is moved into FID's + /// location. Windows's

[PATCH] D27138: Extend CompilationDatabase by a field for the output filename

klimek added inline comments. Comment at: lib/Tooling/JSONCompilationDatabase.cpp:266 +nodeToCommandLine(Syntax, std::get<2>(CommandsRef[I])), +Output ? Output->getValue(OutputStorage) : ""); } joerg wrote: > klimek wrote: > > Optional: I'd

[PATCH] D27138: Extend CompilationDatabase by a field for the output filename

klimek added inline comments. Comment at: lib/Tooling/JSONCompilationDatabase.cpp:266 +nodeToCommandLine(Syntax, std::get<2>(CommandsRef[I])), +Output ? Output->getValue(OutputStorage) : ""); } joerg wrote: > klimek wrote: > > joerg wrote: > >

[PATCH] D27138: Extend CompilationDatabase by a field for the output filename

klimek added inline comments. Comment at: lib/Tooling/JSONCompilationDatabase.cpp:266 +nodeToCommandLine(Syntax, std::get<2>(CommandsRef[I])), +Output ? Output->getValue(OutputStorage) : ""); } Optional: I'd probably let the nodeToCommandLine

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

klimek updated this revision to Diff 79530. klimek added a comment. Add tests, update hasDeclaration to work for ElaboratedType. https://reviews.llvm.org/D27104 Files: include/clang/ASTMatchers/ASTMatchersInternal.h unittests/ASTMatchers/ASTMatchersTraversalTest.cpp Index:

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

klimek added a comment. In https://reviews.llvm.org/D27104#607161, @lukasza wrote: > Forcing shallow matching means that unit test below will stop passing after > this CL. > > TEST(HasDeclaration, DeepTagType) { > std::string input = > "class Foo {};\n" > "using Bar =

[PATCH] D27447: [ASTMatcher] Add hasReplacementType matcher for SubstTemplateTypeParmType

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

[PATCH] D27501: clang-format-vsix: add command to format document

klimek added inline comments. Comment at: tools/clang-format-vs/ClangFormat/ClangFormat.vsct:76 + + Clang Format Document + hans wrote: > amaiorano wrote: > > hans wrote: > > > I think File would be better than Document when referring to

[PATCH] D27440: clang-format-vsix: fail when clang-format outputs to stderr

klimek added a reviewer: djasper. klimek added a comment. Adding djasper, who had brought forward the idea. https://reviews.llvm.org/D27440 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D27501: clang-format-vsix: add command to format document

klimek accepted this revision. klimek added a comment. This revision is now accepted and ready to land. LG. I also think it makes the code nicer by breaking out the right functions. Thanks! Comment at: tools/clang-format-vs/ClangFormat/ClangFormat.vsct:76 + +

[PATCH] D27557: Update the default of the Mozilla coding style

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

[PATCH] D27754: [clang-format] Implement comment reflowing (again).

klimek added inline comments. Comment at: lib/Format/ContinuationIndenter.cpp:1174-1175 LineIndex != EndIndex; ++LineIndex) { -if (!DryRun) - Token->replaceWhitespaceBefore(LineIndex, Whitespaces); +Token->replaceWhitespaceBefore(LineIndex,

[PATCH] D27754: [clang-format] Implement comment reflowing (again).

klimek added inline comments. Comment at: lib/Format/BreakableToken.cpp:471 + WhitespaceManager ) { + if (Tok.is(TT_LineComment)) { +// If this is the first line of a token, inform Whitespace Manager about it.

[PATCH] D28548: Improve include fixer's ranking by taking the paths into account.

klimek created this revision. klimek added a reviewer: bkramer. klimek added a subscriber: cfe-commits. Instead of just using popularity, we also take into account how similar the path of the current file is to the path of the header. Our first approach is to get popularity into a reasonably

[PATCH] D28548: Improve include fixer's ranking by taking the paths into account.

This revision was automatically updated to reflect the committed changes. Closed by commit rL291664: Improve include fixer's ranking by taking the paths into account. (authored by klimek). Changed prior to commit: https://reviews.llvm.org/D28548?vs=83930=83937#toc Repository: rL LLVM

[PATCH] D27207: Adds hasUnqualifiedDesugaredType to allow matching through type sugar.

This revision was automatically updated to reflect the committed changes. Closed by commit rL288366: Adds hasUnqualifiedDesugaredType to allow matching through type sugar. (authored by klimek). Changed prior to commit: https://reviews.llvm.org/D27207?vs=79891=79912#toc Repository: rL LLVM

[PATCH] D27207: Adds hasUnqualifiedDesugaredType to allow matching through type sugar.

klimek added a comment. hasUnqualifiedDesugaredType(hasDeclaration( How about using hasUnqualifiedDesugaredType(recordType(hasDeclaration instead? https://reviews.llvm.org/D27207 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D27181: [ASTImporter] Support for importing UsingDecl and UsingShadowDecl

klimek added a comment. Can you add a test to ASTMatchersNodeTests.cpp? Otherwise LG for the matcher parts. https://reviews.llvm.org/D27181 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D27138: Extend CompilationDatabase by a field for the output filename

klimek added inline comments. Comment at: lib/Tooling/JSONCompilationDatabase.cpp:266 +nodeToCommandLine(Syntax, std::get<2>(CommandsRef[I])), +Output ? Output->getValue(OutputStorage) : ""); } joerg wrote: > klimek wrote: > > joerg wrote: > >

[PATCH] D27140: Allow clang to write compilation database records

klimek added reviewers: bkramer, akyrtzi. klimek added a comment. Adding some folks. One question is whether we can use the additional output stuff doug added at some point for this. https://reviews.llvm.org/D27140 ___ cfe-commits mailing list

[PATCH] D27207: Adds hasUnqualifiedDesugaredType to allow matching through type sugar.

klimek created this revision. klimek added reviewers: bkramer, lukasza. klimek added a subscriber: cfe-commits. https://reviews.llvm.org/D27207 Files: docs/LibASTMatchersReference.html include/clang/ASTMatchers/ASTMatchers.h lib/ASTMatchers/Dynamic/Registry.cpp

[PATCH] D27138: Extend CompilationDatabase by a field for the output filename

klimek added a comment. In https://reviews.llvm.org/D27138#607786, @joerg wrote: > It's not the directory, but the output file. That's optional since it is a > new addition and I don't want to invalidate all existing JSON databases. It seems like we're talking past each other. I'm not

[PATCH] D27211: [clang-format] Implement comment reflowing

klimek added a comment. > for these cases the > original Breakable{Line,Block}Comment still breaks the long lines. Do you intend for this to continue to be the case? https://reviews.llvm.org/D27211 ___ cfe-commits mailing list

[PATCH] D27447: [ASTMatcher] Add hasReplacementType matcher for SubstTemplateTypeParmType

klimek added inline comments. Comment at: include/clang/ASTMatchers/ASTMatchers.h:5034 +/// \c substTemplateTypeParmType(hasReplacementType(type())) matches int +AST_MATCHER_P(SubstTemplateTypeParmType, hasReplacementType, + ast_matchers::internal::Matcher,

[PATCH] D27440: clang-format-vsix: fail when clang-format outputs to stderr

klimek added a comment. Pondering this a bit - one question is whether we should make clang-format not return 0 if we pass -fallback-style=none (it already has this option) and we can't figure out a style. What do you think? https://reviews.llvm.org/D27440

[PATCH] D27140: Allow clang to write compilation database records

klimek accepted this revision. klimek added a comment. This revision is now accepted and ready to land. LG. We can always add more intricate ways later on. Repository: rL LLVM https://reviews.llvm.org/D27140 ___ cfe-commits mailing list

[PATCH] D27350: CFGBuilder: Fix crash when visiting delete expression on dependent type

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

[PATCH] D27138: Extend CompilationDatabase by a field for the output filename

klimek accepted this revision. klimek added a comment. This revision is now accepted and ready to land. In https://reviews.llvm.org/D27138#607859, @joerg wrote: > Which struct are we talking about, `CompileCommandRef` or `CompileCommand`? > It is a pointer in the former and a plain StringRef in

[PATCH] D27207: Adds hasUnqualifiedDesugaredType to allow matching through type sugar.

klimek added a comment. I added a test for multi-stage desugaring. Generally, we want to model Clang's AST where it makes sense. The AST has a getUnqualifiedDesugaredType, thus the matcher as it is is expected. We can add single step desugaring or hasAnyDeclaration or similar things when they

[PATCH] D27207: Adds hasUnqualifiedDesugaredType to allow matching through type sugar.

klimek updated this revision to Diff 79891. klimek added a comment. Add multi-level using test. https://reviews.llvm.org/D27207 Files: docs/LibASTMatchersReference.html include/clang/ASTMatchers/ASTMatchers.h lib/ASTMatchers/Dynamic/Registry.cpp

[PATCH] D28260: Add an argumentsAre matcher

klimek added a reviewer: bkramer. klimek added a comment. +benjamin https://reviews.llvm.org/D28260 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D28465: clang-format: [JS] ASI after imports

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

[PATCH] D27440: clang-format-vsix: fail when clang-format outputs to stderr

klimek added a comment. In https://reviews.llvm.org/D27440#626477, @amaiorano wrote: > In https://reviews.llvm.org/D27440#626415, @ioeric wrote: > > > `llvm::ErrorOr` carries `std::error_code`. If you want richer information > > (e.g. error_code + error message), `llvm::Expcted` and

[PATCH] D27971: Make FormatStyle.GetStyleOfFile test work on MSVC

klimek accepted this revision. klimek added a comment. This revision is now accepted and ready to land. Apart from the error handling LG. Thanks! Comment at: lib/Format/Format.cpp:1920-1922 + std::error_code EC = FS->makeAbsolute(Path); + assert(!EC); + (void)EC;

[PATCH] D27971: Make FormatStyle.GetStyleOfFile test work on MSVC

klimek added inline comments. Comment at: lib/Format/Format.cpp:1920-1922 + std::error_code EC = FS->makeAbsolute(Path); + assert(!EC); + (void)EC; amaiorano wrote: > klimek wrote: > > I think if makeAbsolute doesn't work, we will probably want to err out

[PATCH] D27971: Make FormatStyle.GetStyleOfFile test work on MSVC

klimek added a comment. Why isn't the right solution to make getStyle() use vfs::FileSystem? Generally, everything in clang-format (well, in clang) should use vfs::FileSystem for file access. https://reviews.llvm.org/D27971 ___ cfe-commits

[PATCH] D27440: clang-format-vsix: fail when clang-format outputs to stderr

klimek added a reviewer: ioeric. klimek added a comment. +eric, who has some experience llvm::Error'ing our interfaces :) llvm::ErrorOr seems like the right approach here? https://reviews.llvm.org/D27440 ___ cfe-commits mailing list

[PATCH] D31992: [clangd] Escape only necessary characters in JSON output

klimek added inline comments. Comment at: clangd/Protocol.cpp:26-50 + for (llvm::StringRef::iterator i = Input.begin(), e = Input.end(); i != e; ++i) { +if (*i == '\\') + EscapedInput += ""; +else if (*i == '"') + EscapedInput += "\\\""; +// bell +

[PATCH] D31992: [clangd] Escape only necessary characters in JSON output

klimek added a comment. In https://reviews.llvm.org/D31992#725912, @malaperle-ericsson wrote: > In https://reviews.llvm.org/D31992#725866, @krasimir wrote: > > > Seems that we're starting to hit some YAML/JSON mismatches, or is it that > > your YAML string support is lacking? > > > I don't

[PATCH] D30735: Add missing implementation for AtomicChange::replace(...)

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

[PATCH] D30777: Added `applyAtomicChanges` function.

klimek added inline comments. Comment at: include/clang/Tooling/Refactoring/AtomicChange.h:129 +struct ApplyChangesSpec { + // If true, cleans up redundant/erroneous around changed code with + // clang-format's cleanup functionality, e.g. redundant commas around deleted

[PATCH] D27810: FileManager: mark virtual file entries as valid entries

klimek added a comment. Is the diff view on phab broken, or am I missing something? I only see a single line of diff now, and don't see a way to change the diff. https://reviews.llvm.org/D27810 ___ cfe-commits mailing list

[PATCH] D30650: [clang-tidy] misc-use-after-move: Fix failing assertion

klimek added a comment. In https://reviews.llvm.org/D30650#693165, @Eugene.Zelenko wrote: > I think we should refactor this check as part of Static Analyzer, since it's > path-sensitive. Are you saying it should be path sensitive? Because currently it's not, right?

[PATCH] D30636: [analyzer] Fix crash when building CFG with variable of incomplete type

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

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

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

[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

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

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

[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=109325#toc Repository: rL LLVM

[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

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

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

[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

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

klimek added a comment. In https://reviews.llvm.org/D34440#808156, @vladimir.plyashkun wrote: > **To discuss:** > This patch is required for correct integration //Clang-Tidy// with //CLion > IDE//. > By this moment, we unable to use //CompilationDatabase.json// from //CLion > //side which is

[PATCH] D34267: do more processing in clang-fuzzer (use EmitAssemblyAction)

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

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

klimek added a comment. In https://reviews.llvm.org/D34440#809581, @vladimir.plyashkun wrote: > > Are there any concerns using the alternative? > > I can't say that it's a big problems, but i think that: > //CompilationDatabase.json// is more //CMake //specific format. > It can be generated

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

klimek added a comment. In https://reviews.llvm.org/D34440#809325, @vladimir.plyashkun wrote: > Even if i'll change content of //arguments.rsp// to > `-std=c++11 -Ipath/to/include -Ipath/to/include2 -DMACRO ` > and will try to call clang-tidy process in this way: > `clang-tidy -checks=*

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

klimek added inline comments. Comment at: include/clang/Tooling/Refactoring/ASTSelection.h:30 + /// inside of its source range. + ContainsSelectionPoint, + It's unclear what a selection point is. I'd have expected this to mean "overlaps" when first reading

[PATCH] D33644: Add default values for function parameter chunks

klimek added inline comments. Comment at: lib/Sema/SemaCodeComplete.cpp:2411-2412 + if (DefValue.at(0) != '=') { +// If we don't have = in front of value +return " = " + DefValue; + } Can you expand in the comment on when each of these happens?

[PATCH] D34512: Add preliminary Cross Translation Unit support library

klimek added a comment. Specifically, ping Richard for new top-level lib in clang. https://reviews.llvm.org/D34512 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

  1   2   3   4   5   6   >