Re: [PATCH] D23705: [GraphTraits] Make nodes_iterator dereference to NodeType*

dblaikie accepted this revision. dblaikie added a comment. This revision is now accepted and ready to land. Consider renaming and/or generalizing the handlers. Also, in a follow-up or preliminary commit, could you remove all the DerefFun stuff. Move the definition of CGdereference (if they

Re: [PATCH] D21675: New ODR checker for modules

dblaikie added inline comments. Comment at: lib/AST/DeclBase.cpp:1810-1812 @@ +1809,5 @@ + void VisitNamedDecl(const NamedDecl *D) { +if (IdentifierInfo *II = D->getIdentifier()) { + ID.AddString(II->getName()); +} +Inherited::VisitNamedDecl(D);

Re: [PATCH] D22782: Added 'inline' attribute to __init to inline the basic_string's constructor

On Wed, Aug 10, 2016 at 5:39 PM Eric Fiselier <e...@efcs.ca> wrote: > On Mon, Aug 8, 2016 at 9:32 AM, David Blaikie via cfe-commits < > cfe-commits@lists.llvm.org> wrote: > >> I'm still (as per another similar thread) a bit concerned this is working >> around a

Re: [PATCH] D23130: Add a check for definitions in the global namespace.

On Mon, Aug 8, 2016 at 8:37 AM Benjamin Kramer wrote: > -Wmissing-prototype only warns for functions, I want to catch classes > too. Ah, fair enough. Yeah, a clang-tidy check for things in the global namespace that are in a main file rather than a header could be another

Re: [clang-tools-extra] r277677 - [clang-tidy] Inefficient string operation

On Wed, Aug 3, 2016 at 4:13 PM Alexander Kornienko via cfe-commits < cfe-commits@lists.llvm.org> wrote: > Author: alexfh > Date: Wed Aug 3 18:06:03 2016 > New Revision: 277677 > > URL: http://llvm.org/viewvc/llvm-project?rev=277677=rev > Log: > [clang-tidy] Inefficient string operation > A more

Re: [PATCH] D23130: Add a check for definitions in the global namespace.

This seems to have a lot of overlap with -Wmissing-prototype, really - what do you think of the overlap/distinction between the two? On Wed, Aug 3, 2016 at 1:25 PM Eugene Zelenko via cfe-commits < cfe-commits@lists.llvm.org> wrote: > Eugene.Zelenko added a subscriber: Eugene.Zelenko. >

Re: [PATCH] D22782: Added 'inline' attribute to __init to inline the basic_string's constructor

I'm still (as per another similar thread) a bit concerned this is working around a compiler optimizer bug - I'd love to see a standalone example of this behavior (that adding the inline keyword to an already inline/available definition is what's causing the inlining) so we can look at what the

r277852 - PR26423: Assert on valid use of using declaration of a function with an undeduced auto return type

Author: dblaikie Date: Fri Aug 5 14:03:01 2016 New Revision: 277852 URL: http://llvm.org/viewvc/llvm-project?rev=277852=rev Log: PR26423: Assert on valid use of using declaration of a function with an undeduced auto return type For now just disregard the using declaration in this case.

Re: [PATCH] D22834: Added 'inline' attribute to basic_string's destructor

On Tue, Jul 26, 2016 at 4:37 PM Laxman Sole via cfe-commits < cfe-commits@lists.llvm.org> wrote: > laxmansole created this revision. > laxmansole added reviewers: mclow.lists, howard.hinnant. > laxmansole added subscribers: cfe-commits, sebpop, hiraditya, evandro, > flyingforyou. > > > Currently

Re: r275377 - Use hasFlag instead of hasArg

Ping On Mon, Jul 25, 2016 at 10:24 AM David Blaikie wrote: > Ping? > > On Mon, Jul 18, 2016 at 11:28 AM David Blaikie wrote: > >> What build problem did this cause? Did this just not compile (it looks as >> if ArgList has hasArg and hasFlag, so I'm not

Re: [libcxxabi] r276022 - Attempt to bring peace to -Werror buildbots.

+Richard Trieu <rtr...@google.com> - worth fixing? If anyone does get around to fixing this, would be good to remove the workaround committed here in r276022. On Mon, Jul 25, 2016 at 10:51 AM Richard Smith <rich...@metafoo.co.uk> wrote: > On 25 Jul 2016 6:29 p.m., "David Bla

Re: [libcxxabi] r276022 - Attempt to bring peace to -Werror buildbots.

Should we fix the diagnostic? Or is the code triggering it just esoteric enough to not be a good justification for changing the warning? On Tue, Jul 19, 2016 at 1:42 PM Richard Smith via cfe-commits < cfe-commits@lists.llvm.org> wrote: > Author: rsmith > Date: Tue Jul 19 15:35:09 2016 > New

Re: r275377 - Use hasFlag instead of hasArg

Ping? On Mon, Jul 18, 2016 at 11:28 AM David Blaikie wrote: > What build problem did this cause? Did this just not compile (it looks as > if ArgList has hasArg and hasFlag, so I'm not sure what the specific > problem might've been) > > On Wed, Jul 13, 2016 at 11:45 PM Dean

Re: r276317 - Reroll "Include unreferenced nested types in member list only for CodeView"

It'd be handy to include details of what's different about this commit than the previous time it was committed - makes review easier by focusing on the new changes. (for some commits where I've recommitted them a few times I end up with a bit of a timeline/history in the commit message - "fixed

Re: r275377 - Use hasFlag instead of hasArg

What build problem did this cause? Did this just not compile (it looks as if ArgList has hasArg and hasFlag, so I'm not sure what the specific problem might've been) On Wed, Jul 13, 2016 at 11:45 PM Dean Michael Berris via cfe-commits < cfe-commits@lists.llvm.org> wrote: > Author: dberris >

Re: r275040 - [CodeGen] Treat imported static local variables as declarations

On Tue, Jul 12, 2016 at 3:51 PM David Majnemer via cfe-commits < cfe-commits@lists.llvm.org> wrote: > On Tue, Jul 12, 2016 at 2:55 PM, Robinson, Paul > wrote: > >> A declaration that gets used within the CU generally does get a >> debug-info description. >> > > It does

Re: r274246 - [codeview] Emit qualified display names if -gline-tables-only is on

On Fri, Jul 8, 2016 at 1:04 PM, Nico Weber <tha...@chromium.org> wrote: > On Fri, Jul 8, 2016 at 3:57 PM, David Blaikie via cfe-commits < > cfe-commits@lists.llvm.org> wrote: > >> >> >> On Thu, Jul 7, 2016 at 4:10 PM, Reid Kleckner <r...@google.com>

Re: r274246 - [codeview] Emit qualified display names if -gline-tables-only is on

On Thu, Jul 7, 2016 at 4:10 PM, Reid Kleckner wrote: > On Thu, Jul 7, 2016 at 3:45 PM, David Blaikie wrote: > >> Yeah - is this necessary for CodeView? (does something break, or do you >> just get simple names where you'd prefer full mangled or qualified

Re: r274628 - Include debug info for nested structs and classes

On Thu, Jul 7, 2016 at 4:22 PM, Adrian McCarthy wrote: > This patch was reverted because it breaks a test on the buildbots (that > I've been unable to reproduce locally), so that's why you didn't seen > anything. I'll try again to land the patch once I can fix and verify

Re: r274677 - [CodeGen, DebugInfo] Use hasLocalLinkage instead of hasInternalLinkage

On Wed, Jul 6, 2016 at 2:07 PM, David Majnemer via cfe-commits < cfe-commits@lists.llvm.org> wrote: > Author: majnemer > Date: Wed Jul 6 16:07:53 2016 > New Revision: 274677 > > URL: http://llvm.org/viewvc/llvm-project?rev=274677=rev > Log: > [CodeGen, DebugInfo] Use hasLocalLinkage instead of

Re: r274628 - Include debug info for nested structs and classes

Ah, I see why I might not've seen the result - it was reverted & I didn't spot the revert in a quick search for r274628 (because it was quoted as rL because Phab is weird and likes to put an "L" in there). On Thu, Jul 7, 2016 at 4:03 PM, David Blaikie wrote: > This may

Re: r274628 - Include debug info for nested structs and classes

This may cause problems for DWARF type unit consistency... Under what conditions do nested types appear in the member list? (my simple test case on a fresh clang didn't seem to produce anything about the nested type: struct outer { struct inner { int i; }; int j; }; outer o; ) On Wed, Jul 6,

Re: r274246 - [codeview] Emit qualified display names if -gline-tables-only is on

Yeah - is this necessary for CodeView? (does something break, or do you just get simple names where you'd prefer full mangled or qualified names) It was chosen pretty deliberately to do it this way (use unqualified names) for gmlt to keep size down. Have you got numbers for the size delta for

Re: r272867 - [Lex] Try to fix a 'comparison is always false' warning. NFC.

ah, rightio - thanks for the reminder! On Thu, Jun 16, 2016 at 10:44 AM, George Burgess IV < george.burgess...@gmail.com> wrote: > I don't think so -- 4.7p2 says "If the destination type is unsigned, the > resulting value is the least unsigned integer congruent to the source > integer (modulo 2

Re: r272867 - [Lex] Try to fix a 'comparison is always false' warning. NFC.

I can't remember the rules - but wouldn't this produce UB on a signed char platform in the case where Ch is negative? (or is that just unspecified? implementation defined?) On Wed, Jun 15, 2016 at 7:30 PM, George Burgess IV via cfe-commits < cfe-commits@lists.llvm.org> wrote: > Author: gbiv >

Re: [PATCH] D19754: Allow 'nodebug' on local variables

dblaikie accepted this revision. dblaikie added a comment. Sure, looks good - thanks. Will discuss the broader test issues later/separately. http://reviews.llvm.org/D19754 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

Re: r272253 - clang/test/CodeGenCXX/debug-info-method.cpp: Tweak for thiscall, for targeting Win32 x86.

On Wed, Jun 15, 2016 at 10:59 AM, Reid Kleckner wrote: > On Thu, Jun 9, 2016 at 8:59 AM, David Blaikie wrote: > >> Reid - is this intended fallout? (seems plausible, but just checking) >> >> Is MinGW a good analogy for any of this work? (does it produce

Re: [PATCH] D21256: Improved Visual Studio 2015 visualization of SmallVectorImpl

On Mon, Jun 13, 2016 at 8:55 AM, Michael Spertus wrote: > Hi David, > While I understand the initial reasoning. I have found that this is like a > hundred times better for working on Clang in practice and can't imagine > working without it. The point is that many Clang data

Re: [PATCH] D21256: Improved Visual Studio 2015 visualization of SmallVectorImpl

As for the original change proposed: My guiding principle would be "do whatever std::vector does". (& that's what I did when implementing GDB pretty printers for SmallVector/SmallString/ArrayRef, etc... ) An aside: We generally don't do time limited reviews like this. Either something needs

Re: r245719 - Properly provide alignment of 'byval' arguments down to llvm.

That was certainly one of the counterarguments (& global variables also use a type rather than a size). I've not really settled on which way to go/haven't given it lots of thought. I may loop back around to the original thread when it comes to that. On Fri, Jun 10, 2016 at 2:18 PM, James Y Knight

Re: r245719 - Properly provide alignment of 'byval' arguments down to llvm.

Excuse the necromancy, but do you know if this change (or other work you did in this area) completely eclipsed LLVM's use of inferred alignment via the llvm struct's alignment for byval arguments? I ask because this was something I was going to need to fix for the typeless pointer work & I have

Re: r272253 - clang/test/CodeGenCXX/debug-info-method.cpp: Tweak for thiscall, for targeting Win32 x86.

Reid - is this intended fallout? (seems plausible, but just checking) Is MinGW a good analogy for any of this work? (does it produce DWARF? Does it use the Windows ABI? Does it emit Calling Convention attributes?) On Thu, Jun 9, 2016 at 3:06 AM, NAKAMURA Takumi via cfe-commits <

Re: r272198 - [DebugInfo] Add calling conventions to DISubroutineType

At least in theory it'd be nice to have test cases for the other call sites this change adds CC to. On Wed, Jun 8, 2016 at 1:41 PM, Reid Kleckner via cfe-commits < cfe-commits@lists.llvm.org> wrote: > Author: rnk > Date: Wed Jun 8 15:41:54 2016 > New Revision: 272198 > > URL:

Re: [PATCH] D20964: [clang-tidy] Add modernize-use-emplace

dblaikie added a subscriber: dblaikie. dblaikie added a comment. Could you describe in more detail (ideally in the original patch review summary/description) what this transformation does? Under what conditions does it suggest emplace, etc. http://reviews.llvm.org/D20964

Re: r253909 - Make clang_Cursor_getMangling not mangle if the declaration isn't mangled

Burt Wesarg points out on cfe-dev that this commit message doesn't match the patch (nor the description provided in the code review thread that lead to this commit) - this one might be worth reverting and recommitting with a more accurate commit message (I don't usually suggest this for most

Re: r213213 - DebugInfo: Forward HandleTagDeclRequiredDefinition through MultiplexConsumer to fix debug info emission in the presence of plugins.

Thanks for the catch! Fixed in r271188 On Wed, May 25, 2016 at 10:28 AM, Nico Weber wrote: > Zombie review comment: ".test" isn't part of > test/lit.cfg::config.suffixes, so the two .test files added in this change > never run unless you explicitly run them (e.g. with

r271188 - Enable some accidentally dead tests and fix up the bitrot

Author: dblaikie Date: Sun May 29 14:50:23 2016 New Revision: 271188 URL: http://llvm.org/viewvc/llvm-project?rev=271188=rev Log: Enable some accidentally dead tests and fix up the bitrot Problem found by Nico, originally committed by me in r213213. The .test prefix wasn't actually being run.

Re: [PATCH] D20660: Remove `auto_ptr` in C++17.

ah, indeed :/ On Fri, May 27, 2016 at 10:55 AM, Eric Fiselier wrote: > > As for tests - XFAILing seems a bit general when there's really not > much value in running any of the tests anyway. REQUIRES, perhaps? > > Unfortunately REQUIRES is a conjunction so the obvious "// REQUIRES:

Re: [PATCH] D20660: Remove `auto_ptr` in C++17.

NO_REMOVE seems like a strange way of saying it - is there existing precedent for that naming/description? (rather than something like _LIBCPP_PROVIDE_AUTOPTR ? As for tests - XFAILing seems a bit general when there's really not much value in running any of the tests anyway. REQUIRES, perhaps?

Re: r270164 - Avoid depending on test inputes that aren't in Inputs

I'm not sure, but assume the Inputs directories could be sunk to a common parent (test/Inputs rather than test/CodeGen/Inputs, etc) On Thu, May 19, 2016 at 5:38 PM, Reid Kleckner via cfe-commits < cfe-commits@lists.llvm.org> wrote: > Author: rnk > Date: Thu May 19 19:38:25 2016 > New Revision:

Re: [PATCH] D20349: Fix a clang bug in lambda capture of 'this'

The patch'll need a test case (in clang/tests) - I've not looked at the change/have much opinion there, just suggesting adding a test at minimum so when someone does come to review it it's more complete/closer to/ready to commit. On Tue, May 17, 2016 at 9:23 PM, Taewook Oh via cfe-commits <

Re: [PATCH] D19754: Allow 'nodebug' on local variables

On Tue, May 17, 2016 at 2:30 PM, Robinson, Paul wrote: > What you are describing is what testing literature refers to as criteria > for equivalence classes. There is some level of judgment to that, yes. > > > > Yep yep, to be sure. I'm just generally trying to encourage

Re: [PATCH] D19567: PR21823: 'nodebug' attribute on global/static variables

ping On Mon, May 2, 2016 at 11:51 AM, David Blaikie via cfe-commits < cfe-commits@lists.llvm.org> wrote: > dblaikie added a comment. > > In http://reviews.llvm.org/D19567#414906, @probinson wrote: > > > Huh. There are strange interactions here, which makes me even more

Re: [libcxx] r269789 - Implement LWG2576: istream_iterator and ostream_iterator should use std::addressof

ssed > offline about how not all parts of this change are testable (and hence not > meaningful) but that's what the standard says to do. > > > On Tue, May 17, 2016 at 1:51 PM, David Blaikie via cfe-commits < > cfe-commits@lists.llvm.org> wrote: > >> Test coverage? >

Re: [libcxx] r269789 - Implement LWG2576: istream_iterator and ostream_iterator should use std::addressof

Test coverage? On Tue, May 17, 2016 at 10:44 AM, Marshall Clow via cfe-commits < cfe-commits@lists.llvm.org> wrote: > Author: marshall > Date: Tue May 17 12:44:40 2016 > New Revision: 269789 > > URL: http://llvm.org/viewvc/llvm-project?rev=269789=rev > Log: > Implement LWG2576: istream_iterator

Re: r269717 - Switch from SmallVector to TinyPtrVector for the list of attributes on a declaration. This removes a memory allocation for the common case where the declaration has only one attribute.

On Mon, May 16, 2016 at 3:53 PM, Richard Smith via cfe-commits < cfe-commits@lists.llvm.org> wrote: > Author: rsmith > Date: Mon May 16 17:53:19 2016 > New Revision: 269717 > > URL: http://llvm.org/viewvc/llvm-project?rev=269717=rev > Log: > Switch from SmallVector to TinyPtrVector for the list

Re: [PATCH] D19708: [CGDebugInfo] Generate debug info for member calls in the context of the callee expression

On Mon, May 2, 2016 at 1:17 PM, Hal Finkel wrote: > - Original Message - > > From: "David Blaikie" > > To: reviews+d19708+public+e9ddc42503732...@reviews.llvm.org, "Hal > Finkel" > > Cc: "Richard Smith" ,

Re: [PATCH] D20141: Check for nullptr argument.

If you have a check that doesn't have a test/is never triggered - simple thing to do is just make it an assert & run some testing of your own (selfhost, etc) then if there's insufficient evidence that it's needed, ship it and wait until it fails on a buildbot, etc. Then reduce the test case and

Re: [PATCH] D20040: Treat qualifiers on elaborated types for qualtypenames appropriately.

dblaikie added a subscriber: dblaikie. Comment at: unittests/Tooling/QualTypeNamesTest.cpp:91 @@ -90,2 +90,3 @@ Visitor.ExpectedQualTypeNames["AliasTypeVal"] = "A::B::C::InnerAlias"; + Visitor.ExpectedQualTypeNames["CheckM"] = "const A::B::Class0 *"; Visitor.runOver(

Re: [PATCH] D19754: Allow 'nodebug' on local variables

On Thu, May 5, 2016 at 8:50 AM, Robinson, Paul wrote: > This would be a great conversation to have at the social, sadly I will > have to miss it this month. > Yeah, I don't often make it along to them, unfortunately. > > >> dblaikie wrote: > >>> Doesn't look like the

Re: [PATCH] D19962: [scan-build] fix warnings emitted on Clang StaticAnalyzer code base

dblaikie added a subscriber: dblaikie. Comment at: lib/StaticAnalyzer/Core/PlistDiagnostics.cpp:300 @@ -299,2 +299,3 @@ + assert(SM && "SourceManager is NULL, cannot iterate through the diagnostics"); This assertion seems to be equivalent to replacing the

Re: [PATCH] D19959: [scan-build] fix warning emitted on Clang Driver code baseFix a "logic error" warning of the type Called c++ object pointer isnull" reported by Clang Static Analyzer on the file:-

Should this be a test, or just an assertion? On Thu, May 5, 2016 at 2:34 AM, Apelete Seketeli via cfe-commits < cfe-commits@lists.llvm.org> wrote: > apelete created this revision. > apelete added reviewers: kevin.qin, rsmith. > apelete added a subscriber: cfe-commits. > > Signed-off-by: Apelete

Re: [PATCH] D19960: [scan-build] fix warnings emitted on Clang CodeGen code base

dblaikie added inline comments. Comment at: lib/CodeGen/CGDebugInfo.cpp:1317 @@ -1316,2 +1316,3 @@ } + assert(V && "constant must be not NULL at this point"); TemplateParams.push_back(DBuilder.createTemplateValueParameter( It looks like this

Re: [PATCH] D19084: [scan-build] fix warnings emitted on Clang AST code base

dblaikie added a subscriber: dblaikie. Comment at: lib/AST/ASTDiagnostic.cpp:1686 @@ -1685,3 +1685,3 @@ -if (Same) { +if (Same && FromTD) { OS << "template " << FromTD->getNameAsString(); Should this be a condition, or just an assertion?

Re: [PATCH] D19754: Allow 'nodebug' on local variables

On Tue, May 3, 2016 at 4:38 PM, Paul Robinson via cfe-commits < cfe-commits@lists.llvm.org> wrote: > probinson marked 2 inline comments as done. > > > Comment at: include/clang/Basic/Attr.td:86-88 > @@ -85,1 +85,5 @@ > +def NonParmVar : SubsetSubject +

r268460 - [modules][debuginfo] Only include imported modules when targeting LLDB

Author: dblaikie Date: Tue May 3 18:06:40 2016 New Revision: 268460 URL: http://llvm.org/viewvc/llvm-project?rev=268460=rev Log: [modules][debuginfo] Only include imported modules when targeting LLDB These constructs are only applicable to a debugger capable of loading a Clang AST, so omit them

Re: [PATCH] D19831: [scan-build] fix dead store warnings emitted on clang code base

This revision was automatically updated to reflect the committed changes. Closed by commit rL268453: [scan-build] fix dead store warnings emitted on clang code base (authored by dblaikie). Changed prior to commit: http://reviews.llvm.org/D19831?vs=56059=56071#toc Repository: rL LLVM

r268453 - [scan-build] fix dead store warnings emitted on clang code base

Author: dblaikie Date: Tue May 3 17:14:14 2016 New Revision: 268453 URL: http://llvm.org/viewvc/llvm-project?rev=268453=rev Log: [scan-build] fix dead store warnings emitted on clang code base This fixes dead store warnings of the type "dead assignment" reported by CLang Static Analyzer on the

Re: [PATCH] D19831: [scan-build] fix dead store warnings emitted on clang code base

Thanks - do you need someone (me) to commit this, or do you have commit access? On Tue, May 3, 2016 at 1:32 PM, Apelete Seketeli via cfe-commits < cfe-commits@lists.llvm.org> wrote: > apelete updated this revision to Diff 56059. > apelete added a comment. > > [scan-build] fix dead store warnings

Re: [PATCH] D19831: [scan-build] fix dead store warnings emitted on clang code base

dblaikie accepted this revision. dblaikie added a reviewer: dblaikie. This revision is now accepted and ready to land. Comment at: tools/c-index-test/c-index-test.c:1435-1436 @@ -1434,3 +1434,4 @@ /* recurse to get the first parent record that is not anonymous. */ -

Re: [PATCH] D19831: [scan-build] fix dead store warnings emitted on clang code base

Looks good to me - go ahead & commit whenever you're ready. On Mon, May 2, 2016 at 11:40 PM, Apelete Seketeli via cfe-commits < cfe-commits@lists.llvm.org> wrote: > apelete updated this revision to Diff 55952. > apelete added a comment. > > [scan-build] fix dead store warnings emitted on clang

Re: [PATCH] D19831: [scan-build] fix dead store warnings emitted on clang code base

Any reason not to remove the story instead? On Mon, May 2, 2016 at 1:36 PM, Apelete Seketeli via cfe-commits < cfe-commits@lists.llvm.org> wrote: > apelete created this revision. > apelete added a reviewer: akyrtzi. > apelete added a subscriber: cfe-commits. > > This fixes dead store warnings of

Re: [PATCH] D19754: Allow 'nodebug' on local variables

dblaikie added inline comments. Comment at: test/CodeGenCXX/debug-info-nodebug.cpp:50 @@ -49,1 +49,3 @@ NODEBUG static int static_local = 6; + NODEBUG const int const_local = 7; + NODEBUGint normal_local = 8; Doesn't look like the const case is any

Re: [PATCH] D19567: PR21823: 'nodebug' attribute on global/static variables

dblaikie added a comment. In http://reviews.llvm.org/D19567#414906, @probinson wrote: > Huh. There are strange interactions here, which makes me even more nervous > about testing fewer cases. Generally this sort of thing makes me more interested in testing fewer cases so we can see/make

Re: [PATCH] D19625: [libc++] Void-cast runtime-unused variables.

On Fri, Apr 29, 2016 at 4:59 PM, Stephan T. Lavavej < s...@exchange.microsoft.com> wrote: > [David Blaikie] > > Unused-variable seems pretty low value. > > Yeah, but it still has the potential to detect mistakes (e.g. typos, or > code you intended to write but forgot about). > > I figured I'd

Re: [PATCH] D19625: [libc++] Void-cast runtime-unused variables.

On Fri, Apr 29, 2016 at 4:34 PM, Stephan T. Lavavej via cfe-commits < cfe-commits@lists.llvm.org> wrote: > STL_MSFT marked 2 inline comments as done. > STL_MSFT added a comment. > > What I'm doing is running libcxx's tests against MSVC's compiler and > libraries. I could aggressively suppress

Re: [PATCH] D19625: [libc++] Void-cast runtime-unused variables.

I'm not sure why we'd want to compile the test suite with -Wunused-variable (& even if we did, I imagine Clang's doesn't fire here because the variables are used, just in non-evaluated contexts?)? Is there a benefit to it being clean of unused-variable warnings? (in Clang's test case we just

Re: [PATCH] D19708: [CGDebugInfo] Generate debug info for member calls in the context of the callee expression

You could simplify the test case further, down to just: struct foo { void bar(); }; void f(foo *f) { f->bar(); } and check that the call instruction has the desired column (or if you want a test that doesn't depend on column info (you can force it on with a flag, but we might vary whether it's

Re: r267904 - Debug info: Apply an artificial debug location to __cyg_profile_func.* calls.

On Thu, Apr 28, 2016 at 4:42 PM, Adrian Prantl wrote: > > On Apr 28, 2016, at 4:31 PM, David Blaikie wrote: > > > > On Thu, Apr 28, 2016 at 1:11 PM, Adrian Prantl wrote: > >> >> On Apr 28, 2016, at 12:53 PM, David Blaikie

Re: [PATCH] D19708: [CGDebugInfo] Generate debug info for member calls in the context of the callee expression

As mentioned in the bug, I /think/ the right thing to do here is to change the preferred location of the CXXMemberCallExpr so that we improve diagnostics as well. Any place where the preferred location of an expression and the debug location of an expression are differing I'd really like a pretty

Re: [PATCH] D19689: Add Subjects to NoDebugAttr [NFC]

On Thu, Apr 28, 2016 at 4:46 PM, Robinson, Paul wrote: > Generally tests test something other than "this program doesn't crash" - > should it test that we apply the attribute correctly? (either via ast dump, > or checking the resulting DWARF doesn't have debug info on the

Re: r267904 - Debug info: Apply an artificial debug location to __cyg_profile_func.* calls.

On Thu, Apr 28, 2016 at 1:11 PM, Adrian Prantl wrote: > > On Apr 28, 2016, at 12:53 PM, David Blaikie wrote: > > > > On Thu, Apr 28, 2016 at 12:50 PM, Adrian Prantl wrote: > >> >> On Apr 28, 2016, at 12:34 PM, David Blaikie

Re: [PATCH] D19689: Add Subjects to NoDebugAttr [NFC]

LGTM On Thu, Apr 28, 2016 at 2:10 PM, Paul Robinson via cfe-commits < cfe-commits@lists.llvm.org> wrote: > probinson created this revision. > probinson added a reviewer: aaron.ballman. > probinson added a subscriber: cfe-commits. > > The 'nodebug' attribute had hand-coded constraints; replace

Re: r267904 - Debug info: Apply an artificial debug location to __cyg_profile_func.* calls.

On Thu, Apr 28, 2016 at 12:50 PM, Adrian Prantl wrote: > > On Apr 28, 2016, at 12:34 PM, David Blaikie wrote: > > Should these have no/artificial location? It seems like perhaps they > should have the same location as the scope they're for? (well, the >

Re: r267904 - Debug info: Apply an artificial debug location to __cyg_profile_func.* calls.

Should these have no/artificial location? It seems like perhaps they should have the same location as the scope they're for? (well, the beginning or end of that scope, respectively, etc) On Thu, Apr 28, 2016 at 10:21 AM, Adrian Prantl via cfe-commits < cfe-commits@lists.llvm.org> wrote: >

Re: [PATCH] D19567: PR21823: 'nodebug' attribute on global/static variables

On Wed, Apr 27, 2016 at 3:24 PM, Paul Robinson via cfe-commits < cfe-commits@lists.llvm.org> wrote: > probinson added a comment. > > In http://reviews.llvm.org/D19567#413997, @dblaikie wrote: > > > For 3 code paths (that seem fairly independent from one another) I'd > only really expect to see 3

[clang-tools-extra] r267754 - Fix a bunch of sign-compare warnings

Author: dblaikie Date: Wed Apr 27 13:47:45 2016 New Revision: 267754 URL: http://llvm.org/viewvc/llvm-project?rev=267754=rev Log: Fix a bunch of sign-compare warnings Modified: clang-tools-extra/trunk/unittests/include-fixer/find-all-symbols/FindAllSymbolsTests.cpp Modified:

Re: [PATCH] D19567: PR21823: 'nodebug' attribute on global/static variables

dblaikie added a comment. For 3 code paths (that seem fairly independent from one another) I'd only really expect to see 3 variables in the test - one to exercise each codepath. What's the reason for the larger set of test cases? Then it might be simpler just to include 6 variables, one of

Re: r267054 - Split interesting warnings off from -Wfloat-conversion

On Thu, Apr 21, 2016 at 2:04 PM, Richard Trieu via cfe-commits < cfe-commits@lists.llvm.org> wrote: > Author: rtrieu > Date: Thu Apr 21 16:04:55 2016 > New Revision: 267054 > > URL: http://llvm.org/viewvc/llvm-project?rev=267054=rev > Log: > Split interesting warnings off from -Wfloat-conversion

r266224 - libclang: Use early-return to reduce indentation.

Author: dblaikie Date: Wed Apr 13 13:36:19 2016 New Revision: 266224 URL: http://llvm.org/viewvc/llvm-project?rev=266224=rev Log: libclang: Use early-return to reduce indentation. & since I'll get blamed for all the lines anyway, remove some else-after-return and otherwise tidy things up.

Re: r265994 - libclang: fix two memory leaks (PR26292)

sn't even have a C++ destructor. I'm not going to > dig into that, I just wanted to address this bug that someone > reported. > > - Hans > > > On Tue, Apr 12, 2016 at 9:51 AM, David Blaikie via cfe-commits > <cfe-commits@lists.llvm.org> wrote: > > Any chance of using u

r266222 - Simplify memory management of CXEvalResultKind/ExprEvalResult using unique_ptr and a dtor

Author: dblaikie Date: Wed Apr 13 13:23:33 2016 New Revision: 266222 URL: http://llvm.org/viewvc/llvm-project?rev=266222=rev Log: Simplify memory management of CXEvalResultKind/ExprEvalResult using unique_ptr and a dtor This doesn't seem to need to be a C type, C only handles a void*, so use

r266127 - Add a fixme for an old patch I had lying around that I'm not going to finish any time so n

Author: dblaikie Date: Tue Apr 12 16:22:48 2016 New Revision: 266127 URL: http://llvm.org/viewvc/llvm-project?rev=266127=rev Log: Add a fixme for an old patch I had lying around that I'm not going to finish any time so n Modified: cfe/trunk/lib/CodeGen/CGDebugInfo.cpp Modified:

Re: r265994 - libclang: fix two memory leaks (PR26292)

Any chance of using unique_ptr, or at least a scoped cleanup device, here? On Mon, Apr 11, 2016 at 1:54 PM, Hans Wennborg via cfe-commits < cfe-commits@lists.llvm.org> wrote: > Author: hans > Date: Mon Apr 11 15:53:59 2016 > New Revision: 265994 > > URL:

Re: [PATCH] D18961: Add a readability-deleted-default clang-tidy check.

I don't feel sufficiently strongly to insist - clang-tidy's mostly outside my wheelhouse anyway. As for how to go about it - my rough approach would be to write a small test case that calls an implicitly-deleted-but-explicitly-defaulted move op, run it, check the diagnostic text, find that in

Re: r265934 - [clang-format] Walk backwards from end() instead of forwards from rend().

On Mon, Apr 11, 2016 at 5:19 AM, Benjamin Kramer via cfe-commits < cfe-commits@lists.llvm.org> wrote: > Author: d0k > Date: Mon Apr 11 07:19:19 2016 > New Revision: 265934 > > URL: http://llvm.org/viewvc/llvm-project?rev=265934=rev > Log: > [clang-format] Walk backwards from end() instead of

Re: [PATCH] D18961: Add a readability-deleted-default clang-tidy check.

I'd consider just making this a compiler warning, perhaps? On Mon, Apr 11, 2016 at 5:52 AM, Alex Pilkiewicz via cfe-commits < cfe-commits@lists.llvm.org> wrote: > pilki created this revision. > pilki added a reviewer: alexfh. > pilki added a subscriber: cfe-commits. > > Checks if constructors

Re: [PATCH] D18808: Use the NoDebug emission kind to identify compile units that no debug info should be created from.

dblaikie accepted this revision. dblaikie added a comment. This revision is now accepted and ready to land. Looks good, thanks! Sorry for the delays/mthanks for the chat on IRC (summary: no worse than what we have today, more explicit, maybe we can find an alternative and/or more compact

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

On Thu, Apr 7, 2016 at 5:05 PM, Reid Kleckner via cfe-commits < cfe-commits@lists.llvm.org> wrote: > rnk updated this revision to Diff 52982. > rnk marked 3 inline comments as done. > rnk added a comment. > > - Add -Wshadow-all and -Wshadow-field-in-constructor, also address review > comments >

Re: [PATCH] D18808: Use the NoDebug emission kind to identify compile units that no debug info should be created from.

On Wed, Apr 6, 2016 at 8:44 AM, Adrian Prantl wrote: > > On Apr 6, 2016, at 8:16 AM, David Blaikie wrote: > > > > On Tue, Apr 5, 2016 at 10:17 PM, Eric Christopher via llvm-commits < > llvm-comm...@lists.llvm.org> wrote: > >> echristo added inline

Re: [PATCH] D18565: Implement an "I'm dtrace, please retain all debug types" option.

Okey dokey - I know one of the things we did (& I don't know when it happened compared to this change) is emit a hard list of variables onto any subprogram for an optimized (non -O0) function. So we never lose variables due to optimizations, or at least that's the intent. As for D18477, I'm not

Re: [PATCH] D18808: Use the NoDebug emission kind to identify compile units that no debug info should be created from.

On Tue, Apr 5, 2016 at 10:17 PM, Eric Christopher via llvm-commits < llvm-comm...@lists.llvm.org> wrote: > echristo added inline comments. > > > Comment at: lib/CodeGen/AsmPrinter/DwarfDebug.cpp:477-479 > @@ -476,2 +476,5 @@ > + unsigned DebugCUs = 0; >for (MDNode *N :

Re: [PATCH] D3635: DebugInfo: Emit any enum with a referenced enum constant.

dblaikie abandoned this revision. dblaikie added a subscriber: rsmith. dblaikie added a comment. In http://reviews.llvm.org/D3635#44720, @rsmith wrote: > Implementation LGTM, if you decide that you want to go in this direction. Just looking over some old patches/git branches I had lying

Re: [PATCH] D18565: Implement an "I'm dtrace, please retain all debug types" option.

On Wed, Mar 30, 2016 at 10:49 AM, Adrian Prantl wrote: > > On Mar 29, 2016, at 10:06 PM, David Blaikie wrote: > > > > On Tue, Mar 29, 2016 at 12:03 PM, Adrian Prantl via cfe-commits < > cfe-commits@lists.llvm.org> wrote: > >> >> > On Mar 29, 2016, at 12:00

Re: r264428 - [ASTMatchers] Fix build for VariadicFunction.

t; gcc doesn't want to do the implicit conversion from X to A, but if I make > the conversion explicit it works. > > > On Fri, Mar 25, 2016 at 1:58 PM, Alexey Samsonov <vonos...@gmail.com> > wrote: > >> >> On Fri, Mar 25, 2016 at 10:55 AM, David Blaikie via cfe-commi

Re: [PATCH] D18565: Implement an "I'm dtrace, please retain all debug types" option.

On Tue, Mar 29, 2016 at 12:03 PM, Adrian Prantl via cfe-commits < cfe-commits@lists.llvm.org> wrote: > > > On Mar 29, 2016, at 12:00 PM, Joerg Sonnenberger < > jo...@britannica.bec.de> wrote: > > > > On Tue, Mar 29, 2016 at 06:47:24PM +, Adrian Prantl via cfe-commits > wrote: > >> This code

Re: r264428 - [ASTMatchers] Fix build for VariadicFunction.

On Fri, Mar 25, 2016 at 4:01 PM, Richard Smith <rich...@metafoo.co.uk> wrote: > On Fri, Mar 25, 2016 at 10:55 AM, David Blaikie via cfe-commits > <cfe-commits@lists.llvm.org> wrote: > > > > > > On Fri, Mar 25, 2016 at 10:46 AM, Samuel Benzaquen via cfe-com

Re: r264428 - [ASTMatchers] Fix build for VariadicFunction.

On Fri, Mar 25, 2016 at 10:46 AM, Samuel Benzaquen via cfe-commits < cfe-commits@lists.llvm.org> wrote: > Author: sbenza > Date: Fri Mar 25 12:46:02 2016 > New Revision: 264428 > > URL: http://llvm.org/viewvc/llvm-project?rev=264428=rev > Log: > [ASTMatchers] Fix build for VariadicFunction. > >

Re: [PATCH] D18475: [clang-tidy] Add more detection rules for redundant c_str calls.

On Fri, Mar 25, 2016 at 9:59 AM, Etienne Bergeron via cfe-commits < cfe-commits@lists.llvm.org> wrote: > etienneb created this revision. > etienneb added a subscriber: cfe-commits. > > The string class contains methods which support receiving either a string > literal or a string object. > > For

Re: r264205 - [CUDA] Don't define __NVCC__.

Blaikie" <dblai...@gmail.com> > *Cc: *"cfe-commits" <cfe-commits@lists.llvm.org> > *Sent: *Thursday, March 24, 2016 10:39:18 AM > *Subject: *Re: r264205 - [CUDA] Don't define __NVCC__. > > On Thu, Mar 24, 2016 at 8:30 AM, David Blaikie via cfe-commits &l

<    5   6   7   8   9   10   11   12   13   >