Re: [PATCH] D13388: Add support for querying the visibility of a cursor

klimek added a subscriber: klimek. klimek added a reviewer: milianw. klimek added a comment. This LG, looping in Milian for a second opinion / to find more reviewers :) http://reviews.llvm.org/D13388 ___ cfe-commits mailing list cfe-commits@lists.ll

Re: [PATCH] D13516: Fix overlapping replacements in clang-tidy.

klimek added inline comments. Comment at: clang-tidy/ClangTidyDiagnosticConsumer.cpp:418-419 @@ +417,4 @@ +unsigned Pos; +int Type; +int ErrorId; + }; These need to be documented. Comment at: clang-tidy/ClangTidyDiagnosticConsumer.cp

Re: [PATCH] D10833: Retrieve BinaryOperator::getOpcode and BinaryOperator::getOpcodeStr via libclang and its python interface

klimek added a reviewer: milianw. klimek added a comment. +milian - can you take a look at this patch (or do you know somebody who might be in a good position to review) http://reviews.llvm.org/D10833 ___ cfe-commits mailing list cfe-commits@lists.

Re: [PATCH] D13590: Support every kind of initialization.

klimek accepted this revision. klimek added a comment. This revision is now accepted and ready to land. lg Comment at: clang-tidy/modernize/MakeUniqueCheck.cpp:125-128 @@ +124,6 @@ + // Direct initialization with initialization list. + // \code + // struct S { S

Re: [PATCH] D13549: Added new options to ClangFormat VSIX package.

klimek added reviewers: aaron.ballman, rnk. klimek added a comment. +aaron, as token Windows Wizard +rnk, as I was told you might know people who are in a good position to review MS specific stuff (perhaps engineers from MS might be able to help here? :) http://reviews.llvm.org/D13549 _

Re: [PATCH] D13640: [clang-tidy] Add new check cppcoreguidelines-pro-bounds-array-to-pointer-decay

klimek added a subscriber: klimek. Comment at: clang-tidy/cppcoreguidelines/ProBoundsArrayToPointerDecayCheck.cpp:40 @@ +39,3 @@ + + diag(MatchedCast->getExprLoc(), "do not (implicitly) convert an array to a pointer"); +} Can't we provide a fixit? http://revi

Re: [PATCH] D13658: [VFS] Let the user decide if they want path normalization.

klimek accepted this revision. klimek added a comment. This revision is now accepted and ready to land. lg Comment at: include/clang/Basic/VirtualFileSystem.h:282 @@ -280,2 +281,3 @@ /// Add a buffer to the VFS with a path. The VFS owns the buffer. - void addFile(const Twine

Re: [PATCH] D13658: [VFS] Let the user decide if they want path normalization.

klimek added inline comments. Comment at: include/clang/Basic/VirtualFileSystem.h:276 @@ -275,2 +275,3 @@ std::string WorkingDirectory; + bool UseNormalizedPaths; I'd use an in class initializer. Comment at: include/clang/Basic/VirtualFileS

Re: r250036 - [VFS] Don't try to be heroic with '.' in paths.

I believe we need a more principled approach here :) 1. The first thing we need is to actually produce nice errors when we hit an addFile() with the same path (normalized or non-normalized) I'd propose the following approach: we compare the buffers; if the buffer contents are equal, we declare suc

Re: [PATCH] D13549: Added new options to ClangFormat VSIX package.

klimek added inline comments. Comment at: tools/clang-format-vs/ClangFormat/ClangFormatPackage.cs:186 @@ -145,1 +185,3 @@ +if (!string.IsNullOrEmpty(assumeFilename)) + process.StartInfo.Arguments += " -assume-filename \"" + assumeFilename + "\"";

Re: [PATCH] D13549: Added new options to ClangFormat VSIX package.

klimek added inline comments. Comment at: tools/clang-format-vs/ClangFormat/ClangFormatPackage.cs:186 @@ -145,1 +185,3 @@ +if (!string.IsNullOrEmpty(assumeFilename)) + process.StartInfo.Arguments += " -assume-filename \"" + assumeFilename + "\"";

Re: [PATCH] D13549: Added new options to ClangFormat VSIX package.

