Re: [PATCH] D24383: Add addOrMerge interface to tooling::Replacements.

2016-09-10 Thread Daniel Jasper via cfe-commits
djasper added a comment. This seems like a pretty weird function to have in the interface. Could you explain what use case(s) you are envisioning? https://reviews.llvm.org/D24383 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.

Re: [PATCH] D18313: clang-format: Make include sorting's main include detection configurable.

2016-09-10 Thread Daniel Jasper via cfe-commits
djasper closed this revision. djasper marked 6 inline comments as done. djasper added a comment. Submitted as r263943. Comment at: include/clang/Format/Format.h:415 @@ +414,3 @@ + /// as the "main" include in both a.cc and a_test.cc. + std::string IncludeMainRegex; + -

Re: [PATCH] D24401: clang-format: Add Java detection to git-clang-format.

2016-09-10 Thread Daniel Jasper via cfe-commits
djasper accepted this revision. djasper added a comment. This revision is now accepted and ready to land. Looks good. Repository: rL LLVM https://reviews.llvm.org/D24401 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.or

Re: [PATCH] D21026: [clang-format] append newline after code when inserting new headers at the end of the code which does not end with newline.

2016-09-10 Thread Daniel Jasper via cfe-commits
djasper added a comment. Does this still work with the new Replacements? Comment at: lib/Format/Format.cpp:1572 @@ +1571,3 @@ +// When inserting headers at end of the code, also insert a '\n' at the end +// of the code if it does not ends with '\n'. +// The way of in

Re: [PATCH] D21279: Fix some issues in clang-format's AlignConsecutive modes

2016-09-10 Thread Daniel Jasper via cfe-commits
djasper added inline comments. Comment at: lib/Format/WhitespaceManager.cpp:47 @@ +46,3 @@ + +int WhitespaceManager::Change::ScopeLevel() const { + // NestingLevel is raised on the opening paren/square, and remains raised What I don't understand is why you have t

Re: [PATCH] D24383: Add addOrMerge interface to tooling::Replacements.

2016-09-10 Thread Alexander Shaposhnikov via cfe-commits
alexshap added a comment. disclaimer: i don't know if this method is the right thing to add (to be honest i would prefer a better interface but don't have any good suggestions on my mind at the moment), i see several issues (IMO, i might be mistaken, apologize in advance) with the current inter

Re: [PATCH] D24395: Align declarations that are preceded by different number of commas.

2016-09-10 Thread Daniel Jasper via cfe-commits
djasper added a comment. I think this will also be solved by https://reviews.llvm.org/D21279. https://reviews.llvm.org/D24395 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D22130: Link static PIE programs against rcrt0.o on OpenBSD

2016-09-10 Thread Stefan Kempf via cfe-commits
Ed Maste wrote: > emaste added a comment. > > Seems fine to me, but I'm not particularly knowledgeable about OpenBSD's > toolchain. Could you commit it please if it looks ok? This diff is what OpenBSD has in its tree. We'd like to get it upstream. > https://reviews.llvm.org/D22130 > > > __

r281136 - Add missing test coverage for an inheritance model attrib merge diag.

2016-09-10 Thread Nico Weber via cfe-commits
Author: nico Date: Sat Sep 10 08:03:59 2016 New Revision: 281136 URL: http://llvm.org/viewvc/llvm-project?rev=281136&view=rev Log: Add missing test coverage for an inheritance model attrib merge diag. Without this, no tests fail if I remove the Diag() in the first if in Sema::mergeMSInheritanceAt

Re: [PATCH] D21113: Add support for case-insensitive header lookup

2016-09-10 Thread Nico Weber via cfe-commits
The current status is that this is too slow, so for now the recommendation is to have a case insensitive mount for the files that need it. On Sep 9, 2016 8:18 PM, "John Sheu" wrote: > sheu added a subscriber: sheu. > sheu added a comment. > > I'd be very interested in seeing this patch happen.

Re: [PATCH] D24383: Add addOrMerge interface to tooling::Replacements.

2016-09-10 Thread Eric Liu via cfe-commits
This function has been implemented in several places, so I figure it might be useful to have this function in the library. Although I'm not sure whether this should be a class interface or just a standalone helper function. @alexshap: thanks for the explanation. For your point B, I'm not sure abo

Re: [PATCH] D24383: Add addOrMerge interface to tooling::Replacements.

2016-09-10 Thread Daniel Jasper via cfe-commits
There are several things I find particularly confusing about this. Fundamentally, Replacements refer to a specific version of the code. Say you have a Replacements object R and a Replacement r that you want to integrate into it. Further assume, that C0 is the initial version of the code whereas C1

Re: [PATCH] D24439: [Clang] Fix some Clang-tidy modernize-use-bool-literals and Include What You Use warnings; other minor fixes

2016-09-10 Thread Saleem Abdulrasool via cfe-commits
compnerd added inline comments. Comment at: lib/Basic/SourceManager.cpp:395 @@ -383,2 +394,3 @@ + for (llvm::DenseMap::iterator I = FileInfos.begin(), E = FileInfos.end(); I != E; ++I) { if (I->second) { Cant this be rewritten as: for (const aut

Re: [PATCH] D24383: Add addOrMerge interface to tooling::Replacements.

2016-09-10 Thread Александр Шапошников via cfe-commits
@ioeric: 1. regarding B: i apologize in advance, i am not sure if it's directly related to your patch, but i'm kinda afraid that the current approach will shadow the issue (if it's considered to be an issue). For simplicity let's take a look at the following example: Point.h: struct Point {};

r281150 - CodeGen: remove unnecessary else case

2016-09-10 Thread Saleem Abdulrasool via cfe-commits
Author: compnerd Date: Sat Sep 10 20:25:15 2016 New Revision: 281150 URL: http://llvm.org/viewvc/llvm-project?rev=281150&view=rev Log: CodeGen: remove unnecessary else case Refactor the assignment so that its much more clear that the if-clause contains the lookup, and once cached is directly used

Re: [PATCH] D24439: [Clang] Fix some Clang-tidy modernize-use-bool-literals and Include What You Use warnings; other minor fixes

2016-09-10 Thread Piotr Padlewski via cfe-commits
Prazek added a subscriber: Prazek. Prazek accepted this revision. Prazek added a reviewer: Prazek. Prazek added a comment. This revision is now accepted and ready to land. Lgtm Repository: rL LLVM https://reviews.llvm.org/D24439 ___ cfe-commits m

Re: [PATCH] D22057: Prevent devirtualization of calls to un-instantiated functions.

2016-09-10 Thread Piotr Padlewski via cfe-commits
Prazek added a subscriber: Prazek. Comment at: lib/Sema/Sema.cpp:684 @@ +683,3 @@ + for (auto PII : Pending) +if (FunctionDecl *Func = dyn_cast(PII.first)) + Func->setMarkedForPendingInstantiation(); Dry. Use auto https://reviews.llvm.org/