Re: [clang-tools-extra] r303735 - Modify test so that it looks for patterns in stderr as well

2017-06-15 Thread David Blaikie via cfe-commits
https://sarcasm.github.io/notes/dev/compilation-database.html#cmake If you enable the CMAKE_EXPORT_COMPILE_COMMANDS option in cmake (& have a sufficiently recent cmake), then CMake will generate a compile_commands.json in the root of the build tree. The test finds this & fails, instead of finding

Re: [clang-tools-extra] r303735 - Modify test so that it looks for patterns in stderr as well

2017-06-14 Thread David Blaikie via cfe-commits
On Wed, Jun 14, 2017, 8:17 AM Serge Pavlov wrote: > 2017-06-14 4:24 GMT+07:00 David Blaikie : > >> Ah, I find that the test passes if I remove the compile_commands.json >> file from my build directory (I have Ninja configured to generate a >>

Re: [clang-tools-extra] r303735 - Modify test so that it looks for patterns in stderr as well

2017-06-13 Thread David Blaikie via cfe-commits
Ah, I find that the test passes if I remove the compile_commands.json file from my build directory (I have Ninja configured to generate a compile_commands.json file). Looks like what happens is it finds the compilation database and fails hard when the database doesn't contain a compile command

Re: [PATCH] D34052: [XRay][clang] Support capturing the implicit `this` argument to C++ class member functions

2017-06-13 Thread David Blaikie via cfe-commits
hard to say if it's more readable without seeing it - if you could attach a patch, if it's easy enough/you worked it up before, might be worth comparing/contrasting On Tue, Jun 13, 2017 at 12:28 AM Dean Michael Berris via Phabricator < revi...@reviews.llvm.org> wrote: > dberris added a comment.

Re: [clang-tools-extra] r303735 - Modify test so that it looks for patterns in stderr as well

2017-06-12 Thread David Blaikie via cfe-commits
I've been seeing errors from this test recently: Command Output (stderr): -- 1 error generated. Error while processing /usr/local/google/home/blaikie/dev/llvm/src/tools/clang/tools/extra/test/clang-tidy/diagnostic.cpp.nonexistent.cpp.

Re: r305182 - Revert r305164/5/7.

2017-06-12 Thread David Blaikie via cfe-commits
hey Saleem - might be worth sending a cfe-dev email about what the general direction/goals are here (may not need precommit review for every patch, but just some sanity checking) On Mon, Jun 12, 2017 at 1:08 AM Daniel Jasper via cfe-commits < cfe-commits@lists.llvm.org> wrote: > Author: djasper

Re: [PATCH] D33102: [clang] Implement -Wcast-qual for C++

2017-06-12 Thread David Blaikie via cfe-commits
Yeah, looks like the UB is baked in pretty deep here, so it's not reasonable to try to fix it just because of this. I'd probably suggest trying making that cast in PointerUnion.h into a reinterpret cast? Would that suffice to address the const issues? Otherwise a const_cast + reinterpret_cast?

Re: r305213 - Add regression test for r305179.

2017-06-12 Thread David Blaikie via cfe-commits
Prefer FileCheck over grep, generally? On Mon, Jun 12, 2017 at 11:05 AM Yaron Keren via cfe-commits < cfe-commits@lists.llvm.org> wrote: > Author: yrnkrn > Date: Mon Jun 12 13:05:13 2017 > New Revision: 305213 > > URL: http://llvm.org/viewvc/llvm-project?rev=305213=rev > Log: > Add regression

Re: [PATCH] D33102: [clang] Implement -Wcast-qual for C++

2017-06-12 Thread David Blaikie via cfe-commits
On Mon, Jun 12, 2017 at 10:10 AM Roman Lebedev via Phabricator < revi...@reviews.llvm.org> wrote: > lebedev.ri added a comment. > > So i'm trying to analyze that stage2 warning. > Could you link to the buildbot failure to see the original LLVM project code triggering this situation? > The

Re: r304892 - [Sema] Silence unused variable warning.

2017-06-12 Thread David Blaikie via cfe-commits
Richard, looks like this might be better as: if (auto QL = OE->getQualifierLoc()) ... QL.getBeginLoc() ... ? On Wed, Jun 7, 2017 at 3:23 AM Benjamin Kramer via cfe-commits < cfe-commits@lists.llvm.org> wrote: > Author: d0k > Date: Wed Jun 7 05:23:17 2017 > New Revision: 304892 > > URL:

Re: [PATCH] D33932: [clang-format] Add support for case-insensitive header matching and use it to improve support for LLVM-style include sorting.

2017-06-12 Thread David Blaikie via cfe-commits
On Tue, Jun 6, 2017 at 3:56 AM Chandler Carruth via Phabricator via cfe-commits wrote: > chandlerc created this revision. > Herald added subscribers: mcrosier, sanjoy, klimek. > > This really is a collection of improvements to the rules for LLVM > include sorting: > >

Re: [PATCH] D33102: [clang] Implement -Wcast-qual for C++

2017-06-10 Thread David Blaikie via cfe-commits
On Sat, Jun 10, 2017 at 3:33 AM Roman Lebedev via Phabricator < revi...@reviews.llvm.org> wrote: > lebedev.ri planned changes to this revision. > lebedev.ri added a comment. > > In https://reviews.llvm.org/D33102#773296, @dblaikie wrote: > > > But sure. Could you also (manually, I guess) confirm

Re: [PATCH] D33102: [clang] Implement -Wcast-qual for C++

2017-06-09 Thread David Blaikie via cfe-commits
Looks all good, please commit whenever you're ready - if you don't have commit access, I (or anyone else with commit access) can commit this for you. On Tue, Jun 6, 2017 at 1:57 PM Roman Lebedev wrote: > On Tue, Jun 6, 2017 at 8:52 PM, David Blaikie

Re: [PATCH] D33102: [clang] Implement -Wcast-qual for C++

2017-06-06 Thread David Blaikie via cfe-commits
On Tue, Jun 6, 2017 at 3:59 AM Roman Lebedev via Phabricator < revi...@reviews.llvm.org> wrote: > lebedev.ri added a comment. > > In https://reviews.llvm.org/D33102#773296, @dblaikie wrote: > > > I still feel like that's more testing than would be ideal (does the > context of the cast matter?

Re: r299921 - [lsan] Enable LSan on arm Linux, clang part

2017-06-05 Thread David Blaikie via cfe-commits
On Mon, Jun 5, 2017 at 10:28 AM Maxim Ostapenko wrote: > On 05/06/17 20:16, David Blaikie wrote: > > This change seemed to be buggy (& I assume missing test coverage to > > demonstrate that). Galina fixed it in > > http://llvm.org/viewvc/llvm-project?rev=304663=rev based on a

Re: r304663 - Fixed warning: enum constant in boolean context.

2017-06-05 Thread David Blaikie via cfe-commits
I followed up on the original commit (r299921) to request that as well. On Sat, Jun 3, 2017 at 1:26 PM Richard Smith via cfe-commits < cfe-commits@lists.llvm.org> wrote: > Can we get a testcase for this bug? Seems like we can't have any coverage > for the case where IsArmArch is supposed to be

Re: r299921 - [lsan] Enable LSan on arm Linux, clang part

2017-06-05 Thread David Blaikie via cfe-commits
This change seemed to be buggy (& I assume missing test coverage to demonstrate that). Galina fixed it in http://llvm.org/viewvc/llvm-project?rev=304663=rev based on a compiler warning. Can you add test coverage to this code to exercise these untested codepaths? On Tue, Apr 11, 2017 at 12:34 AM

Re: [clang-tools-extra] r304583 - [clang-tidy] Add `const` to operator() to fix a warning.

2017-06-05 Thread David Blaikie via cfe-commits
what was the warning? On Fri, Jun 2, 2017 at 11:48 AM Alexander Kornienko via cfe-commits < cfe-commits@lists.llvm.org> wrote: > Author: alexfh > Date: Fri Jun 2 13:47:50 2017 > New Revision: 304583 > > URL: http://llvm.org/viewvc/llvm-project?rev=304583=rev > Log: > [clang-tidy] Add `const` to

Re: [PATCH] D33765: Show correct column nr. when multi-byte utf8 chars are used.

2017-06-05 Thread David Blaikie via cfe-commits
Is it right to only change the behavior of this caller? Presumably other callers (like getSpellingColumnNumber, getExpansionColumnNumber, etc) probably want the same handling? Do any callers /not/ want this behavior? On Thu, Jun 1, 2017 at 3:14 AM Erik Verbruggen via Phabricator via cfe-commits

Re: [PATCH] D33750: CGCleanup: (NFC) add a test that used to trigger broken IR

2017-06-05 Thread David Blaikie via cfe-commits
On Wed, May 31, 2017 at 5:45 PM Gor Nishanov via Phabricator via cfe-commits wrote: > GorNishanov created this revision. > > Coroutine related test that used to trigger broken IR prior to r304335. > > > https://reviews.llvm.org/D33750 > > Files: >

Re: [PATCH] D33708: [test] [libcxx] Disable MSVC++'s compare(a, b) implies !compare(b, a) assertion in predicate application count tests

2017-06-05 Thread David Blaikie via cfe-commits
Alternatively, you could disable the check (or change the count it checks) if this is defined. (& if you disable the check when this is defined, you could change an existing build/test config (or add a new one) to have this disabled) On Tue, May 30, 2017 at 8:19 PM Billy Robert O'Neal III via

Re: r304190 - Diagnose attempts to build a preprocessed module that defines an unavailable submodule.

2017-06-05 Thread David Blaikie via cfe-commits
On Mon, May 29, 2017 at 10:23 PM Richard Smith via cfe-commits < cfe-commits@lists.llvm.org> wrote: > Author: rsmith > Date: Tue May 30 00:22:59 2017 > New Revision: 304190 > > URL: http://llvm.org/viewvc/llvm-project?rev=304190=rev > Log: > Diagnose attempts to build a preprocessed module that

Re: [PATCH] D33660: [coroutines] Fix assertion during -Wuninitialized analysis

2017-06-05 Thread David Blaikie via cfe-commits
On Mon, May 29, 2017 at 3:28 PM Eric Fiselier via Phabricator via cfe-commits wrote: > EricWF created this revision. > > @rsmith Is there a better place to put this test? > > > https://reviews.llvm.org/D33660 > > Files: > lib/Sema/SemaCoroutine.cpp >

r304456 - Add compatibility alias for -Wno-#warnings

2017-06-01 Thread David Blaikie via cfe-commits
Author: dblaikie Date: Thu Jun 1 14:08:34 2017 New Revision: 304456 URL: http://llvm.org/viewvc/llvm-project?rev=304456=rev Log: Add compatibility alias for -Wno-#warnings GCC uses -Wno-cpp for this, so seems reasonable to add an alias to match. Modified:

Re: [PATCH] D33705: [CGVTables] Finalize SP before attempting to clone it

2017-05-30 Thread David Blaikie via cfe-commits
I'll let Adrian weigh in - but I suspect, if possible, it'd be better to just finalize them all once IRGen for the function finishes. On Tue, May 30, 2017 at 6:02 PM Keno Fischer wrote: > For some reason your comments aren't showing up on Phabricator, so > replying via

Re: [PATCH] D33705: [CGVTables] Finalize SP before attempting to clone it

2017-05-30 Thread David Blaikie via cfe-commits
If SPs are now all finalized early - I'm assuming there's some other code that finalizes SPs that's no longer needed? Also - this seems like a layering/separation issue - does other code call into the DI as casually/explicitly as this code being added to CGVTables? (Or should the callback go

Re: r303714 - [coroutines] Fix leak in CGCoroutine.cpp

2017-05-29 Thread David Blaikie via cfe-commits
Fair enough. I don't have all the context there either. Perhaps Richard Smith could sanity check what the right memory management scheme is here. On Mon, May 29, 2017 at 3:54 PM Gor Nishanov wrote: > My clang-foo is not strong enough :) . > > I wanted to make sure that:

Re: r303714 - [coroutines] Fix leak in CGCoroutine.cpp

2017-05-29 Thread David Blaikie via cfe-commits
On Mon, May 29, 2017 at 2:12 PM Gor Nishanov wrote: > It is not known in advance whether the final block is needed or not. It > will become known once the user-authored body of the coroutine is emitted. > I cannot defer creation of it up until that point, since final bb

Re: r303714 - [coroutines] Fix leak in CGCoroutine.cpp

2017-05-29 Thread David Blaikie via cfe-commits
Could you avoid creating the FinalBB unless it's needed rather than creating it and then adding it so it can be removed? (or, if the creation can't be avoided, maybe it's OK to 'delete FinalBB' here, rather than adding it for it to be removed later?) (also bracing seems off - could you run your

Re: r303934 - "*" => "+" to avoid matching on empty string.

2017-05-29 Thread David Blaikie via cfe-commits
Why would matching on an empty string be bad in this case? On Thu, May 25, 2017 at 4:25 PM Richard Trieu via cfe-commits < cfe-commits@lists.llvm.org> wrote: > Author: rtrieu > Date: Thu May 25 18:25:36 2017 > New Revision: 303934 > > URL: http://llvm.org/viewvc/llvm-project?rev=303934=rev >

Re: r304127 - IRGen: Add optnone attribute on function during O0

2017-05-29 Thread David Blaikie via cfe-commits
I'm assuming most of these tests aren't actually testing for attributes - perhaps it'd be better to remove their dependence on a particular attribute list number so future changes to attributes don't require so many touches? On Sun, May 28, 2017 at 10:38 PM Mehdi Amini via cfe-commits <

r301786 - Fix line endings (dos -> unix) and clang-format while I'm here

2017-04-30 Thread David Blaikie via cfe-commits
Author: dblaikie Date: Sun Apr 30 21:11:39 2017 New Revision: 301786 URL: http://llvm.org/viewvc/llvm-project?rev=301786=rev Log: Fix line endings (dos -> unix) and clang-format while I'm here Modified: cfe/trunk/test/SemaCXX/cxx1z-lambda-star-this.cpp Modified:

r301684 - Enable -fno-split-dwarf-inlining even when -gsplit-dwarf isn't specified.

2017-04-28 Thread David Blaikie via cfe-commits
Author: dblaikie Date: Fri Apr 28 15:50:25 2017 New Revision: 301684 URL: http://llvm.org/viewvc/llvm-project?rev=301684=rev Log: Enable -fno-split-dwarf-inlining even when -gsplit-dwarf isn't specified. Since -gsplit-dwarf is specified on a backend compile (in ThinLTO parlance) it isn't passed

Re: r301470 - Fix API breaks

2017-04-26 Thread David Blaikie via cfe-commits
uits you at the time seems fine :) > Reverted my revert in r301472. > Thanks! > > vedant > > > > > > > On Wed, Apr 26, 2017 at 2:14 PM Vedant Kumar <v...@apple.com> wrote: > > Hi David, > > > > It looks like this conflicts with my revert. I wil

