klimek accepted this revision.
klimek added a comment.
Yep, makes sense. Open issues are all about types :)
https://reviews.llvm.org/D26032
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-co
Author: klimek
Date: Tue Nov 1 05:30:50 2016
New Revision: 285685
URL: http://llvm.org/viewvc/llvm-project?rev=285685&view=rev
Log:
Fix parenthesized assert (nfc).
Modified:
cfe/trunk/lib/AST/Decl.cpp
Modified: cfe/trunk/lib/AST/Decl.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/l
klimek added a comment.
If the files do not exist, how does this work? Won't that potentially give
different results if the files exist locally vs if they don't?
https://reviews.llvm.org/D26288
___
cfe-commits mailing list
cfe-commits@lists.llvm.or
+richard
On Fri, Oct 14, 2016 at 10:18 AM Joshua Hurwitz via cfe-commits <
cfe-commits@lists.llvm.org> wrote:
> See attached.
>
> Returning a bool from main is a special case of return type mismatch. The
> common convention when returning a bool is that 'true' (== 1) indicates
> success and 'fals
klimek added a comment.
In https://reviews.llvm.org/D26288#586932, @ioeric wrote:
> - Addressed comments: handle non-existing files.
We're not really handling them now though? We're just printing an error?
My point is that we might run the replacement generation on a distributed
system, and t
klimek added a comment.
Ok, makes sense. Can you add a comment to the function that explains what
happens with replacements for files that do not exist (we ignore them, unless
I'm mistaken). Otherwise LG.
https://reviews.llvm.org/D26288
___
cfe-co
On Sun, Nov 6, 2016 at 1:18 AM Michał Górny via cfe-commits <
cfe-commits@lists.llvm.org> wrote:
> On Fri, 14 Oct 2016 17:17:59 +
> Joshua Hurwitz via cfe-commits wrote:
>
> > See attached.
> >
> > Returning a bool from main is a special case of return type mismatch. The
> > common convention
Yea, we had that discussion a few times, and I can never remember why we
ended up in the state we're in.
We definitely had a time where we switched to just using the exact same
name as the node's class name for the matchers.
I *think* we didn't do it for cxxRecordDecl, because Richard said that's a
Author: klimek
Date: Tue Sep 8 05:11:26 2015
New Revision: 246998
URL: http://llvm.org/viewvc/llvm-project?rev=246998&view=rev
Log:
Fix documentation of numSelectorArgs.
Currently, the documentation for numSelectorArgs includes an incorrect
example. It shows a case where an argument of 1 will ma
klimek accepted this revision.
klimek added a reviewer: klimek.
klimek added a comment.
This revision is now accepted and ready to land.
LG, as this is a documentation change that looks about right, and comes with
tests, and the original author doesn't jump in.
http://reviews.llvm.org/D12471
klimek closed this revision.
klimek added a comment.
Committed as r246998.
http://reviews.llvm.org/D12471
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Author: klimek
Date: Tue Sep 8 05:31:09 2015
New Revision: 247001
URL: http://llvm.org/viewvc/llvm-project?rev=247001&view=rev
Log:
Update code owners for AST matchers / libtooling.
Modified:
cfe/trunk/CODE_OWNERS.TXT
Modified: cfe/trunk/CODE_OWNERS.TXT
URL:
http://llvm.org/viewvc/llvm-pro
On Tue, Sep 8, 2015 at 2:23 PM Aaron Ballman wrote:
> On Tue, Sep 8, 2015 at 5:40 AM, Manuel Klimek wrote:
> > Yea, we had that discussion a few times, and I can never remember why we
> > ended up in the state we're in.
> > We definitely had a time where we switched to just using the exact same
On Tue, Sep 8, 2015 at 3:23 PM Aaron Ballman wrote:
> On Tue, Sep 8, 2015 at 9:18 AM, Manuel Klimek wrote:
> > On Tue, Sep 8, 2015 at 2:23 PM Aaron Ballman
> wrote:
> >>
> >> On Tue, Sep 8, 2015 at 5:40 AM, Manuel Klimek
> wrote:
> >> > Yea, we had that discussion a few times, and I can never
Author: klimek
Date: Tue Sep 8 10:14:06 2015
New Revision: 247018
URL: http://llvm.org/viewvc/llvm-project?rev=247018&view=rev
Log:
Fix performance regression when running clang tools.
Brings tool start time for a large synthetic test case down from (on my
machine) 4 seconds to 0.5 seconds.
Mod
In r247018 I get a ~8x speedup on a synthetic benchmark I tried. Can you
validate this fixes the regression?
On Sat, Sep 5, 2015 at 12:56 AM Hans Wennborg wrote:
> On Fri, Aug 14, 2015 at 2:55 AM, Manuel Klimek via cfe-commits
> wrote:
> > Author: klimek
> > Date: Fri Au
@@ -179,11 +185,13 @@ public:
/// \param Directory The base directory used in the
FixedCompilationDatabase.
static FixedCompilationDatabase *loadFromCommandLine(int &Argc,
const char *const
*Argv,
-
klimek added a comment.
Ok, so just for my understanding: the nested name specifier in the changed
tests canonicaltype is *not* the user-written type (thus, what the elaborated
type would get us), but the full nested name specifier of the canonical type?
http://reviews.llvm.org/D11797
_
klimek accepted this revision.
klimek added a comment.
This revision is now accepted and ready to land.
LG
http://reviews.llvm.org/D12732
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-comm
klimek added inline comments.
Comment at: clang-tidy/modernize/LoopConvertCheck.cpp:458
@@ -438,2 +457,3 @@
// variable.
for (const auto &I : Usages) {
+ std::string ReplaceText;
I'd call that 'Usage' here, as it's not an iterator.
klimek added inline comments.
Comment at: clang-tidy/modernize/LoopConvertUtils.cpp:765-766
@@ -764,2 +764,4 @@
// exactly one usage.
- addUsage(Usage(nullptr, false, C->getLocation()));
+ // We are using the 'IsArrow' field of Usage to store if we have to add
+
klimek added inline comments.
Comment at: clang-tidy/modernize/LoopConvertCheck.cpp:468-470
@@ +467,5 @@
+// are VarDecl). If the index is captured by value, add '&' to capture
+// by reference instead.
+ReplaceText =
+Usage.Kind == Usage::UK_Ca
klimek accepted this revision.
klimek added a comment.
This revision is now accepted and ready to land.
LG. This is great, thanks!
http://reviews.llvm.org/D12736
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/m
On Thu, Sep 10, 2015 at 8:36 PM Argyrios Kyrtzidis
wrote:
> On Sep 10, 2015, at 1:48 AM, Manuel Klimek wrote:
>
> @@ -179,11 +185,13 @@ public:
>/// \param Directory The base directory used in the
> FixedCompilationDatabase.
>static FixedCompilationDatabase *loadFromCommandLine(int &Argc
klimek accepted this revision.
klimek added a comment.
This revision is now accepted and ready to land.
In http://reviews.llvm.org/D12734#243157, @angelgarcia wrote:
> Comment the enumerators.
>
> > Do we need default?
>
>
> I think so. We need to set the cases that do not fall in any of these
>
Richard! We need an informed opinion :D
On Fri, Sep 11, 2015 at 3:07 PM Aaron Ballman
wrote:
> Ping?
>
> On Tue, Sep 8, 2015 at 9:26 AM, Manuel Klimek wrote:
> > On Tue, Sep 8, 2015 at 3:23 PM Aaron Ballman
> wrote:
> >>
> >> On Tue, Sep 8, 2015 at 9:18 AM, Manuel Klimek
> wrote:
> >> > On Tu
Ok, looked at the original patch again, and if we're fixing the
FixedCompilationDatabase to only insert the file when it actually produces
a CompileCommand it seems to be fine.
On Fri, Sep 11, 2015 at 6:32 PM Argyrios Kyrtzidis
wrote:
> On Sep 11, 2015, at 12:21 AM, Manuel Klimek wrote:
>
> On
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,
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
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
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
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
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
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 (
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
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
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
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:
>
>>
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
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
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
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
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
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
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
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
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
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) {
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
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
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
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
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
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
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
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
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
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
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-
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
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'
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
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/
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:
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
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
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
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
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
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
@@ +
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
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
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) {
+
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
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:
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
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
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::
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++
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
__
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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/
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[
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
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
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
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
101 - 200 of 588 matches
Mail list logo
