Re: r263785 - Make LookupResult movable again.

2016-03-19 Thread David Blaikie via cfe-commits
On Fri, Mar 18, 2016 at 6:31 AM, Benjamin Kramer via cfe-commits < cfe-commits@lists.llvm.org> wrote: > Author: d0k > Date: Fri Mar 18 08:31:00 2016 > New Revision: 263785 > > URL: http://llvm.org/viewvc/llvm-project?rev=263785&view=rev > Log: > Make LookupResult movable again. > This shouldn't'v

Re: [PATCH] D18123: Fix implicit copy ctor and copy assignment operator warnings when -Wdeprecated passed.

2016-03-19 Thread David Blaikie via cfe-commits
On Wed, Mar 16, 2016 at 11:38 AM, don hinton via cfe-commits < cfe-commits@lists.llvm.org> wrote: > hintonda updated this revision to Diff 50843. > hintonda added a comment. > > Added FIXME comment, and reformated with clang-format. > > > http://reviews.llvm.org/D18123 > > Files: > include/clang

r263732 - Remove defaulted move ops, the type is zero-cost copyable anyway, so there's no need for specific move ops

2016-03-19 Thread David Blaikie via cfe-commits
Author: dblaikie Date: Thu Mar 17 13:28:16 2016 New Revision: 263732 URL: http://llvm.org/viewvc/llvm-project?rev=263732&view=rev Log: Remove defaulted move ops, the type is zero-cost copyable anyway, so there's no need for specific move ops (addresses MSVC build error, since MSVC 2013 can't gen

Re: [PATCH] D18271: Avoid -Wshadow warnings about constructor parameters named after fields

2016-03-19 Thread David Blaikie via cfe-commits
On Fri, Mar 18, 2016 at 11:08 AM, Reid Kleckner via cfe-commits < cfe-commits@lists.llvm.org> wrote: > rnk added a comment. > > I'm not sure your example is in scope for -Wshadow, though. Any function > call that takes a non-const reference to the parameter could modify it. Sure - my argument is

Re: [PATCH] D18123: Fix implicit copy ctor and copy assignment operator warnings when -Wdeprecated passed.

2016-03-19 Thread David Blaikie via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL263730: Fix implicit copy ctor and copy assignment operator warnings when… (authored by dblaikie). Changed prior to commit: http://reviews.llvm.org/D18123?vs=50930&id=50961#toc Repository: rL LLVM h

Re: [PATCH] D16965: Fix for Bug 14644 - clang confuses scope operator for global namespace giving extra qualification on member

2016-03-19 Thread David Blaikie via cfe-commits
dblaikie added a comment. Not sure I fully understand the issue with the existing diagnostic/change in wording. '::' is a nested name specifier, even if it's a degenerate case, I think - so the old wording doesn't seem wrong, as such (& the new text seems correct in the other cases too - shoul

Re: [PATCH] D18123: Fix implicit copy ctor and copy assignment operator warnings when -Wdeprecated passed.

2016-03-19 Thread David Blaikie via cfe-commits
On Wed, Mar 16, 2016 at 7:54 AM, don hinton via cfe-commits < cfe-commits@lists.llvm.org> wrote: > hintonda updated this revision to Diff 50823. > hintonda added a comment. > > Address FIXME now that Sema::LookupInlineAsmField() has been fixed. > > > http://reviews.llvm.org/D18123 > > Files: > i

Re: [PATCH] D18123: Fix implicit copy ctor and copy assignment operator warnings when -Wdeprecated passed.

2016-03-19 Thread David Blaikie via cfe-commits
On Wed, Mar 16, 2016 at 10:19 AM, don hinton wrote: > The current behavior, albeit deprecated, is to implicitly define these. > Therefore, it would be incorrect not to delete them if the implicit > versions don't do the right thing. > > I'd be happy to add a FIXME, but I doubt they will ever be r

Re: r263785 - Make LookupResult movable again.

2016-03-19 Thread David Blaikie via cfe-commits
Also might be marginally tidier if Paths was a unique_ptr, then you wouldn't have to explicitly null it out. I think this is still a pretty subtle thing to allow move or copy support for & perhaps best avoided, or split into the query parameters and the result if we're going to support it. For ex

Re: [PATCH] D18123: Fix implicit copy ctor and copy assignment operator warnings when -Wdeprecated passed.

2016-03-19 Thread David Blaikie via cfe-commits
On Wed, Mar 16, 2016 at 10:52 AM, don hinton wrote: > -Werror clean is great. > > If you could add -Wdeprecated, then we wouldn't need to delete them -- the > warning is only issued with -Wdeprecated is passed. > Right, that's what I'm saying - add a fixme so that once we turn on the deprecated

Re: r263732 - Remove defaulted move ops, the type is zero-cost copyable anyway, so there's no need for specific move ops

2016-03-18 Thread David Blaikie via cfe-commits
ovable parts - but I can't see any evidence of that). Added explicit (empty) move ops in r263747 > > thanks... > don > > On Thu, Mar 17, 2016 at 2:28 PM, David Blaikie via cfe-commits < > cfe-commits@lists.llvm.org> wrote: > >> Author: dblaikie >> Date: Thu

Re: [PATCH] D18271: Avoid -Wshadow warnings about constructor parameters named after fields

2016-03-18 Thread David Blaikie via cfe-commits
dblaikie added a subscriber: dblaikie. dblaikie added a comment. It's not just modifications of shadowed variables that are a problem - one of the one's I'm concerned we should catch is: struct foo { std::unique_ptr p; foo(std::unique_ptr p) : p(std::move(p)) { f(*p); } };

Re: [PATCH] D18271: Avoid -Wshadow warnings about constructor parameters named after fields

2016-03-18 Thread David Blaikie via cfe-commits
+Lang, because he was asking me recently about this improvement & thinking of chipping in On Fri, Mar 18, 2016 at 10:56 AM, David Blaikie via cfe-commits < cfe-commits@lists.llvm.org> wrote: > dblaikie added a subscriber: dblaikie. > dblaikie added a comment. > > It'

Re: [PATCH] D18175: Avoid using LookupResult's implicit copy ctor and assignment operator to avoid warnings

2016-03-15 Thread David Blaikie via cfe-commits
On Tue, Mar 15, 2016 at 11:26 AM, John McCall via cfe-commits < cfe-commits@lists.llvm.org> wrote: > rjmccall added a comment. > > Ignore Dave and fix the problem, please. :) > +1, sorry for the race on review. > > > Repository: > rL LLVM > > http://reviews.llvm.org/D18175 > > > > ___