Re: r301470 - Fix API breaks

2017-04-26 Thread David Blaikie via cfe-commits
> vedant > > > On Apr 26, 2017, at 1:58 PM, David Blaikie via cfe-commits < > cfe-commits@lists.llvm.org> wrote: > > > > Author: dblaikie > > Date: Wed Apr 26 15:58:21 2017 > > New Revision: 301470 > > > > URL: http://llvm.org/viewvc/llvm-pro

r301470 - Fix API breaks

2017-04-26 Thread David Blaikie via cfe-commits
Author: dblaikie Date: Wed Apr 26 15:58:21 2017 New Revision: 301470 URL: http://llvm.org/viewvc/llvm-project?rev=301470=rev Log: Fix API breaks Modified: cfe/trunk/lib/CodeGen/MacroPPCallbacks.cpp cfe/trunk/lib/CodeGen/MacroPPCallbacks.h Modified:

[clang-tools-extra] r301468 - Fix API breaks

2017-04-26 Thread David Blaikie via cfe-commits
Author: dblaikie Date: Wed Apr 26 15:58:03 2017 New Revision: 301468 URL: http://llvm.org/viewvc/llvm-project?rev=301468=rev Log: Fix API breaks Modified: clang-tools-extra/trunk/pp-trace/PPCallbacksTracker.cpp clang-tools-extra/trunk/pp-trace/PPCallbacksTracker.h Modified:

Re: r300523 - Debug Info: Remove special-casing of indirect function argument handling.

2017-04-24 Thread David Blaikie via cfe-commits
\o/ awesome! (I added that "indirect" hack ages ago, unfortunately - one consolation is that it was worse before: Clang would describe the parameter's type as "T&" in the DWARF, instead of as "T"... ) On Mon, Apr 17, 2017 at 6:34 PM Adrian Prantl via cfe-commits < cfe-commits@lists.llvm.org>

r301063 - Move Split DWARF handling to an MC option/command line argument rather than using metadata

2017-04-21 Thread David Blaikie via cfe-commits
Author: dblaikie Date: Fri Apr 21 18:35:36 2017 New Revision: 301063 URL: http://llvm.org/viewvc/llvm-project?rev=301063=rev Log: Move Split DWARF handling to an MC option/command line argument rather than using metadata Since Split DWARF needs to name the actual .dwo file that is generated, it

r300741 - Parse backend options during thinlto backend compile actions

2017-04-19 Thread David Blaikie via cfe-commits
Author: dblaikie Date: Wed Apr 19 15:08:21 2017 New Revision: 300741 URL: http://llvm.org/viewvc/llvm-project?rev=300741=rev Log: Parse backend options during thinlto backend compile actions Added: cfe/trunk/test/CodeGen/thinlto-backend-option.ll Modified:

r300461 - Use default ref capture to simplify local lambdas, use a template to avoid std::function overhead, other cleanup

2017-04-17 Thread David Blaikie via cfe-commits
Author: dblaikie Date: Mon Apr 17 12:16:19 2017 New Revision: 300461 URL: http://llvm.org/viewvc/llvm-project?rev=300461=rev Log: Use default ref capture to simplify local lambdas, use a template to avoid std::function overhead, other cleanup Modified:

Re: r300145 - ExternalASTMerger.cpp: Silence another warning. [-Wunused-lambda-capture]

2017-04-17 Thread David Blaikie via cfe-commits
I'll change this to [&] capture - any lambda that doesn't escape it's scope should generally use [&] & that'll avoid the need for explicitly discarding conditionally unused captures, etc. On Wed, Apr 12, 2017 at 5:45 PM NAKAMURA Takumi via cfe-commits < cfe-commits@lists.llvm.org> wrote: >

r300106 - Modular Codegen: Include testing for inline asm as well as some commentary on the implementaiton choice.

2017-04-12 Thread David Blaikie via cfe-commits
Author: dblaikie Date: Wed Apr 12 16:14:04 2017 New Revision: 300106 URL: http://llvm.org/viewvc/llvm-project?rev=300106=rev Log: Modular Codegen: Include testing for inline asm as well as some commentary on the implementaiton choice. Modified: cfe/trunk/test/Modules/Inputs/codegen/foo.h

r300105 - Fix up test to handle the now split -fmodules-codegen and -fmodules-debuginfo flags

2017-04-12 Thread David Blaikie via cfe-commits
Author: dblaikie Date: Wed Apr 12 16:09:34 2017 New Revision: 300105 URL: http://llvm.org/viewvc/llvm-project?rev=300105=rev Log: Fix up test to handle the now split -fmodules-codegen and -fmodules-debuginfo flags Modified: cfe/trunk/test/Modules/codegen-nodep.test Modified:

r300104 - Modular Codegen: Separate flags for function and debug info support

2017-04-12 Thread David Blaikie via cfe-commits
Author: dblaikie Date: Wed Apr 12 15:58:33 2017 New Revision: 300104 URL: http://llvm.org/viewvc/llvm-project?rev=300104=rev Log: Modular Codegen: Separate flags for function and debug info support This allows using and testing these two features separately. (noteably, debug info is, so far as I

r299987 - Modular Codegen: Support homing debug info for types in modular objects

2017-04-11 Thread David Blaikie via cfe-commits
Author: dblaikie Date: Tue Apr 11 16:13:37 2017 New Revision: 299987 URL: http://llvm.org/viewvc/llvm-project?rev=299987=rev Log: Modular Codegen: Support homing debug info for types in modular objects Matching the function-homing support for modular codegen. Any type implicitly (implicit

r299982 - Modular Codegen: Add/use a bit in serialized function definitions to track whether they are the subject of modular codegen

2017-04-11 Thread David Blaikie via cfe-commits
Author: dblaikie Date: Tue Apr 11 15:46:34 2017 New Revision: 299982 URL: http://llvm.org/viewvc/llvm-project?rev=299982=rev Log: Modular Codegen: Add/use a bit in serialized function definitions to track whether they are the subject of modular codegen Some decls are created not where they are

Re: [PATCH] D31153: Add the ability to use the children() range API in a const-correct manner

2017-04-11 Thread David Blaikie via cfe-commits
Ah - perhaps it'd be worth having a standalone forward declaration for clarity? I don't think I've seen this construct anywhere else in LLVM? But I could be wrong. On Tue, Apr 11, 2017 at 10:51 AM Aaron Ballman via Phabricator < revi...@reviews.llvm.org> wrote: > aaron.ballman added inline

Re: [PATCH] D31153: Add the ability to use the children() range API in a const-correct manner

2017-04-11 Thread David Blaikie via cfe-commits
On Tue, Apr 11, 2017 at 10:27 AM Aaron Ballman via Phabricator < revi...@reviews.llvm.org> wrote: > aaron.ballman added inline comments. > > > > Comment at: include/clang/AST/Expr.h:4025 >child_range children() { > +const_child_range CCR = const_cast *>(this)->children();

[clang-tools-extra] r299561 - Fix -Wmissing-field-initializer warnings to unbreak the -Werror build

2017-04-05 Thread David Blaikie via cfe-commits
Author: dblaikie Date: Wed Apr 5 11:50:19 2017 New Revision: 299561 URL: http://llvm.org/viewvc/llvm-project?rev=299561=rev Log: Fix -Wmissing-field-initializer warnings to unbreak the -Werror build Modified: clang-tools-extra/trunk/unittests/clang-rename/RenameClassTest.cpp Modified:

Re: [PATCH] D31153: Add the ability to use the children() range API in a const-correct manner

2017-03-27 Thread David Blaikie via cfe-commits
On Mon, Mar 27, 2017 at 10:20 AM Aaron Ballman via Phabricator < revi...@reviews.llvm.org> wrote: > aaron.ballman added a comment. > > In https://reviews.llvm.org/D31153#711287, @dblaikie wrote: > > > As I mentioned to Craig Topper recently on another review, generally > when implementing const

Re: [PATCH] D30760: Record command lines in objects built by clang, Clang part

2017-03-26 Thread David Blaikie via cfe-commits
Yeah, I don't know/mind either way - I think there's a tidy simplicity to including exactly what the user wrote on the command line, so don't mind if it's not removed, but can see the argument to omit it. I'd probably leave it in for simplicity. On Fri, Mar 24, 2017 at 5:48 PM Eric Christopher

Re: [PATCH] D30760: Record command lines in objects built by clang, Clang part

2017-03-24 Thread David Blaikie via cfe-commits
As Adrian mentioned, this can probably be covered/added to an existing test case in clang/test/Driver On Fri, Mar 24, 2017 at 1:57 PM Zhizhou Yang via Phabricator < revi...@reviews.llvm.org> wrote: > zhizhouy updated this revision to Diff 93003. > zhizhouy marked 3 inline comments as done. >

Re: r297975 - Use arg_begin() instead of getArgumentList().begin(), the argument list is an implementation detail

2017-03-20 Thread David Blaikie via cfe-commits
On Mon, Mar 20, 2017 at 8:59 AM Reid Kleckner wrote: > I came across llvm/docs/HistoricalNotes/2002-06-25-MegaPatchInfo.txt, > which has this: > """ > * The Function class now has helper functions for accessing the Arguments > list. > Instead of having to go through

Re: r297975 - Use arg_begin() instead of getArgumentList().begin(), the argument list is an implementation detail

2017-03-20 Thread David Blaikie via cfe-commits
On Thu, Mar 16, 2017 at 12:07 PM Reid Kleckner via cfe-commits < cfe-commits@lists.llvm.org> wrote: > Author: rnk > Date: Thu Mar 16 13:55:46 2017 > New Revision: 297975 > > URL: http://llvm.org/viewvc/llvm-project?rev=297975=rev > Log: > Use arg_begin() instead of getArgumentList().begin(), the

r297322 - Defensively ensure that GetExternalDeclStmt protects itself from nested deserialization

2017-03-08 Thread David Blaikie via cfe-commits
Author: dblaikie Date: Wed Mar 8 17:57:08 2017 New Revision: 297322 URL: http://llvm.org/viewvc/llvm-project?rev=297322=rev Log: Defensively ensure that GetExternalDeclStmt protects itself from nested deserialization Modified: cfe/trunk/lib/Serialization/ASTReader.cpp Modified:

Re: r297140 - [clang-format] Support namespaces ending in semicolon

2017-03-07 Thread David Blaikie via cfe-commits
Looks to be failing existing tests? FAIL: Clang-Unit :: Format/FormatTests/FormatTest.BreaksLongDeclarations (12427 of 32080) TEST 'Clang-Unit :: Format/FormatTests/FormatTest.BreaksLongDeclarations' FAILED Note: Google Test filter =

r296386 - Remove unused variable

2017-02-27 Thread David Blaikie via cfe-commits
Author: dblaikie Date: Mon Feb 27 15:14:42 2017 New Revision: 296386 URL: http://llvm.org/viewvc/llvm-project?rev=296386=rev Log: Remove unused variable Modified: cfe/trunk/include/clang/Serialization/ASTReader.h Modified: cfe/trunk/include/clang/Serialization/ASTReader.h URL:

Re: [PATCH] D30315: [Driver] Move architecture-specific free helper functions to their own files.

2017-02-27 Thread David Blaikie via cfe-commits
On Thu, Feb 23, 2017 at 4:08 PM David L. Jones via Phabricator via cfe-commits wrote: > dlj created this revision. > Herald added subscribers: mgorny, nemanjai, jyknight, dschuff, srhines, > danalbert, aemerson. > Herald added a reviewer: javed.absar. > > This patch

Re: r296221 - [ODRHash] Move inherited visitor call to end of function.

2017-02-27 Thread David Blaikie via cfe-commits
An explanation as to why the code was moved (& possibly test cases, or "NFC" in the description) would be handy here. On Fri, Feb 24, 2017 at 5:41 PM Richard Trieu via cfe-commits < cfe-commits@lists.llvm.org> wrote: > Author: rtrieu > Date: Fri Feb 24 19:29:34 2017 > New Revision: 296221 > >

Re: [PATCH] D13330: Implement __attribute__((unique_instantiation))

2017-02-22 Thread David Blaikie via cfe-commits
On Tue, Feb 14, 2017 at 4:21 PM Mehdi AMINI via Phabricator via cfe-commits wrote: > mehdi_amini added a comment. > > In https://reviews.llvm.org/D13330#582607, @rsmith wrote: > > > I think this attribute is poorly named. Explicit instantiation > definitions are

Re: [PATCH] D30035: Add const to function parameters

2017-02-22 Thread David Blaikie via cfe-commits
Adding const to pointed/referenced types doesn't usually help the compiler, since they don't guarantee that the underlying object is const (nor that any use can't involve const_casting away the constness and mutating the value anyway). On Fri, Feb 17, 2017 at 2:34 PM Aditya Kumar via Phabricator

r294904 - ASTReader: Refactor common code for writing function definitions, to match the writing code

2017-02-12 Thread David Blaikie via cfe-commits
Author: dblaikie Date: Sun Feb 12 12:45:31 2017 New Revision: 294904 URL: http://llvm.org/viewvc/llvm-project?rev=294904=rev Log: ASTReader: Refactor common code for writing function definitions, to match the writing code Modified: cfe/trunk/lib/Serialization/ASTReaderDecl.cpp Modified:

[clang-tools-extra] r294823 - Fix memory leak by using unique_ptr

2017-02-10 Thread David Blaikie via cfe-commits
Author: dblaikie Date: Fri Feb 10 23:25:21 2017 New Revision: 294823 URL: http://llvm.org/viewvc/llvm-project?rev=294823=rev Log: Fix memory leak by using unique_ptr Modified: clang-tools-extra/trunk/modularize/CoverageChecker.cpp clang-tools-extra/trunk/modularize/CoverageChecker.h

Re: [PATCH] D16135: Macro Debug Info support in Clang

2017-02-09 Thread David Blaikie via cfe-commits
r294676 On Thu, Feb 9, 2017 at 4:05 PM David L. Jones via Phabricator < revi...@reviews.llvm.org> wrote: > dlj added inline comments. > > > > Comment at: cfe/trunk/lib/CodeGen/MacroPPCallbacks.cpp:125 > + switch (Status) { > + default: > +llvm_unreachable("Do not expect to

r294676 - Fix the -Werror build by removing an unused default in a fully covered switch

2017-02-09 Thread David Blaikie via cfe-commits
Author: dblaikie Date: Thu Feb 9 18:06:38 2017 New Revision: 294676 URL: http://llvm.org/viewvc/llvm-project?rev=294676=rev Log: Fix the -Werror build by removing an unused default in a fully covered switch Modified: cfe/trunk/lib/CodeGen/MacroPPCallbacks.cpp Modified:

Re: [PATCH] D16135: Macro Debug Info support in Clang

2017-02-08 Thread David Blaikie via cfe-commits
On Wed, Feb 8, 2017 at 2:25 PM Amjad Aboud via Phabricator < revi...@reviews.llvm.org> wrote: > aaboud added a comment. > > > How much does the build directory grow? > > Is there any noticeable compile time regression? > > I build clang in release mode using GCC, then used that build to build >

r294512 - Initialize builtins during modular codegen

2017-02-08 Thread David Blaikie via cfe-commits
Author: dblaikie Date: Wed Feb 8 14:51:11 2017 New Revision: 294512 URL: http://llvm.org/viewvc/llvm-project?rev=294512=rev Log: Initialize builtins during modular codegen Added: cfe/trunk/test/Modules/Inputs/codegen-opt/ cfe/trunk/test/Modules/Inputs/codegen-opt/bar.h - copied,

Re: [PATCH] D16135: Macro Debug Info support in Clang

2017-02-07 Thread David Blaikie via cfe-commits
On Tue, Feb 7, 2017 at 11:01 AM Amjad Aboud via Phabricator < revi...@reviews.llvm.org> wrote: > aaboud added a comment. > > In https://reviews.llvm.org/D16135#669416, @aprantl wrote: > > > In https://reviews.llvm.org/D16135#669045, @aaboud wrote: > > > > > Addressed Adrian last comments. > > >

r293692 - Fix modules codegen to be compatible with modules-ts

2017-01-31 Thread David Blaikie via cfe-commits
Author: dblaikie Date: Tue Jan 31 15:28:19 2017 New Revision: 293692 URL: http://llvm.org/viewvc/llvm-project?rev=293692=rev Log: Fix modules codegen to be compatible with modules-ts The Module::WithCodegen flag was only being set when the module was parsed from a ModuleMap. Instead set it late,

Re: r293457 - Tidy up codegen modules test & make it x86 specific since it relies on Itanium name manglings

2017-01-30 Thread David Blaikie via cfe-commits
age- > > From: cfe-commits [mailto:cfe-commits-boun...@lists.llvm.org] On Behalf > Of > > David Blaikie via cfe-commits > > Sent: Sunday, January 29, 2017 9:34 PM > > To: cfe-commits@lists.llvm.org > > Subject: r293457 - Tidy up codegen modules test & make it

Re: [PATCH] D28007: Switch TableGen to emit calls to ASTRecordReader for AttrPCHRead.

2017-01-30 Thread David Blaikie via cfe-commits
This review thread is missing on-list approval. I assume it happened on Phab & just wasn't reflected here (if the Phab approval has no text it doesn't send an email, which is unfortunate - not sure if anyone's planning to fix that/change the settings, but until then it's helpful to include "LGTM"

Re: r293395 - Modules: Clarify ownership of ModuleFile instances in ModuleManager, NFC

2017-01-30 Thread David Blaikie via cfe-commits
Thanks! Always love to see cleanup like this :) On Sat, Jan 28, 2017 at 2:35 PM Duncan P. N. Exon Smith via cfe-commits < cfe-commits@lists.llvm.org> wrote: > Author: dexonsmith > Date: Sat Jan 28 16:24:01 2017 > New Revision: 293395 > > URL: http://llvm.org/viewvc/llvm-project?rev=293395=rev >

r293462 - Reapply "DebugInfo: Omit class definitions even in the presence of available_externally vtables"

2017-01-29 Thread David Blaikie via cfe-commits
Author: dblaikie Date: Mon Jan 30 00:36:08 2017 New Revision: 293462 URL: http://llvm.org/viewvc/llvm-project?rev=293462=rev Log: Reapply "DebugInfo: Omit class definitions even in the presence of available_externally vtables" Accounts for a case that caused an assertion failure by attempting

r293457 - Tidy up codegen modules test & make it x86 specific since it relies on Itanium name manglings

2017-01-29 Thread David Blaikie via cfe-commits
Author: dblaikie Date: Sun Jan 29 23:33:51 2017 New Revision: 293457 URL: http://llvm.org/viewvc/llvm-project?rev=293457=rev Log: Tidy up codegen modules test & make it x86 specific since it relies on Itanium name manglings Modified: cfe/trunk/test/Modules/codegen.test Modified:

Re: [PATCH] D28845: Prototype of modules codegen

2017-01-29 Thread David Blaikie via cfe-commits
On Sun, Jan 29, 2017 at 10:21 AM Richard Smith via Phabricator < revi...@reviews.llvm.org> wrote: > rsmith accepted this revision. > rsmith added inline comments. > This revision is now accepted and ready to land. > > > > Comment at: lib/AST/ExternalASTSource.cpp:33 >

r293456 - Prototype of modules codegen

2017-01-29 Thread David Blaikie via cfe-commits
Author: dblaikie Date: Sun Jan 29 23:00:26 2017 New Revision: 293456 URL: http://llvm.org/viewvc/llvm-project?rev=293456=rev Log: Prototype of modules codegen First pass at generating weak definitions of inline functions from module files (& skipping (-O0) or emitting available_externally

r293344 - Fix linkage of static locals in available_externally functions to be DiscardableODR/linkonce_odr

2017-01-27 Thread David Blaikie via cfe-commits
Author: dblaikie Date: Fri Jan 27 17:11:10 2017 New Revision: 293344 URL: http://llvm.org/viewvc/llvm-project?rev=293344=rev Log: Fix linkage of static locals in available_externally functions to be DiscardableODR/linkonce_odr As Mehdi put it, entities should either be

Re: [PATCH] D29233: Fix linkage of static locals in available_externally functions to be DiscardableODR/linkonce_odr

2017-01-27 Thread David Blaikie via cfe-commits
On Fri, Jan 27, 2017 at 2:51 PM Mehdi Amini wrote: > On Jan 27, 2017, at 2:43 PM, David Blaikie wrote: > > > > On Fri, Jan 27, 2017 at 2:13 PM Mehdi Amini wrote: > > CC Hans. > > This is not a regression (AFAICT), but this is a

Re: [PATCH] D29233: Fix linkage of static locals in available_externally functions to be DiscardableODR/linkonce_odr

2017-01-27 Thread David Blaikie via cfe-commits
On Fri, Jan 27, 2017 at 2:48 PM Mehdi Amini wrote: > On Jan 27, 2017, at 2:44 PM, David Blaikie wrote: > > > > On Fri, Jan 27, 2017 at 2:11 PM Mehdi AMINI via Phabricator < > revi...@reviews.llvm.org> wrote: > > mehdi_amini accepted this revision. >

Re: [PATCH] D29233: Fix linkage of static locals in available_externally functions to be DiscardableODR/linkonce_odr

2017-01-27 Thread David Blaikie via cfe-commits
Reid: Richard and I played around with this with MSVC on godbolt

Re: [PATCH] D29233: Fix linkage of static locals in available_externally functions to be DiscardableODR/linkonce_odr

2017-01-27 Thread David Blaikie via cfe-commits
On Fri, Jan 27, 2017 at 2:11 PM Mehdi AMINI via Phabricator < revi...@reviews.llvm.org> wrote: > mehdi_amini accepted this revision. > mehdi_amini added a comment. > This revision is now accepted and ready to land. > > LGTM. > > > > > Comment at: lib/AST/ASTContext.cpp:8909 > + >

Re: [PATCH] D29233: Fix linkage of static locals in available_externally functions to be DiscardableODR/linkonce_odr

2017-01-27 Thread David Blaikie via cfe-commits
On Fri, Jan 27, 2017 at 2:13 PM Mehdi Amini wrote: > CC Hans. > > This is not a regression (AFAICT), but this is a quality improvement, so > may be worth considering in the 4.0 branch? > Perhaps - I'd generally err on the "if it's not a regression, don't hold the boat".

Re: [PATCH] D28845: Prototype of modules codegen

2017-01-24 Thread David Blaikie via cfe-commits
Richard: This ought to be ready for another round of review at this point, I Think. Got test cases (showing opt and non-opt behavior, direct and indirect modular dependencies, unused modular code, etc - might need some more comments?), flag, moved the linkage code to GetGVALinkageForFunction.

r292801 - Revert "DebugInfo: Omit class definitions even in the presence of available_externally vtables"

2017-01-23 Thread David Blaikie via cfe-commits
Author: dblaikie Date: Mon Jan 23 10:57:14 2017 New Revision: 292801 URL: http://llvm.org/viewvc/llvm-project?rev=292801=rev Log: Revert "DebugInfo: Omit class definitions even in the presence of available_externally vtables" Patch crashing on a bootstrapping sanitizer bot:

Re: r292632 - Fix actually-reachable llvm_unreachable.

2017-01-23 Thread David Blaikie via cfe-commits
Should this test that some specific mangling comes out the other end (tests that amount to "test that this does anything other than crashing" always make me a bit suspicious that there's /some/ specific behavior that is intended/should be tested for) On Fri, Jan 20, 2017 at 11:01 AM Richard Smith

r292768 - DebugInfo: Omit class definitions even in the presence of available_externally vtables

2017-01-22 Thread David Blaikie via cfe-commits
Author: dblaikie Date: Sun Jan 22 20:24:03 2017 New Revision: 292768 URL: http://llvm.org/viewvc/llvm-project?rev=292768=rev Log: DebugInfo: Omit class definitions even in the presence of available_externally vtables To ensure optimization level doesn't pessimize the -fstandalone-debug vtable

r292439 - Remove now redundant code that ensured debug info for class definitions was emitted under certain circumstances

2017-01-18 Thread David Blaikie via cfe-commits
Author: dblaikie Date: Wed Jan 18 15:15:18 2017 New Revision: 292439 URL: http://llvm.org/viewvc/llvm-project?rev=292439=rev Log: Remove now redundant code that ensured debug info for class definitions was emitted under certain circumstances Introduced in r181561 - it may've been subsumed by

Re: [PATCH] D28845: Prototype of modules codegen

2017-01-18 Thread David Blaikie via cfe-commits
Oh, remembered I had one other question/idea: Tangentially related to the question about non-inline functions in headers: Currently .pcm files have the 'interesting decls' list. Are there any interesting decls once modules codegen is in use? It seems mostly the interesting decls are things like

Re: [PATCH] D28845: Prototype of modules codegen

2017-01-18 Thread David Blaikie via cfe-commits
On Wed, Jan 18, 2017 at 4:12 AM Hal Finkel via Phabricator < revi...@reviews.llvm.org> wrote: > hfinkel added a comment. > > Can you provide a high-level description of what you're trying to > accomplish and the usage model? > Oh, for sure - sorry for the assumption/missing info. Stuff that's

Re: [PATCH] D28548: Improve include fixer's ranking by taking the paths into account.

2017-01-17 Thread David Blaikie via cfe-commits
On Tue, Jan 17, 2017 at 12:57 AM Manuel Klimek wrote: > It's by design. Do we want to change this? I always had the impression > turning on the state changes for the list is a bit noisy, but I'm happy if > that's not the general sentiment. > For approval it's important that

Re: [PATCH] D28548: Improve include fixer's ranking by taking the paths into account.

2017-01-16 Thread David Blaikie via cfe-commits
Looks like Ben signed off on this on Phab - but the email didn't go to the list (making this look like code was sent for review, then committed, without review/approval happening) Ben: I think Phab doesn't send mail for an approval with no text, so at least as a workaround you can write something

r291953 - Fix shared library build after r291938 by adding missing dependency on libOption

2017-01-13 Thread David Blaikie via cfe-commits
Author: dblaikie Date: Fri Jan 13 13:47:55 2017 New Revision: 291953 URL: http://llvm.org/viewvc/llvm-project?rev=291953=rev Log: Fix shared library build after r291938 by adding missing dependency on libOption Thanks to Reid Kleckner for the help reproducing/fixing. Modified:

r291938 - unique_ptrify Driver::ToolChains

2017-01-13 Thread David Blaikie via cfe-commits
Author: dblaikie Date: Fri Jan 13 12:53:43 2017 New Revision: 291938 URL: http://llvm.org/viewvc/llvm-project?rev=291938=rev Log: unique_ptrify Driver::ToolChains Modified: cfe/trunk/include/clang/Driver/Driver.h cfe/trunk/lib/Driver/Driver.cpp Modified:

r291919 - unique_ptrify createDriverOptTable

2017-01-13 Thread David Blaikie via cfe-commits
Author: dblaikie Date: Fri Jan 13 11:34:15 2017 New Revision: 291919 URL: http://llvm.org/viewvc/llvm-project?rev=291919=rev Log: unique_ptrify createDriverOptTable Modified: cfe/trunk/include/clang/Driver/Driver.h cfe/trunk/include/clang/Driver/Options.h

Re: [PATCH] D25435: Add -fdebug-info-for-profiling to emit more debug info for sample pgo profile collection

2017-01-09 Thread David Blaikie via cfe-commits
I started the llvm-dev thread for the instruction location tradeoffs (removing the location from instructions to improve profiling support - possibly at the cost of other debug info consumers). The motivation for this flag is different - for added functionality that's generally (imho/based on

Re: r291058 - [Sema] Mark undefined ctors as deleted. NFC.

2017-01-09 Thread David Blaikie via cfe-commits
Alternatively could make the bool ctor a template with some SFINAE to restrict it to only parameters of type bool - thus blocking all conversions there, if they're particularly problematic. On Wed, Jan 4, 2017 at 5:32 PM George Burgess IV via cfe-commits < cfe-commits@lists.llvm.org> wrote: >

Re: [libcxx] r291275 - [libc++] Cleanup and document <__threading_support>

2017-01-06 Thread David Blaikie via cfe-commits
Thanks! On Fri, Jan 6, 2017 at 3:26 PM Eric Fiselier wrote: > Should be fixed in r291298. Sorry for the breakage. > > /Eric > > On Fri, Jan 6, 2017 at 4:18 PM, Eric Fiselier wrote: > > Hi David, > > Thanks for the information. Looking into it now. > > /Eric > > On

Re: [libcxx] r291275 - [libc++] Cleanup and document <__threading_support>

2017-01-06 Thread David Blaikie via cfe-commits
On Fri, Jan 6, 2017 at 12:16 PM Eric Fiselier via cfe-commits < cfe-commits@lists.llvm.org> wrote: > Author: ericwf > Date: Fri Jan 6 14:05:40 2017 > New Revision: 291275 > > URL: http://llvm.org/viewvc/llvm-project?rev=291275=rev > Log: > [libc++] Cleanup and document <__threading_support> > >

[clang-tools-extra] r291272 - Reapply "Fix for shared_ptrification in Clang"

2017-01-06 Thread David Blaikie via cfe-commits
Author: dblaikie Date: Fri Jan 6 13:49:09 2017 New Revision: 291272 URL: http://llvm.org/viewvc/llvm-project?rev=291272=rev Log: Reapply "Fix for shared_ptrification in Clang" Aleksey Shlyapnikov pointed out the memory leak I'd introduced, so recommitted the clang change with a fix for that.

<    4   5   6   7   8   9   10   11   12   13   >