klimek added inline comments. Comment at: tools/clang-format-vs/ClangFormat/ClangFormatPackage.cs:186 @@ -145,1 +185,3 @@ +if (!string.IsNullOrEmpty(assumeFilename)) + process.StartInfo.Arguments += " -assume-filename \"" + assumeFilename + "\"";

Re: [PATCH] D13549: Added new options to ClangFormat VSIX package.

klimek added inline comments. Comment at: tools/clang-format/ClangFormat.cpp:227 @@ -213,1 +226,3 @@ + llvm::outs() << """; + break; default: curdeius wrote: > klimek wrote: > > For XML, we should only need "<" and "&". Everything else is just impor

Re: [PATCH] D13549: Added new options to ClangFormat VSIX package.

klimek added inline comments. Comment at: lib/Driver/Tools.cpp:2356 @@ -2355,3 +2355,3 @@ CmdArgs.push_back( -Args.MakeArgString("-dwarf-version=" + llvm::itostr(DwarfVersion))); +Args.MakeArgString("-dwarf-version=" + Twine(DwarfVersion))); } ---

Re: [PATCH] D13639: Add decayedType and hasDecayedType AST matchers

klimek added a comment. Oh, please also run the docs update script cd docs/tools python ./dump_ast_matchers.py http://reviews.llvm.org/D13639 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo

Re: [PATCH] D13639: Add decayedType and hasDecayedType AST matchers

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

Re: r249831 - [VFS] Wire up driver VFS through tooling.

On Fri, Oct 9, 2015 at 3:05 PM Benjamin Kramer via cfe-commits < cfe-commits@lists.llvm.org> wrote: > Author: d0k > Date: Fri Oct 9 08:03:25 2015 > New Revision: 249831 > > URL: http://llvm.org/viewvc/llvm-project?rev=249831&view=rev > Log: > [VFS] Wire up driver VFS through tooling. > > Sadly I

Re: [PATCH] D13474: [VFS] Port tooling to use the in-memory file system.

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

Re: [clang-tools-extra] r249399 - Add a new module for the C++ Core Guidelines, and the first checker for those guidelines: cppcoreguidelines-pro-type-reinterpret-cast.

On Wed, Oct 7, 2015 at 7:34 PM Aaron Ballman via cfe-commits < cfe-commits@lists.llvm.org> wrote: > On Wed, Oct 7, 2015 at 12:05 PM, Joerg Sonnenberger via cfe-commits > wrote: > > On Tue, Oct 06, 2015 at 01:31:01PM -, Aaron Ballman via cfe-commits > wrote: > >> Log: > >> Add a new module for

Re: [PATCH] D13523: ASTMatchers: Keep AllCallbacks in a set instead of a vector

klimek accepted this revision. klimek added a comment. This revision is now accepted and ready to land. That makes a lot of sense. LG http://reviews.llvm.org/D13523 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bi

Re: [PATCH] D13368: [clang-tidy] add check cppcoreguidelines-pro-type-static-cast-downcast

On Tue, Oct 6, 2015 at 4:18 PM Aaron Ballman wrote: > On Tue, Oct 6, 2015 at 10:15 AM, Manuel Klimek wrote: > > On Tue, Oct 6, 2015 at 4:12 PM Aaron Ballman > > wrote: > >> > >> aaron.ballman added a comment. > >> > >> In http://reviews.llvm.org/D13368#260672, @klimek wrote: > >> > >> > In http

Re: [PATCH] D13368: [clang-tidy] add check cppcoreguidelines-pro-type-static-cast-downcast

On Tue, Oct 6, 2015 at 4:12 PM Aaron Ballman wrote: > aaron.ballman added a comment. > > In http://reviews.llvm.org/D13368#260672, @klimek wrote: > > > In http://reviews.llvm.org/D13368#260669, @aaron.ballman wrote: > > > > > This wasn't a comment on the rule so much as a comment on the > diagnos

Re: [PATCH] D13368: [clang-tidy] add check cppcoreguidelines-pro-type-static-cast-downcast

klimek added a subscriber: klimek. klimek added a comment. In http://reviews.llvm.org/D13368#260669, @aaron.ballman wrote: > This wasn't a comment on the rule so much as a comment on the diagnostic not > being very helpful.In this case, you're telling the user to not do something, > but it is u

Re: [PATCH] D13469: Create interfaces and tests for the overlapping replacements fix in clang-tidy.

klimek accepted this revision. klimek added a comment. This revision is now accepted and ready to land. LG Comment at: unittests/clang-tidy/OverlappingReplacementsTest.cpp:21 @@ +20,3 @@ +const char BoundIf[] = "if"; + +class UseCharCheck : public ClangTidyCheck { --

Re: [PATCH] D13469: Create interfaces and tests for the overlapping replacements fix in clang-tidy.

klimek added inline comments. Comment at: unittests/clang-tidy/OverlappingReplacementsTest.cpp:133 @@ +132,3 @@ + +TEST(OverlappingReplacementsTest, TestingChecksWorkAsExpected) { + const char Code[] = I'd split it up and give the tests better names. TestingCheck

r249391 - Adds a way for tools to deduce the target config from a compiler name.

Author: klimek Date: Tue Oct 6 05:45:03 2015 New Revision: 249391 URL: http://llvm.org/viewvc/llvm-project?rev=249391&view=rev Log: Adds a way for tools to deduce the target config from a compiler name. Adds `addTargetAndModeForProgramName`, a utility function that will add appropriate `-target

Re: [PATCH] D13318: Add a utility function to add target information to a command line

klimek closed this revision. klimek added a comment. Submitted as r249391. http://reviews.llvm.org/D13318 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D13430: [VFS] Add an in-memory file system implementation.

klimek accepted this revision. klimek added a comment. This revision is now accepted and ready to land. lg Comment at: lib/Basic/VirtualFileSystem.cpp:1263-1265 @@ -957,5 +1262,5 @@ if (!F->useExternalName(UseExternalNames)) { // Provide a file wrapper that returns the

Re: [PATCH] D13433: Use better mocks in modernize-make-unique, and fix matcher.

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

Re: [PATCH] D13203: [Clang] - Massaging code to fix MSVS 2015 win32-release configuration

klimek added a comment. Note: with VS Professional 14.0.23107.0 D14REL I do not get this error. http://reviews.llvm.org/D13203 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D13430: [VFS] Add an in-memory file system implementation.

klimek added inline comments. Comment at: include/clang/Basic/VirtualFileSystem.h:286-288 @@ +285,5 @@ + } + std::error_code setCurrentWorkingDirectory(const Twine &Path) override { +WorkingDirectory = Path.str(); +return std::error_code(); + } Return e

Re: [PATCH] D13431: Document a bug in loop-convert and fix one of its subcases.

klimek accepted this revision. klimek added a comment. This revision is now accepted and ready to land. lg Comment at: clang-tidy/modernize/LoopConvertCheck.cpp:529 @@ -527,1 +528,3 @@ Range = Paren->getSourceRange(); + } else if (const auto *Uop = Parents[

Re: [PATCH] D13318: Add a utility function to add target information to a command line

klimek added a comment. Did you test this with cmake? I get undef reference functions when linking ToolingTest... http://reviews.llvm.org/D13318 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/

Re: [PATCH] D13246: Fix bug in modernize-use-nullptr.

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

Re: [PATCH] D11908: Clang support for -fthinlto.

klimek added a comment. In http://reviews.llvm.org/D11908#258570, @tejohnson wrote: > Sorry for the duplicate - my previous response didn't go to Duncan or Mehdi > for some reason. Trying again... > > In http://reviews.llvm.org/D11908#258540, @klimek wrote: > > > Perhaps "sharded" would fit what

Re: [PATCH] D13352: [PATCH] Add a CERT category for clang-tidy checkers

I dont think we need finer grained code owners, but I also don't have real objections. On Fri, Oct 2, 2015, 3:31 PM Aaron Ballman wrote: > aaron.ballman closed this revision. > aaron.ballman added a comment. > > Thanks! I've commit in r249130. > > Do we want to have code owners for this sort of

Re: [PATCH] D13352: [PATCH] Add a CERT category for clang-tidy checkers

klimek added a subscriber: klimek. klimek accepted this revision. klimek added a reviewer: klimek. klimek added a comment. This revision is now accepted and ready to land. lg http://reviews.llvm.org/D13352 ___ cfe-commits mailing list cfe-commits@li

Re: [PATCH] D13381: Handle trailing underscores on modernize-loop-convert variable namer.

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

Re: [PATCH] D11908: Clang support for -fthinlto.

klimek added a subscriber: klimek. klimek added a comment. Perhaps "sharded" would fit what it is? http://reviews.llvm.org/D11908 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D13381: Handle trailing underscores on modernize-loop-convert variable namer.

klimek added inline comments. Comment at: clang-tidy/modernize/LoopConvertUtils.cpp:822 @@ -821,1 +821,3 @@ IteratorName = ContainerName.substr(0, Len - 1); +// Ej: (auto thing : things) +if (!declarationExists(IteratorName)) Do you mean: E.g.? http

Re: [PATCH] D12854: [SourceManager] Support buffers that are not null-terminated

klimek added a subscriber: klimek. klimek added a comment. Generally, I thought clang often relies on buffers being null terminated to speed up parse times. Usually the MemoryBuffers have an option to guarantee null-terminatedness (and copy if necessary) Repository: rL LLVM http://reviews.l

Re: [PATCH] D13344: Keep the IfStmt node even if the condition is invalid

klimek added a reviewer: rsmith. klimek added a comment. +Richard, whom we need to validate AST changes to make sure they're benign. http://reviews.llvm.org/D13344 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin

Re: [PATCH] D13346: Update clang-tidy documentation.

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

Re: [PATCH] D13203: [Clang] - Massaging code to fix MSVS 2015 win32-release configuration

klimek added a subscriber: klimek. klimek added a comment. Not sure whether it makes sense to work around compiler bugs in CL. I assume this happens with clang from trunk? Is there a bug filed against CL? http://reviews.llvm.org/D13203 ___ cfe-comm

Re: [PATCH] D13342: Prevent loop-convert from leaving empty lines after removing an alias declaration.

klimek accepted this revision. klimek added a comment. This revision is now accepted and ready to land. apart from that LG http://reviews.llvm.org/D13342 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/l

Re: [PATCH] D13342: Prevent loop-convert from leaving empty lines after removing an alias declaration.

klimek added inline comments. Comment at: clang-tidy/modernize/LoopConvertCheck.cpp:455-457 @@ +454,5 @@ +/// +/// FIXME: if 'next_instruction' is a closing brace ('}'), after the replacement +/// it will be over-indented. But then, who would declare an alias and do +/// nothing

Re: [PATCH] D13249: Divide TraverseInitListExpr to a different function for each form.

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

Re: [PATCH] D13292: Add support for 'cbegin()' and 'cend()' on modernize-loop-convert.

klimek accepted this revision. klimek added a comment. This revision is now accepted and ready to land. ... Which indicates that the tests might need better names and more comments, so I don't mess them up next time :D http://reviews.llvm.org/D13292 __

Re: [PATCH] D13318: Add a utility function to add target information to a command line

klimek added a comment. In http://reviews.llvm.org/D13318#257091, @echristo wrote: > This seems a little odd, could you explain in a bit more detail? Me not > understanding I imagine. :) Seems to be explained well in the comments? If the compilation database contains: i686-linux-android-g++

Re: [PATCH] D13318: Add a utility function to add target information to a command line

klimek accepted this revision. klimek added a comment. This revision is now accepted and ready to land. LG Comment at: include/clang/Tooling/Tooling.h:437 @@ +436,3 @@ +/// +/// \note This will not set \c CommandLine[0] to \c InvokedAs. +void addTargetAndModeForProgramName(std::

Re: [PATCH] D13249: Divide TraverseInitListExpr to a different function for each form.

klimek added inline comments. Comment at: include/clang/AST/RecursiveASTVisitor.h:2066-2089 @@ -2058,26 +2065,26 @@ -// InitListExpr is a tricky one, because we want to do all our work on -// the syntactic form of the listexpr, but this method takes the -// semantic form by defa

Re: [PATCH] D13249: Divide TraverseInitListExpr to a different function for each form.

klimek added a reviewer: rsmith. klimek added a comment. +richard to tell us whether that comment is correct :) http://reviews.llvm.org/D13249 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe

Re: [PATCH] D13249: Divide TraverseInitListExpr to a different function for each form.

I think it's worth figuring out when this is called with the semantic or syntactic version and why this can't lead to double visitation. Then add a comment while you're changing the method so the next person doesn't have to figure it all out :) On Wed, Sep 30, 2015 at 12:15 AM Angel Garcia wrote:

Re: [PATCH] D13249: Divide TraverseInitListExpr to a different function for each form.

klimek added inline comments. Comment at: include/clang/AST/RecursiveASTVisitor.h:2097 @@ +2096,3 @@ +template +bool RecursiveASTVisitor::TraverseInitListExpr(InitListExpr *S) { + TRY_TO(TraverseSyntacticInitListExpr(S)); Did you try putting an assert(S->isSeman

Re: [PATCH] D13166: Create modernize-make-unique check.

klimek accepted this revision. klimek added a comment. This revision is now accepted and ready to land. apart from the comment, LG Comment at: clang-tidy/modernize/MakeUniqueCheck.cpp:67-70 @@ +66,6 @@ + SourceLocation ConstructCallEnd; + if (LAngle == StringRef::npos) { +

Re: [PATCH] D13210: Make brace styles more configurable

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

Re: [PATCH] D13228: clang-format: Extend #include sorting functionality

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

Re: [PATCH] D13228: clang-format: Extend #include sorting functionality

klimek added inline comments. Comment at: lib/Format/Format.cpp:1665 @@ +1664,3 @@ + + // Create pre-compile regular expressions for the #include categories. + SmallVector CategoryRegexs; pre-compiled Comment at: lib/Format/Format.cpp:1677 @@ +

Re: [clang-tools-extra] r248438 - Fix loop-convert for trivially copyable types.

Yes that's already planned. On Mon, Sep 28, 2015, 5:10 PM David Blaikie wrote: > On Sat, Sep 26, 2015 at 11:21 PM, Manuel Klimek via cfe-commits < > cfe-commits@lists.llvm.org> wrote: > >> Yep. We'll make it better by limiting the size, but trivially copyable is

Re: [PATCH] D13166: Create modernize-make-unique check.

klimek added a comment. In http://reviews.llvm.org/D13166#254730, @angelgarcia wrote: > This raises a question. Do we want to do replacements when we use an alias > for std::unique_ptr? That fact that something is an unique_ptr might be an > implementation detail that should not be exposed, but

Re: [PATCH] D13166: Create modernize-make-unique check.

klimek added inline comments. Comment at: clang-tidy/modernize/MakeUniqueCheck.cpp:75-76 @@ +74,4 @@ + std::string Identifier = removeWhitespace(ExprStr.substr(0, LAngle)); + if (Identifier != "unique_ptr" && Identifier != "std::unique_ptr") +return; + SourceLocation Constr

Re: Bug 23529: Add support for gcc's attribute abi_tag (needed for compatibility with gcc 5's libstdc++)

On Sun, Sep 27, 2015 at 2:45 PM Stefan Bühler via cfe-commits < cfe-commits@lists.llvm.org> wrote: > Hi, > > it seems moderation didn't approve the phabricator mails for D12834. > (I have no intention to be subscribed to the list just to get > phabricator mails through. For now I am subscribed but

Re: [PATCH] D13206: Add clang-query tool to installation targets

klimek closed this revision. klimek added a comment. Submitted in r248710 http://reviews.llvm.org/D13206 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] r248710 - Install clang-query by default.

Author: klimek Date: Mon Sep 28 08:26:39 2015 New Revision: 248710 URL: http://llvm.org/viewvc/llvm-project?rev=248710&view=rev Log: Install clang-query by default. It is already installed by the autotools build, and it is useful for developers who are not working on LLVM/Clang itself. Modified:

Re: [PATCH] D13206: Add clang-query tool to installation targets

klimek added a subscriber: klimek. klimek accepted this revision. klimek added a reviewer: klimek. klimek added a comment. This revision is now accepted and ready to land. LG As we already a) install it from auto-tools and b) this tool is useful for non-llvm/clang devs http://reviews.llvm.org/

Re: [PATCH] D13166: Create modernize-make-unique check.

klimek added a comment. In http://reviews.llvm.org/D13166#254520, @angelgarcia wrote: > Two tests in which 'getTokenLength' returns 0. Thanks for the tests - question is: I would have expected us to use something like Lexer::getSourceText, which should give us the full range in the first test

Re: [PATCH] D13166: Create modernize-make-unique check.

klimek added inline comments. Comment at: clang-tidy/modernize/MakeUniqueCheck.cpp:26-28 @@ +25,5 @@ +/// \brief Returns the length of the token that goes since the beggining of the +/// constructor call until the '<' of the template. This token should either be +/// 'unique_ptr'

Re: [clang-tools-extra] r248438 - Fix loop-convert for trivially copyable types.

Yep. We'll make it better by limiting the size, but trivially copyable is an improvement, as there are orders of magnitude more loops over small copyable types than over large ones. On Sat, Sep 26, 2015, 9:02 PM comex wrote: > On Thu, Sep 24, 2015 at 7:28 AM, Manuel Klimek via cfe

Re: [PATCH] D12666: [LibClang] Fix clang_getCursorAvailability

klimek added a comment. Submitted as r248596. Sergey, if you plan to work on libclang more, please get commit access (it's easy :) Thx http://reviews.llvm.org/D12666 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-

Re: [PATCH] D12666: [LibClang] Fix clang_getCursorAvailability

Submitted as r248596. Sergey, if you plan to work on libclang more, please get commit access (it's easy :) Thx On Fri, Sep 18, 2015 at 5:25 AM Milian Wolff wrote: > milianw added a subscriber: milianw. > milianw added a comment. > > Ping? Can someone please submit this upstream? > > > http://re

r248596 - Fix bug on reporting availability of deleted methods in libclang.

Author: klimek Date: Fri Sep 25 12:53:16 2015 New Revision: 248596 URL: http://llvm.org/viewvc/llvm-project?rev=248596&view=rev Log: Fix bug on reporting availability of deleted methods in libclang. Patch by Sergey Kalinichev. Added: cfe/trunk/test/Index/availability.cpp Modified: cfe/tr

Re: [PATCH] D13166: Create modernize-make-unique check.

klimek added a comment. This is definitely a useful check to have in modernize. Comment at: clang-tidy/modernize/MakeUniqueCheck.cpp:25-27 @@ +24,5 @@ + +/// \brief Returns the length of the token that goes since the beggining of the +/// constructor call until the '<' of the te

Re: [PATCH] D13133: Remove dangling parenthesis.

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

Re: [PATCH] D13133: Remove dangling parenthesis.

klimek added inline comments. Comment at: clang-tidy/modernize/LoopConvertCheck.cpp:482 @@ +481,3 @@ +auto Parents = Context->getParents(*Usage.Expression); +if (Parents.size() == 1) { + if (const auto *Paren = Parents[0].get()) Perhaps ad

Re: [PATCH] D13133: Remove dangling parenthesis.

klimek added a comment. Can you add a test where we need the parens? (where the element is of ** type or something) http://reviews.llvm.org/D13133 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinf

Re: [Diffusion] rL248438: Fix loop-convert for trivially copyable types.

Yep, as I said, I would love to do that, but it would require significant effort :( On Thu, Sep 24, 2015 at 7:03 AM Alexander Kornienko wrote: > Too bad. Making these two kinds of mails go to the same thread is hardly a > trivial thing. And completely switching commit notifications to Phabricato

Re: [Diffusion] rL248438: Fix loop-convert for trivially copyable types.

The biggest problem is that those comments don't go on the cfe-commmits thread that gets auto-triggered by commits, and we really want to not add new threads. On Thu, Sep 24, 2015 at 4:28 AM Alexander Kornienko wrote: > alexfh added inline comments. > > /clang-tools-extra/trunk/clang-tidy/modern

[clang-tools-extra] r248449 - Use simpler interface for getting the pointee type for a node.

Author: klimek Date: Wed Sep 23 19:16:38 2015 New Revision: 248449 URL: http://llvm.org/viewvc/llvm-project?rev=248449&view=rev Log: Use simpler interface for getting the pointee type for a node. Modified: clang-tools-extra/trunk/clang-tidy/modernize/LoopConvertCheck.cpp Modified: clang-tool

[clang-tools-extra] r248438 - Fix loop-convert for trivially copyable types.

Author: klimek Date: Wed Sep 23 17:28:14 2015 New Revision: 248438 URL: http://llvm.org/viewvc/llvm-project?rev=248438&view=rev Log: Fix loop-convert for trivially copyable types. Previously, we would rewrite: void f(const vector &v) { for (size_t i = 0; i < v.size(); ++i) { to for (const aut

[clang-tools-extra] r248418 - Fix loop-convert for const references to containers.

Author: klimek Date: Wed Sep 23 13:40:47 2015 New Revision: 248418 URL: http://llvm.org/viewvc/llvm-project?rev=248418&view=rev Log: Fix loop-convert for const references to containers. Previously we would use a non-const loop variable in the range-based loop for: void f(const std::vector &v) {

Re: [PATCH] D12407: [clang-format-vs] Add an option to reformat source code when file is saved to disk

klimek added a comment. It's not falling into oblivion, but it's not going to happen before next week, unless somebody else picks the review up. http://reviews.llvm.org/D12407 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm

Re: [PATCH] D12407: [clang-format-vs] Add an option to reformat source code when file is saved to disk

It's not falling into oblivion, but it's not going to happen before next week, unless somebody else picks the review up. On Wed, Sep 23, 2015 at 1:08 AM Beren Minor wrote: > berenm added a comment. > > Ping? Just to be sure it's not going to fall to oblivion. :) > > > http://reviews.llvm.org/D12

Re: [PATCH] D11240: Add basic #include sorting functionality to clang-format

klimek accepted this revision. klimek added a comment. This revision is now accepted and ready to land. Ok, I think this is now understandable enough for me to go in. http://reviews.llvm.org/D11240 ___ cfe-commits mailing list cfe-commits@lists.llvm

Re: [PATCH] D11240: Add basic #include sorting functionality to clang-format

klimek added a comment. Sending another batch of comments. Comment at: lib/Tooling/Core/Replacement.cpp:305-307 @@ +304,5 @@ + Replacements Result; + // Iterate over both sets and always add the next element (smallest total + // Offset) from either 'First' or 'Second'. Merge

Re: [PATCH] D12797: Refactor LoopConvertCheck.

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

Re: PATCH: Provide the compile commands of the JSON database in consistent order

LG in general; I think if we like the order to be deterministic we should try to come up with a unit test so nobody regresses this in the future. On Fri, Sep 18, 2015 at 4:44 PM Argyrios Kyrtzidis wrote: > Hi, > > I have found it useful for the getAllCompileCommands() to return the > commands in

Re: recordDecl() AST matcher

LG, ship it. On Wed, Sep 16, 2015 at 2:03 PM Aaron Ballman wrote: > Attached is an updated patch for clang-tools-extra that does not have > my in-progress, not-related-to-any-of-this code in it. ;-) > > ~Aaron > > On Wed, Sep 16, 2015 at 4:04 PM, Aaron Ballman > wrote: > > Quick ping. I know th

Re: PATCH: Expose the 'file' that is associated with a compile database command

ks for reviewing! >> >> On Sep 11, 2015, at 10:24 AM, Manuel Klimek via cfe-commits < >> cfe-commits@lists.llvm.org> wrote: >> >> Ok, looked at the original patch again, and if we're fixing the >> FixedCompilationDatabase to only insert the file when it

Re: [PATCH] D12797: Refactor LoopConvertCheck.

klimek added inline comments. Comment at: clang-tidy/modernize/LoopConvertCheck.cpp:419 @@ +418,3 @@ + // assumption the user is trying to modernize their codebase. + if (getLangOpts().CPlusPlus) { +Finder->addMatcher(makeArrayLoopMatcher(), this); Now you c

Re: recordDecl() AST matcher

Feel free to rename the AST nodes :) On Mon, Sep 14, 2015, 2:44 PM Daniel Jasper wrote: > Ok. I am happy with this then. > > (Just personally grumpy having to write > cxxRecordDecl(has(cxxConstructorDecl(..))) in the future ;-) ). > > On Mon, Sep 14, 2015 at 11:41 PM, Manuel Klimek wrote: > >>

Re: recordDecl() AST matcher

On Mon, Sep 14, 2015 at 2:29 PM Aaron Ballman wrote: > On Mon, Sep 14, 2015 at 4:38 PM, Manuel Klimek wrote: > > > > > > On Mon, Sep 14, 2015 at 12:26 PM Aaron Ballman > > wrote: > >> > >> On Mon, Sep 14, 2015 at 2:45 PM, Daniel Jasper > wrote: > >> > By this point, I see that change might be

Re: [PATCH] D11240: Add basic #include sorting functionality to clang-format

klimek added a comment. First round of comments; some things are still a bit confusing, so I hope another round will help to weed them out. Comment at: include/clang/Tooling/Core/Replacement.h:223-224 @@ -222,1 +222,4 @@ +/// \brief Merges to sets of replacements with the sec

Re: recordDecl() AST matcher

On Mon, Sep 14, 2015 at 12:26 PM Aaron Ballman wrote: > On Mon, Sep 14, 2015 at 2:45 PM, Daniel Jasper wrote: > > By this point, I see that change might be profitable overall. However, > lets > > completely map this out. Changing just cxxRecordDecl() can actually > increase > > confusion in othe

Re: recordDecl() AST matcher

On Mon, Sep 14, 2015 at 11:45 AM Daniel Jasper wrote: > By this point, I see that change might be profitable overall. However, > lets completely map this out. Changing just cxxRecordDecl() can actually > increase confusion in other areas. Right now, not a single AST matcher has > the cxx prefix (

Re: recordDecl() AST matcher

On Mon, Sep 14, 2015 at 10:30 AM Daniel Jasper wrote: > On Mon, Sep 14, 2015 at 7:24 PM, Manuel Klimek wrote: > >> On Mon, Sep 14, 2015 at 10:21 AM Daniel Jasper >> wrote: >> >>> So, back in the day when we implemented the matchers, we decided on >>> actually wanting to remove all the CXX... AS

Re: recordDecl() AST matcher

On Mon, Sep 14, 2015 at 10:21 AM Daniel Jasper wrote: > So, back in the day when we implemented the matchers, we decided on > actually wanting to remove all the CXX... AST nodes (there are more of > them). > Note that Richard has paddled back on this and now says the CXX... AST nodes are the rig

Re: recordDecl() AST matcher

On Mon, Sep 14, 2015, 8:40 AM Aaron Ballman wrote: > On Sat, Sep 12, 2015 at 11:06 PM, Manuel Klimek wrote: > > > > > > On Sat, Sep 12, 2015, 9:25 PM Aaron Ballman > wrote: > >> > >> On Sat, Sep 12, 2015 at 8:22 AM, Manuel Klimek > wrote: > >> > > >> > > >> > On Fri, Sep 11, 2015 at 10:39 PM A

Re: recordDecl() AST matcher

On Sat, Sep 12, 2015, 9:25 PM Aaron Ballman wrote: > On Sat, Sep 12, 2015 at 8:22 AM, Manuel Klimek wrote: > > > > > > On Fri, Sep 11, 2015 at 10:39 PM Aaron Ballman > > wrote: > >> > >> On Fri, Sep 11, 2015 at 4:30 PM, Richard Smith > >> wrote: > >> > I don't think CXXRecordDecl is an anachro

Re: recordDecl() AST matcher

On Fri, Sep 11, 2015 at 10:39 PM Aaron Ballman wrote: > On Fri, Sep 11, 2015 at 4:30 PM, Richard Smith > wrote: > > I don't think CXXRecordDecl is an anachronism, so much as an > implementation > > detail; it makes sense to use a smaller class when in C mode, as we don't > > need most of the fea

Re: PATCH: Expose the 'file' that is associated with a compile database command

Test would be awesome :) Thx! On Fri, Sep 11, 2015 at 10:44 PM Argyrios Kyrtzidis wrote: > In r247468, thanks for reviewing! > > On Sep 11, 2015, at 10:24 AM, Manuel Klimek via cfe-commits < > cfe-commits@lists.llvm.org> wrote: > > Ok, looked at the original patch again,

<    1   2   3   4   5   6   >