Re: [PATCH] D18175: Avoid using LookupResult's implicit copy ctor and assignment operator to avoid warnings

2016-03-15 Thread David Blaikie via cfe-commits
dblaikie accepted this revision. dblaikie added a comment. This revision is now accepted and ready to land. Looks good - thanks! Repository: rL LLVM http://reviews.llvm.org/D18175 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lis

Re: [PATCH] D18123: Fix implicit copy ctor and copy assignment operator warnings when -Wdeprecated passed.

2016-03-14 Thread David Blaikie via cfe-commits
On Mon, Mar 14, 2016 at 3:30 PM, don hinton wrote: > > > On Mon, Mar 14, 2016 at 6:24 PM, David Blaikie wrote: > >> >> >> On Mon, Mar 14, 2016 at 3:07 PM, don hinton wrote: >> >>> UnresolvedSetImpl isn't even constructible, much less copy >>> constructible, but it is a public base class to Unre

Re: [PATCH] D18123: Fix implicit copy ctor and copy assignment operator warnings when -Wdeprecated passed.

2016-03-14 Thread David Blaikie via cfe-commits
On Mon, Mar 14, 2016 at 3:07 PM, don hinton wrote: > UnresolvedSetImpl isn't even constructible, much less copy constructible, > but it is a public base class to UnresolvedSet, it it is -- at least until > we turn it off by deleting the copy ctor. > > template class UnresolvedSet : > public

Re: [PATCH] D18123: Fix implicit copy ctor and copy assignment operator warnings when -Wdeprecated passed.

2016-03-14 Thread David Blaikie via cfe-commits
UNresolvedSetImpl isn't copy constructible, so should it really be copy assignable? (it looks like it only needs to be so because of LookupResult - so once LookupResult is fixed, UnresolvedSetImpl can go back to the way it was) Perhaps we should just wait for the fix for LookupResult copying in Se

Re: r263429 - [Frontend] Disable value name discarding for all sanitizers.

2016-03-14 Thread David Blaikie via cfe-commits
On Mon, Mar 14, 2016 at 8:55 AM, Evgenii Stepanov wrote: > On Mon, Mar 14, 2016 at 8:48 AM, Benjamin Kramer > wrote: > > On Mon, Mar 14, 2016 at 3:59 PM, David Blaikie > wrote: > >> Yeah - how are they relying on them in a non-asserts build anyway? > (were we > >> naming certain things regardle

Re: r263429 - [Frontend] Disable value name discarding for all sanitizers.

2016-03-14 Thread David Blaikie via cfe-commits
Yeah - how are they relying on them in a non-asserts build anyway? (were we naming certain things regardless of +/-Asserts? (well, I know we were naming some things, but mostly down in LLVM, I thought Clang generally used the IRBuilder & thus didn't name things in non-Asserts builds)) On Mon, Mar

Re: [PATCH] D18127: Remove compile time PreserveName in favor of a runtime cc1 -discard-value-names option

2016-03-13 Thread David Blaikie via cfe-commits
Interesting question going forwards: We usually try to make test cases +/-Asserts agnostic (not using named values), now that we have a flag for it, should we not bother with that & just add the command line argument to enable names when names make testing easier? Or do we think that'll make tests

Re: [PATCH] D18123: Fix implicit copy ctor and copy assignment operator warnings when -Wdeprecated passed.

2016-03-13 Thread David Blaikie via cfe-commits
On Sat, Mar 12, 2016 at 3:42 PM, don hinton via cfe-commits < cfe-commits@lists.llvm.org> wrote: > hintonda created this revision. > hintonda added a reviewer: rjmccall. > hintonda added a subscriber: cfe-commits. > > Fix implicit copy ctor and copy assignment operator warnings > when -Wdeprecated

Re: r263299 - Add fix-it for format-security warnings.

2016-03-11 Thread David Blaikie via cfe-commits
On Fri, Mar 11, 2016 at 4:14 PM, Bob Wilson via cfe-commits < cfe-commits@lists.llvm.org> wrote: > I’m not sure how to interpret that. It says: "Clang must recover from > errors as if the fix-it had been applied.” I suppose that format-security > could be an error if you’re building with -Werror,

Re: r262851 - Module Debugging: Fix a crash when emitting debug info for nested tag types

2016-03-10 Thread David Blaikie via cfe-commits
Not sure if it's worth it, but you might be able to abort the loop if you ever reach a namespace (or anything that's not a tag decl maybe? Not sure about function local types) On Thu, Mar 10, 2016 at 4:09 PM, Adrian Prantl wrote: > > On Mar 10, 2016, at 3:21 PM, David Blaikie wrote: > > OK, so

Re: r262851 - Module Debugging: Fix a crash when emitting debug info for nested tag types

2016-03-10 Thread David Blaikie via cfe-commits
OK, so the idea is that when you see that a tag definition, skip it if any parent is incomplete - because when the parent's definition is complete, you'll emit all its children anyway? On Thu, Mar 10, 2016 at 8:19 AM, Adrian Prantl wrote: > > > On Mar 9, 2016, at 5:22 PM, David Blaikie wrote: >

Re: r262851 - Module Debugging: Fix a crash when emitting debug info for nested tag types

2016-03-09 Thread David Blaikie via cfe-commits
Is this bug only reachable with modules debug info? Could you describe the nature of the fix (not quite clear to me just by looking at it) On Mon, Mar 7, 2016 at 12:58 PM, Adrian Prantl via cfe-commits < cfe-commits@lists.llvm.org> wrote: > Author: adrian > Date: Mon Mar 7 14:58:52 2016 > New Re

Re: [PATCH] D17983: Eliminate many benign instances of "potentially uninitialized local variable" warnings

2016-03-09 Thread David Blaikie via cfe-commits
On Wed, Mar 9, 2016 at 11:58 AM, Alexander Riccio via cfe-commits < cfe-commits@lists.llvm.org> wrote: > ariccio added a comment. > > In http://reviews.llvm.org/D17983#370717, @dblaikie wrote: > > > Initializations we never expect to use (eg because we have a covered > switch > > that initializes

Re: [PATCH] D17983: Eliminate many benign instances of "potentially uninitialized local variable" warnings

2016-03-09 Thread David Blaikie via cfe-commits
On Wed, Mar 9, 2016 at 12:06 PM, Alexander Riccio via cfe-commits < cfe-commits@lists.llvm.org> wrote: > ariccio added a comment. > > Oh, and by the way, what's the policy on using `enum class`es instead of C > style enums? I bet the compiler would have fewer false positives with > strongly typed

Re: [PATCH] D17983: Eliminate many benign instances of "potentially uninitialized local variable" warnings

2016-03-09 Thread David Blaikie via cfe-commits
On Mar 9, 2016 8:18 AM, "Teresa Johnson" wrote: > > > > On Wed, Mar 9, 2016 at 8:13 AM, David Blaikie wrote: >> >> >> On Mar 9, 2016 8:11 AM, "Teresa Johnson via llvm-commits" < llvm-comm...@lists.llvm.org> wrote: >> > >> > tejohnson added a subscriber: tejohnson. >> > tejohnson added a comment.

Re: [PATCH] D17983: Eliminate many benign instances of "potentially uninitialized local variable" warnings

2016-03-09 Thread David Blaikie via cfe-commits
On Mar 9, 2016 8:11 AM, "Teresa Johnson via llvm-commits" < llvm-comm...@lists.llvm.org> wrote: > > tejohnson added a subscriber: tejohnson. > tejohnson added a comment. > > Thanks for doing this! > > I will fix these 3, which one of my changes would have provoked. It is a benign case since we do h

Re: [PATCH] D17983: Eliminate many benign instances of "potentially uninitialized local variable" warnings

2016-03-09 Thread David Blaikie via cfe-commits
I think we had a discussion about this before. Clang has a few versions of this warning. One version is probably as aggressive as the warning you are trying to quiet (-Wmaybe-uninitialized) and we made a deliberate choice not to enable it for the project. I think we should probably make the same c

Re: [PATCH] D17983: Eliminate many benign instances of "potentially uninitialized local variable" warnings

2016-03-09 Thread David Blaikie via cfe-commits
dblaikie added a subscriber: dblaikie. dblaikie added a comment. I think we had a discussion about this before. Clang has a few versions of this warning. One version is probably as aggressive as the warning you are trying to quiet (-Wmaybe-uninitialized) and we made a deliberate choice not to ena

Re: [PATCH] D16949: Fix for: Bug 5941 - improve diagnostic for * vs & confusion

2016-03-04 Thread David Blaikie via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL262752: PR5941 - improve diagnostic for * vs & confusion when choosing overload… (authored by dblaikie). Changed prior to commit: http://reviews.llvm.org/D16949?vs=49577&id=49856#toc Repository: rL L

r262752 - PR5941 - improve diagnostic for * vs & confusion when choosing overload candidate with a parameter of incomplete (ref or pointer) type

2016-03-04 Thread David Blaikie via cfe-commits
Author: dblaikie Date: Fri Mar 4 16:29:11 2016 New Revision: 262752 URL: http://llvm.org/viewvc/llvm-project?rev=262752&view=rev Log: PR5941 - improve diagnostic for * vs & confusion when choosing overload candidate with a parameter of incomplete (ref or pointer) type Reviewers: dblaikie Diffe

Re: [PATCH] D17807: [clang-tidy] Add "clang-tidy as a clang plugin" skeleton.

2016-03-02 Thread David Blaikie via cfe-commits
On Wed, Mar 2, 2016 at 10:36 AM, Benjamin Kramer wrote: > In theory yes, in practice that needs a bit of build system tweaking > to either build clang-tidy into clang or as a shared library. Then it > can be used like any regular Clang plugin. > I haven't looked at this patch in detail - soundds

Re: [PATCH] D17807: [clang-tidy] Add "clang-tidy as a clang plugin" skeleton.

2016-03-02 Thread David Blaikie via cfe-commits
Would this also be usable as a way to run clang-tidy checks as part of normal compilation? (some things in clang-tidy aren't there because the FP is too high to be a hard error on old codebases (but might be viable once a codebase is able to be cleaned up (like several have been with LLVM)) or beca

Re: [PATCH] D16949: Fix for: Bug 5941 - improve diagnostic for * vs & confusion

2016-03-01 Thread David Blaikie via cfe-commits
dblaikie accepted this revision. dblaikie added a reviewer: dblaikie. dblaikie added a comment. This revision is now accepted and ready to land. Looks great - thanks! http://reviews.llvm.org/D16949 ___ cfe-commits mailing list cfe-commits@lists.llvm

Re: [PATCH] D16949: Fix for: Bug 5941 - improve diagnostic for * vs & confusion

2016-03-01 Thread David Blaikie via cfe-commits
dblaikie added a comment. Are there not any existing cases that test this diagnostic? Could you just update some existing cases to cover this? (it looks like, in the same file, the test case added for PR 6117 could be expanded to test this as well, perhaps?) It looks like that's the original te

Re: [PATCH] D17772: [clang-tidy]: string_view of temporary string

2016-03-01 Thread David Blaikie via cfe-commits
Any way to/would it be worth broadening this with an annotation or some such (or just a lost for now) of types that maintain references to their ctor params? (Eg: llvm arrayref, etc) On Mar 1, 2016 9:19 AM, "Jonathan B Coe via cfe-commits" < cfe-commits@lists.llvm.org> wrote: > jbcoe created this

Re: [PATCH] D17772: [clang-tidy]: string_view of temporary string

2016-03-01 Thread David Blaikie via cfe-commits
dblaikie added a subscriber: dblaikie. dblaikie added a comment. Any way to/would it be worth broadening this with an annotation or some such (or just a lost for now) of types that maintain references to their ctor params? (Eg: llvm arrayref, etc) Repository: rL LLVM http://reviews.llvm.org/D

Re: r262290 - [index] Fix issue where data visitation was disabled with C++ operator call expressions, during indexing.

2016-02-29 Thread David Blaikie via cfe-commits
On Mon, Feb 29, 2016 at 8:34 PM, Argyrios Kyrtzidis wrote: > This is to reduce stack usage, whether it hits stack overflow or not is > highly dependent on configuration. > I've tried forcing smaller stack on the specific test > (test/Index/index-many-call-ops.cpp) but then it can hit stack overfl

Re: r262290 - [index] Fix issue where data visitation was disabled with C++ operator call expressions, during indexing.

2016-02-29 Thread David Blaikie via cfe-commits
Does this change any behavior? (missing test case?) On Mon, Feb 29, 2016 at 6:46 PM, Argyrios Kyrtzidis via cfe-commits < cfe-commits@lists.llvm.org> wrote: > Author: akirtzidis > Date: Mon Feb 29 20:46:32 2016 > New Revision: 262290 > > URL: http://llvm.org/viewvc/llvm-project?rev=262290&view=re

Re: r261774 - Bail on compilation as soon as a job fails.

2016-02-29 Thread David Blaikie via cfe-commits
On Mon, Feb 29, 2016 at 11:19 AM, Justin Lebar via cfe-commits < cfe-commits@lists.llvm.org> wrote: > > Can you expand on this? The idea is that those implicit deps are in > addition to regular dependencies, not a replacement. > > Sure. > > What I was trying to say is, suppose you have two CUDA de

Re: [PATCH] D16949: Fix for: Bug 5941 - improve diagnostic for * vs & confusion

2016-02-25 Thread David Blaikie via cfe-commits
On Tue, Feb 16, 2016 at 10:45 PM, David Blaikie wrote: > > > On Tue, Feb 16, 2016 at 10:01 PM, Ryan Yee via cfe-commits < > cfe-commits@lists.llvm.org> wrote: > >> ryee88 updated this revision to Diff 48149. >> ryee88 added a comment. >> >> Keeping the number of test files to a minimum makes sens

Re: r244989 - Avoid iteration invalidation issues around MaterializedTemporaryExpr

2016-02-25 Thread David Blaikie via cfe-commits
Seems like this test got flagged as 'slow' by Google's internal infrastructure - and that makes me wonder about whether it's appropriate to have in the lit test suite - we really want to keep these tests as fast as possible. I think we're generally OK committing iterator invalidation fixes without

Re: r261674 - Rename Action::begin() to Action::input_begin().

2016-02-23 Thread David Blaikie via cfe-commits
On Tue, Feb 23, 2016 at 1:02 PM, Nico Weber wrote: > On Tue, Feb 23, 2016 at 3:55 PM, David Blaikie wrote: > >> >> >> On Tue, Feb 23, 2016 at 12:46 PM, Nico Weber wrote: >> >>> On Tue, Feb 23, 2016 at 2:44 PM, David Blaikie via cfe-commits

Re: r261674 - Rename Action::begin() to Action::input_begin().

2016-02-23 Thread David Blaikie via cfe-commits
On Tue, Feb 23, 2016 at 12:46 PM, Nico Weber wrote: > On Tue, Feb 23, 2016 at 2:44 PM, David Blaikie via cfe-commits < > cfe-commits@lists.llvm.org> wrote: > >> >> >> On Tue, Feb 23, 2016 at 11:30 AM, Nico Weber via cfe-commits < >> cfe-commits@lists.llv

Re: r261674 - Rename Action::begin() to Action::input_begin().

2016-02-23 Thread David Blaikie via cfe-commits
On Tue, Feb 23, 2016 at 11:30 AM, Nico Weber via cfe-commits < cfe-commits@lists.llvm.org> wrote: > Author: nico > Date: Tue Feb 23 13:30:43 2016 > New Revision: 261674 > > URL: http://llvm.org/viewvc/llvm-project?rev=261674&view=rev > Log: > Rename Action::begin() to Action::input_begin(). > > Al

Re: r261626 - Fix a -Wunused-variable diagnostic.

2016-02-23 Thread David Blaikie via cfe-commits
On Tue, Feb 23, 2016 at 2:29 AM, Alexander Kornienko via cfe-commits < cfe-commits@lists.llvm.org> wrote: > Author: alexfh > Date: Tue Feb 23 04:29:04 2016 > New Revision: 261626 > > URL: http://llvm.org/viewvc/llvm-project?rev=261626&view=rev > Log: > Fix a -Wunused-variable diagnostic. > > Modif

Re: [PATCH] D17434: [Clang-tidy] Make readability-container-size-empty working with STL string

2016-02-19 Thread David Blaikie via cfe-commits
On Fri, Feb 19, 2016 at 11:10 AM, Eugene Zelenko wrote: > On Fri, Feb 19, 2016 at 11:06 AM, David Blaikie > wrote: > > > > Could we take this conversation back to the list? (better to discuss > things > > with everyone) > > > > On Fri, Feb 19, 2016 at 11:02 AM, Eugene Zelenko < > eugene.zele...@

Re: [PATCH] D17434: [Clang-tidy] Make readability-container-size-empty working with STL string

2016-02-19 Thread David Blaikie via cfe-commits
On Fri, Feb 19, 2016 at 10:38 AM, Eugene Zelenko wrote: > On Fri, Feb 19, 2016 at 10:32 AM, David Blaikie > wrote: > > > > > > On Fri, Feb 19, 2016 at 10:25 AM, Eugene Zelenko via cfe-commits > > wrote: > >> > >> Eugene.Zelenko added a comment. > >> > >> Also I agree that testing is good idea,

Re: [PATCH] D17434: [Clang-tidy] Make readability-container-size-empty working with STL string

2016-02-19 Thread David Blaikie via cfe-commits
On Fri, Feb 19, 2016 at 10:25 AM, Eugene Zelenko via cfe-commits < cfe-commits@lists.llvm.org> wrote: > Eugene.Zelenko added a comment. > > Also I agree that testing is good idea, it doesn't make sense in current > incarnation which test only vector and set and only with containers' code > snippet

Re: [PATCH] D17434: [Clang-tidy] Make readability-container-size-empty working with STL string

2016-02-18 Thread David Blaikie via cfe-commits
should probably have test coverage On Thu, Feb 18, 2016 at 6:46 PM, Eugene Zelenko via cfe-commits < cfe-commits@lists.llvm.org> wrote: > Eugene.Zelenko created this revision. > Eugene.Zelenko added reviewers: alexfh, xazax.hun, aaron.ballman. > Eugene.Zelenko added a subscriber: cfe-commits. > E

Re: [PATCH] D17375: Add parentheses around arithmetic in operand of '|' in llvm-dwp.cpp.

2016-02-18 Thread David Blaikie via cfe-commits
Thanks all! Which compiler flagged this? Wonder if/why Clang didn't flag it for me? On Thu, Feb 18, 2016 at 5:27 AM, Phabricator via cfe-commits < cfe-commits@lists.llvm.org> wrote: > This revision was automatically updated to reflect the committed changes. > Closed by commit rL261207: Add parent

Re: [PATCH] D16949: Fix for: Bug 5941 - improve diagnostic for * vs & confusion

2016-02-16 Thread David Blaikie via cfe-commits
On Tue, Feb 16, 2016 at 10:01 PM, Ryan Yee via cfe-commits < cfe-commits@lists.llvm.org> wrote: > ryee88 updated this revision to Diff 48149. > ryee88 added a comment. > > Keeping the number of test files to a minimum makes sense. > > Couldn't find an existing test for this diagnostic. That woul

Re: [PATCH] D16376: clang-tidy check: misc-deprecated-special-member-functions

2016-02-16 Thread David Blaikie via cfe-commits
Is this anything more than the -Wdeprecated warning? (could we split out the -Wdeprecated warning that deals with the deprecated implicit special member generation, then just use that warning for this clang-tidy check?) On Thu, Feb 11, 2016 at 3:16 PM, Jonathan B Coe via cfe-commits < cfe-commits@

Re: [PATCH] D15977: [Clang] Supporting all entities declared in lexical scope in LLVM debug info

2016-02-16 Thread David Blaikie via cfe-commits
dblaikie added a comment. Thanks - please commit when ready http://reviews.llvm.org/D15977 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D16949: Fix for: Bug 5941 - improve diagnostic for * vs & confusion

2016-02-16 Thread David Blaikie via cfe-commits
Since this is just a wording change, presumably the diagnostic is already tested in an existing file - perhaps you could exetendi that test to cover this functionality (we try not to add new test files too much - better to test functionality once/in one place as much as possible (both for ease of r

Re: [clang-tools-extra] r260532 - Merge branch 'arcpatch-D16922'

2016-02-11 Thread David Blaikie via cfe-commits
On Thu, Feb 11, 2016 at 8:03 AM, Cong Liu via cfe-commits < cfe-commits@lists.llvm.org> wrote: > Author: congliu > Date: Thu Feb 11 10:03:27 2016 > New Revision: 260532 > > URL: http://llvm.org/viewvc/llvm-project?rev=260532&view=rev > Log: > Merge branch 'arcpatch-D16922' > Please be sure to che

Re: [PATCH] D16949: Fix for: Bug 5941 - improve diagnostic for * vs & confusion

2016-02-11 Thread David Blaikie via cfe-commits
dblaikie added a subscriber: dblaikie. dblaikie added a comment. Needs test coverage http://reviews.llvm.org/D16949 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D17021: Adding doxygen comments to the LLVM intrinsics (part 5, f16cintrin.h)

2016-02-09 Thread David Blaikie via cfe-commits
It's best not to commit things without approval once they've been sent for review (the assumption being that if you asked for review it's because the change needed review - time doesn't change that fact) - if approval was given off-list (eg: on IRC) it's best to mention who gave it & where (& ideal

Re: [PATCH] D16947: [PGO] assignment operator does not get profile data

2016-02-09 Thread David Blaikie via cfe-commits
On Tue, Feb 9, 2016 at 12:07 PM, David Li via llvm-commits < llvm-comm...@lists.llvm.org> wrote: > This revision was automatically updated to reflect the committed changes. > Closed by commit rL260270: [PGO] Fix issue: explicitly defaulted assignop > is not profiled (authored by davidxl). > In ge

Re: [PATCH] D16947: [PGO] assignment operator does not get profile data

2016-02-09 Thread David Blaikie via cfe-commits
dblaikie added inline comments. Comment at: cfe/trunk/test/Profile/def-assignop.cpp:27 @@ +26,3 @@ + +int main() { + A a1, a2; This doesn't need to be main or have an int return. Just make it a void function (with some generic name) & drop the "return 0" to keep

Re: [PATCH] D16947: [PGO] assignment operator does not get profile data

2016-02-09 Thread David Blaikie via cfe-commits
On Tue, Feb 9, 2016 at 11:41 AM, Xinliang David Li wrote: > On Tue, Feb 9, 2016 at 11:30 AM, David Blaikie wrote: > > > > > > On Tue, Feb 9, 2016 at 11:26 AM, Xinliang David Li > > wrote: > >> > >> On Tue, Feb 9, 2016 at 11:14 AM, David Blaikie > wrote: > >> > > >> > > >> > On Mon, Feb 8, 2016

Re: [PATCH] D16947: [PGO] assignment operator does not get profile data

2016-02-09 Thread David Blaikie via cfe-commits
On Tue, Feb 9, 2016 at 11:26 AM, Xinliang David Li wrote: > On Tue, Feb 9, 2016 at 11:14 AM, David Blaikie wrote: > > > > > > On Mon, Feb 8, 2016 at 9:23 PM, Xinliang David Li > > wrote: > >> > >> Wrong in the sense the the coverage result for the default operators > >> (the line where they are

Re: [PATCH] D16947: [PGO] assignment operator does not get profile data

2016-02-09 Thread David Blaikie via cfe-commits
On Mon, Feb 8, 2016 at 9:23 PM, Xinliang David Li wrote: > Wrong in the sense the the coverage result for the default operators > (the line where they are declared) is marked as if they are not called > which can be confusing to the user. > Presumably a user would have the same problem with impl

r260246 - Simplify EnterTokenStream API to make it more robust for memory management

2016-02-09 Thread David Blaikie via cfe-commits
Author: dblaikie Date: Tue Feb 9 12:52:09 2016 New Revision: 260246 URL: http://llvm.org/viewvc/llvm-project?rev=260246&view=rev Log: Simplify EnterTokenStream API to make it more robust for memory management While this won't help fix things like the bug that r260219 addressed, it seems like goo

Re: [PATCH] D16947: [PGO] assignment operator does not get profile data

2016-02-08 Thread David Blaikie via cfe-commits
On Mon, Feb 8, 2016 at 9:00 PM, Xinliang David Li wrote: > On Mon, Feb 8, 2016 at 8:46 PM, David Blaikie wrote: > > > > On Mon, Feb 8, 2016 at 7:39 PM, Xinliang David Li > > wrote: > >> > >> I took a look at the problem. The implicitly defaulted operators > >> should not be instrumented as spec

Re: [PATCH] D16947: [PGO] assignment operator does not get profile data

2016-02-08 Thread David Blaikie via cfe-commits
On Mon, Feb 8, 2016 at 7:39 PM, Xinliang David Li wrote: > I took a look at the problem. The implicitly defaulted operators > should not be instrumented as specified -- I actually I just added the > new test case for that (checking profile counter not generated) right > after my previous reply an

Re: r260048 - [Frontend] Make the memory management of FrontendAction pointers explicit by using unique_ptr.

2016-02-08 Thread David Blaikie via cfe-commits
FWIW, I tried to do something like this, perhaps with some other improvements, a few years ago. Not sure if things have changed for the better since then, but maybe those old patches may provide some insight/other improvements/options: http://lists.llvm.org/pipermail/cfe-commits/Week-of-Mon-201409

Re: [PATCH] D16947: [PGO] assignment operator does not get profile data

2016-02-08 Thread David Blaikie via cfe-commits
On Mon, Feb 8, 2016 at 5:05 PM, Xinliang David Li wrote: > ha! somehow I kept thinking you are referring to implicit declared ctors. > Ah, glad we figured out the disconnect - thanks for bearing with me! > > From your test case, it is seems that the implicit copy/move op is > also broken and i

Re: [PATCH] D16947: [PGO] assignment operator does not get profile data

2016-02-08 Thread David Blaikie via cfe-commits
On Mon, Feb 8, 2016 at 4:31 PM, Xinliang David Li wrote: > On Mon, Feb 8, 2016 at 4:05 PM, David Blaikie wrote: > > > > > > On Mon, Feb 8, 2016 at 3:58 PM, Xinliang David Li > > wrote: > >> > >> To be clear, you are suggesting breaking the test into two (one for > >> copy, and one for move) ? I

Re: [PATCH] D16947: [PGO] assignment operator does not get profile data

2016-02-08 Thread David Blaikie via cfe-commits
On Mon, Feb 8, 2016 at 3:58 PM, Xinliang David Li wrote: > To be clear, you are suggesting breaking the test into two (one for > copy, and one for move) ? I am totally fine with that. Nah, no need to split the test case - we try to keep the number of test files down (& group related tests into

Re: [PATCH] D16947: [PGO] assignment operator does not get profile data

2016-02-08 Thread David Blaikie via cfe-commits
On Mon, Feb 8, 2016 at 3:46 PM, Xinliang David Li wrote: > On Mon, Feb 8, 2016 at 3:35 PM, David Blaikie wrote: > > > > > > On Mon, Feb 8, 2016 at 3:21 PM, Xinliang David Li > > wrote: > >> > >> On Mon, Feb 8, 2016 at 3:17 PM, David Blaikie > wrote: > >> > > >> > > >> > On Mon, Feb 8, 2016 at

Re: [PATCH] D16947: [PGO] assignment operator does not get profile data

2016-02-08 Thread David Blaikie via cfe-commits
On Mon, Feb 8, 2016 at 3:21 PM, Xinliang David Li wrote: > On Mon, Feb 8, 2016 at 3:17 PM, David Blaikie wrote: > > > > > > On Mon, Feb 8, 2016 at 12:07 PM, Xinliang David Li > > wrote: > >> > >> On Mon, Feb 8, 2016 at 11:39 AM, David Blaikie > wrote: > >> > > >> > > >> > On Mon, Feb 8, 2016 a

Re: [PATCH] D16947: [PGO] assignment operator does not get profile data

2016-02-08 Thread David Blaikie via cfe-commits
On Mon, Feb 8, 2016 at 12:07 PM, Xinliang David Li wrote: > On Mon, Feb 8, 2016 at 11:39 AM, David Blaikie wrote: > > > > > > On Mon, Feb 8, 2016 at 9:25 AM, David Li via llvm-commits > > wrote: > >> > >> davidxl updated this revision to Diff 47217. > >> davidxl added a comment. > >> > >> Simpl

Re: r260058 - Make nozlibcompress.c pass and reenable it.

2016-02-08 Thread David Blaikie via cfe-commits
Thanks for fixing this! On Sun, Feb 7, 2016 at 1:32 PM, Nico Weber via cfe-commits < cfe-commits@lists.llvm.org> wrote: > Author: nico > Date: Sun Feb 7 15:32:17 2016 > New Revision: 260058 > > URL: http://llvm.org/viewvc/llvm-project?rev=260058&view=rev > Log: > Make nozlibcompress.c pass and r

Re: [PATCH] D16947: [PGO] assignment operator does not get profile data

2016-02-08 Thread David Blaikie via cfe-commits
ah, right, sorry about that - gmail didn't render cfe-commits on the to line in the first email... weird. Anyway, no need to include llvm-commits on clang-only changes. On Mon, Feb 8, 2016 at 11:30 AM, Xinliang David Li wrote: > Both cfe-commits and llvm-commits are cc'ed. > > David > > On Mon,

Re: [PATCH] D16947: [PGO] assignment operator does not get profile data

2016-02-08 Thread David Blaikie via cfe-commits
On Mon, Feb 8, 2016 at 9:25 AM, David Li via llvm-commits < llvm-comm...@lists.llvm.org> wrote: > davidxl updated this revision to Diff 47217. > davidxl added a comment. > > Simplified test case suggested by Vedant. > > > http://reviews.llvm.org/D16947 > > Files: > lib/CodeGen/CGClass.cpp > te

Re: [PATCH] D16947: [PGO] assignment operator does not get profile data

2016-02-08 Thread David Blaikie via cfe-commits
This looks like a change to clang - could you test it in clang (& review it on cfe-commits instead of llvm-commits)? On Sat, Feb 6, 2016 at 11:57 AM, David Li via cfe-commits < cfe-commits@lists.llvm.org> wrote: > davidxl created this revision. > davidxl added a reviewer: vsk. > davidxl added sub

Re: r260126 - Simplify test cases

2016-02-08 Thread David Blaikie via cfe-commits
On Mon, Feb 8, 2016 at 11:14 AM, Xinliang David Li via cfe-commits < cfe-commits@lists.llvm.org> wrote: > Author: davidxl > Date: Mon Feb 8 13:14:14 2016 > New Revision: 260126 > > URL: http://llvm.org/viewvc/llvm-project?rev=260126&view=rev > Log: > Simplify test cases > It's handy to mention t

Re: [PATCH] D16376: clang-tidy check: rule-of-five

2016-02-03 Thread David Blaikie via cfe-commits
On Wed, Feb 3, 2016 at 1:40 PM, Aaron Ballman wrote: > On Wed, Feb 3, 2016 at 4:13 PM, David Blaikie wrote: > > > > > > On Wed, Feb 3, 2016 at 1:03 PM, Jonathan Coe wrote: > >> > >> > >> > >> On 3 February 2016 at 18:44, David Blaikie wrote: > >>> > >>> > >>> > >>> On Wed, Feb 3, 2016 at 10:23

Re: [PATCH] D16376: clang-tidy check: rule-of-five

2016-02-03 Thread David Blaikie via cfe-commits
On Wed, Feb 3, 2016 at 1:03 PM, Jonathan Coe wrote: > > > On 3 February 2016 at 18:44, David Blaikie wrote: > >> >> >> On Wed, Feb 3, 2016 at 10:23 AM, Jonathan Coe wrote: >> >>> All the C++ compilers I have tried using (GCC,Clang,MSVC) will generate >>> assignment operators even if the user de

Re: [PATCH] D16376: clang-tidy check: rule-of-five

2016-02-03 Thread David Blaikie via cfe-commits
On Wed, Feb 3, 2016 at 10:23 AM, Jonathan Coe wrote: > All the C++ compilers I have tried using (GCC,Clang,MSVC) will generate > assignment operators even if the user defines a copy-constructor. This is > the behaviour I set out to write a check for. > > The cpp core guide lines recommend definin

Re: [PATCH] D16376: clang-tidy check: rule-of-five

2016-02-03 Thread David Blaikie via cfe-commits
Is this really that useful of a rule? The language does the right thing for the most part already (you don't need to explicitly delete them - they're implicitly deleted if you define any others - except for backcompat with C++98, but those cases are deprecated & we should probably split out the war

Re: [PATCH] D15977: [Clang] Supporting all entities declared in lexical scope in LLVM debug info

2016-02-02 Thread David Blaikie via cfe-commits
dblaikie added a comment. Looks good - though you could do that one further simplification to the test case, I think. (removing x/if x) Comment at: test/CodeGenCXX/debug-info-lb.cpp:5 @@ +4,3 @@ + static int bar = 1; + if (x) + { Looks like you could remove

Re: r259507 - Make the remaining headers self-contained.

2016-02-02 Thread David Blaikie via cfe-commits
On Tue, Feb 2, 2016 at 11:11 AM, Rafael Espíndola < cfe-commits@lists.llvm.org> wrote: > Out of curiosity, what technique were you using to find out if the > headers were self-contained? Just "clang -c foo.h"? > Building LLVM & Clang with C++ modules enabled > > Cheers, > Rafael > > > On 2 Febr

Re: [PATCH][modules][PR26237]

2016-01-30 Thread David Blaikie via cfe-commits
On Sat, Jan 30, 2016 at 9:00 AM, Vassil Vassilev wrote: > AFAICT the making a test case independent on STL is the hard part. I think > it will be always failing due to the relocation of the memory region of the > underlying SmallVector. > Sorry, I think I didn't explain myself well. What I mean

Re: [PATCH][modules][PR26237]

2016-01-30 Thread David Blaikie via cfe-commits
Yeah, it's good to have a reproduction, but I wouldn't actually commit one - unless you have a checked stl to test against that always fails on possibly-invalidating sequences of operations, then you'd have a fairly simple reproduction On Jan 30, 2016 8:23 AM, "Vassil Vassilev" wrote: > Sorry. >

Re: [PATCH][modules][PR26237]

2016-01-30 Thread David Blaikie via cfe-commits
It might be handy to give an overview of the issue in the review (& certainly in the commit) message rather than only citing the bug number On Jan 30, 2016 6:49 AM, "Vassil Vassilev via cfe-commits" < cfe-commits@lists.llvm.org> wrote: > Attaching a fix to https://llvm.org/bugs/show_bug.cgi?id=262

Re: [PATCH] D15977: [Clang] Supporting all entities declared in lexical scope in LLVM debug info

2016-01-29 Thread David Blaikie via cfe-commits
dblaikie accepted this revision. dblaikie added a comment. This revision is now accepted and ready to land. Looks good to me. Test case could be simplified a bit further, but feel free to commit after that. Comment at: lib/CodeGen/CGDebugInfo.cpp:1479 @@ +1478,3 @@ +void CGDebu

Re: [PATCH] D8822: Proposed fix for PR23076 (conditional branch debug line info)

2016-01-28 Thread David Blaikie via cfe-commits
Richard Smith: Is there a reasonable way to get the ')' of an "if ()" expression in the AST? Is it just a matter of going down and playing games with source locations/lexing to get the token after the end location of the condition expression? On Thu, Jan 28, 2016 at 4:57 PM, Wolfgang Pieb via cfe-

Re: [PATCH] D16697: Updating .debug_line section version information to match DWARF version.

2016-01-28 Thread David Blaikie via cfe-commits
+Adrian for Apple/LLDB perspective One way to merge the tests would be to add another command line argument into DwarfDebug (see the way the split-dwarf command line argument is used), just for testing purposes. The command line argument can override the value specified in the metadata - so you ca

Re: r259031 - Fix isBeforeInTranslationUnit to not abort on macros defined in cmdline.

2016-01-28 Thread David Blaikie via cfe-commits
Test case? On Thu, Jan 28, 2016 at 1:28 AM, Yury Gribov via cfe-commits < cfe-commits@lists.llvm.org> wrote: > Author: ygribov > Date: Thu Jan 28 03:28:18 2016 > New Revision: 259031 > > URL: http://llvm.org/viewvc/llvm-project?rev=259031&view=rev > Log: > Fix isBeforeInTranslationUnit to not abo

Re: [PATCH] D8822: Proposed fix for PR23076 (conditional branch debug line info)

2016-01-27 Thread David Blaikie via cfe-commits
dblaikie added a comment. Fair question on overloaded operators... Taking the original code: 1: int main() { 2: if ( 3: tr() 4: && 5: tr() 6: ) 7: func(); 8: if ( 9: fa() 10: && 11: tr() 12: ) 13: func(); 14: } & making tr/fa return a user defined type with a

Re: [PATCH] D16572: PR23057: fix use-after-free due to local token buffer in ParseCXXAmbiguousParenExpression

2016-01-26 Thread David Blaikie via cfe-commits
I don't fully understand your explanation. Is your change introducing a memory leak? (or was the old code causing a double free (if EnterTokenStream does take ownership of this argument)in addition to use-after-free?) On Tue, Jan 26, 2016 at 2:45 AM, Dmitry Polukhin via cfe-commits < cfe-commits@l

Re: [PATCH] D16566: [Clang-tidy] Fix Clang-tidy modernize-use-override warning in unittests/clang-tidy/IncludeInserterTest.cpp; other minor fixes

2016-01-26 Thread David Blaikie via cfe-commits
On Mon, Jan 25, 2016 at 6:20 PM, Eugene Zelenko via cfe-commits < cfe-commits@lists.llvm.org> wrote: > Eugene.Zelenko created this revision. > Eugene.Zelenko added reviewers: alexfh, aaron.ballman. > Eugene.Zelenko added a subscriber: cfe-commits. > Eugene.Zelenko set the repository for this revis

Re: r258507 - Module Debugging: Use a nonzero DWO id for precompiled headers.

2016-01-22 Thread David Blaikie via cfe-commits
On Fri, Jan 22, 2016 at 9:43 AM, Adrian Prantl via cfe-commits < cfe-commits@lists.llvm.org> wrote: > Author: adrian > Date: Fri Jan 22 11:43:43 2016 > New Revision: 258507 > > URL: http://llvm.org/viewvc/llvm-project?rev=258507&view=rev > Log: > Module Debugging: Use a nonzero DWO id for precompi

Re: [PATCH] D15977: [Clang] Supporting all entities declared in lexical scope in LLVM debug info

2016-01-20 Thread David Blaikie via cfe-commits
dblaikie added inline comments. Comment at: lib/CodeGen/CGDebugInfo.cpp:1479 @@ +1478,3 @@ +void CGDebugInfo::recordDeclarationLexicalScope(const Decl &D) { + assert(LexicalBlockMap.find(&D) == LexicalBlockMap.end() && + "D is already mapped to lexical block scope");

Re: r258251 - Module Debugging: Don't emit external type references to anonymous types.

2016-01-19 Thread David Blaikie via cfe-commits
On Tue, Jan 19, 2016 at 4:04 PM, Adrian Prantl wrote: > > On Jan 19, 2016, at 3:52 PM, David Blaikie wrote: > > > > On Tue, Jan 19, 2016 at 3:42 PM, Adrian Prantl via cfe-commits < > cfe-commits@lists.llvm.org> wrote: > >> Author: adrian >> Date: Tue Jan 19 17:42:53 2016 >> New Revision: 258251

<    7   8   9   10   11   12   13   14   15   >