Re: [PATCH] D36386: [clang] Remove unit test which uses reverse-iterate and fix a PointerLikeTypeTrait specialization

2017-08-09 Thread David Blaikie via cfe-commits
OK - go ahead and remove the tests then, if the functionality is now covered by the buildbot+existing tests. On Wed, Aug 9, 2017 at 12:04 PM Grang, Mandeep Singh wrote: > > Ah, OK. I'm still curious about whether this results in a loss of test > coverage. Without this

Re: [PATCH] D36386: [clang] Remove unit test which uses reverse-iterate and fix a PointerLikeTypeTrait specialization

2017-08-09 Thread David Blaikie via cfe-commits
On Wed, Aug 9, 2017 at 1:44 AM Grang, Mandeep Singh wrote: > In D35043 I have removed the llvm tests which use -reverse-iterate. This > patch removes the clang tests. > Ah, OK. I'm still curious about whether this results in a loss of test coverage. Without this test,

r310508 - PointerLikeTypeTraits: class->struct to match LLVM change

2017-08-09 Thread David Blaikie via cfe-commits
Author: dblaikie Date: Wed Aug 9 11:34:22 2017 New Revision: 310508 URL: http://llvm.org/viewvc/llvm-project?rev=310508=rev Log: PointerLikeTypeTraits: class->struct to match LLVM change Modified: cfe/trunk/include/clang/AST/CanonicalType.h cfe/trunk/include/clang/AST/DeclCXX.h

Re: [PATCH] D36386: [clang] Remove unit test which uses reverse-iterate and fix a PointerLikeTypeTrait specialization

2017-08-07 Thread David Blaikie via cfe-commits
On Mon, Aug 7, 2017 at 12:08 PM Mandeep Singh Grang via Phabricator < revi...@reviews.llvm.org> wrote: > mgrang added a comment. > > This patch does 3 things: > > 1. Get rid of the unit test objc-modern-metadata-visibility2.mm because > this test check uses flag -reverse-iterate. This flag will

Re: [PATCH] D36386: [clang] Remove unit test which uses reverse-iterate and fix a PointerLikeTypeTrait specialization

2017-08-07 Thread David Blaikie via cfe-commits
Not sure I understand the context for these changes - could you describe the motivation(s) in more detail? On Sun, Aug 6, 2017 at 10:39 PM Mandeep Singh Grang via Phabricator < revi...@reviews.llvm.org> wrote: > mgrang created this revision. > > This patch is in response to

Re: [PATCH] D35746: Make new PM honor -fdebug-info-for-profiling (clang side)

2017-07-31 Thread David Blaikie via cfe-commits
On Thu, Jul 27, 2017 at 8:30 AM Dehao Chen via Phabricator via cfe-commits < cfe-commits@lists.llvm.org> wrote: > danielcdh marked an inline comment as done. > danielcdh added a comment. > > Thanks for the review! > > In https://reviews.llvm.org/D35746#822498, @chandlerc wrote: > > > LGTM with a

Re: [PATCH] D35783: Ignore shadowing for declarations coming from within macros.

2017-07-31 Thread David Blaikie via cfe-commits
On Tue, Jul 25, 2017 at 1:19 AM Roman Lebedev via Phabricator via cfe-commits wrote: > lebedev.ri added a comment. > > How does this relate to the gcc behavior? > I suspect not everyone would want to have this relaxed `-Wshadow` mode. > Generally I think the goal

Re: r309496 - Improve readability of CXX method overrides list

2017-07-31 Thread David Blaikie via cfe-commits
Test coverage? On Sun, Jul 30, 2017 at 5:52 AM Lenar Safin via cfe-commits < cfe-commits@lists.llvm.org> wrote: > Author: lll > Date: Sat Jul 29 13:42:58 2017 > New Revision: 309496 > > URL: http://llvm.org/viewvc/llvm-project?rev=309496=rev > Log: > Improve readability of CXX method overrides

Re: [libcxxabi] r309349 - [demangler] Fix some overzealous -Wreturn-type errors

2017-07-31 Thread David Blaikie via cfe-commits
On Thu, Jul 27, 2017 at 6:35 PM Erik Pilkington via cfe-commits < cfe-commits@lists.llvm.org> wrote: > Author: epilk > Date: Thu Jul 27 18:35:14 2017 > New Revision: 309349 > > URL: http://llvm.org/viewvc/llvm-project?rev=309349=rev > Log: > [demangler] Fix some overzealous -Wreturn-type errors >

Re: [PATCH] D35961: [llvm] Update MachOObjectFile::exports interface

2017-07-28 Thread David Blaikie via cfe-commits
On Fri, Jul 28, 2017 at 4:01 PM Kevin Enderby wrote: > On Jul 28, 2017, at 10:30 AM, David Blaikie wrote: > > > > On Fri, Jul 28, 2017 at 10:24 AM Kevin Enderby wrote: > >> On Jul 27, 2017, at 9:50 PM, Saleem Abdulrasool via Phabricator

Re: [PATCH] D35961: [llvm] Update MachOObjectFile::exports interface

2017-07-28 Thread David Blaikie via cfe-commits
On Fri, Jul 28, 2017 at 10:24 AM Kevin Enderby wrote: > On Jul 27, 2017, at 9:50 PM, Saleem Abdulrasool via Phabricator < > revi...@reviews.llvm.org> wrote: > > compnerd added a comment. > > Does anyone use the overload with the `O` for `exports` with `nullptr` > instead of

Re: [PATCH] D35715: Preserve typedef names in debug info for template type parameters

2017-07-21 Thread David Blaikie via cfe-commits
On Fri, Jul 21, 2017 at 12:57 PM David Blaikie wrote: > On Fri, Jul 21, 2017 at 11:34 AM David Majnemer via Phabricator < > revi...@reviews.llvm.org> wrote: > >> majnemer added a comment. >> >> This might be a silly question but why not do this by default? >> > > I'd hazard a

Re: [PATCH] D35715: Preserve typedef names in debug info for template type parameters

2017-07-21 Thread David Blaikie via cfe-commits
On Fri, Jul 21, 2017 at 11:34 AM David Majnemer via Phabricator < revi...@reviews.llvm.org> wrote: > majnemer added a comment. > > This might be a silly question but why not do this by default? > I'd hazard a guess that GDB wouldn't cope well with this (in terms of identifying templates as the

Re: r307232 - [modules ts] Do not emit strong function definitions from the module interface unit in every user.

2017-07-16 Thread David Blaikie via cfe-commits
Looks good - does this support available_externally definitions of strong external linkage functions in the users of a module? (is that tested?) Should it? Also should we consider having two flags for modular codegen - one for correctness (external function definitions), one for linkage size

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

2017-07-12 Thread David Blaikie via cfe-commits
Thanks all! On Wed, Jul 12, 2017 at 7:46 AM Alexander Kornienko wrote: > Done in r307661. > > On Mon, Jul 10, 2017 at 2:08 PM, Alexander Kornienko > wrote: > >> Benjamin has actually fixed the issue by reverting to the old behavior in >> r306822. >> I'll

Re: [PATCH] D14358: DWARF's forward decl of a template should have template parameters.

2017-07-11 Thread David Blaikie via cfe-commits
I'm roughly where I was before, I think: "In any case, it seems like it might make sense for you to upstream your template naming change and put it under the PS4 debugger tuning option, and put this change there too, once the motivation for it is in-tree. At that point, while I'd be curious about

Re: [PATCH] D34896: Enable the new PM + SamlePGO + ThinLTO testing.

2017-07-11 Thread David Blaikie via cfe-commits
On Mon, Jul 10, 2017 at 10:58 AM Dehao Chen wrote: > This test was originally added in https://reviews.llvm.org/D34721 with > clang change. It's kind of dup of the previous test > (-check-prefix=SAMPLEPGO) in terms of testing the clang bits. But we > want to make sure the

Re: r307316 - Reject attempts to build a module without -fmodules, rather than silently doing weird things.

2017-07-10 Thread David Blaikie via cfe-commits
What's the reason that passing a module file can't imply -fmodules without the user needing to specify it? On Thu, Jul 6, 2017 at 2:06 PM Richard Smith via cfe-commits < cfe-commits@lists.llvm.org> wrote: > Author: rsmith > Date: Thu Jul 6 14:05:56 2017 > New Revision: 307316 > > URL:

Re: [PATCH] D34896: Enable the new PM + SamlePGO + ThinLTO testing.

2017-07-10 Thread David Blaikie via cfe-commits
Does this test any code in Clang? (given the test is being committed without any change in Clang, I'm guessing maybe not) - perhaps this test doesn't belong in Clang's test suite? Looks like changes/functionality in LTOPreLinkDefaultPipeline are tested in test/Other/new-pm-thinlto-defaults.ll at

Re: r306809 - Ambiguity might be also uninitialized. Use llvm::Optional.

2017-07-10 Thread David Blaikie via cfe-commits
Which compiler/what warning was flagging this? It doesn't look like Clang builds by default with -Wconditional-uninitialized on, so I'm surprised if Clang was diagnosing anything here. In any case - it's probably better to omit the "WasAmbiguous" flag. Optional already has a flag that says

Re: r305860 - Special-case handling of destructors in override lists when dumping ASTs.

2017-07-10 Thread David Blaikie via cfe-commits
Ping for a response from Lang on Richard's CR feedback On Tue, Jun 20, 2017 at 3:30 PM Richard Smith via cfe-commits < cfe-commits@lists.llvm.org> wrote: > On 20 June 2017 at 14:30, Lang Hames via cfe-commits < > cfe-commits@lists.llvm.org> wrote: > >> Author: lhames >> Date: Tue Jun 20 16:30:43

Re: r305850 - Preserve CXX method overrides in ASTImporter

2017-07-10 Thread David Blaikie via cfe-commits
ping on CR feedback On Mon, Jun 26, 2017 at 7:02 PM David Blaikie wrote: > On Tue, Jun 20, 2017 at 2:06 PM Lang Hames via cfe-commits < > cfe-commits@lists.llvm.org> wrote: > >> Author: lhames >> Date: Tue Jun 20 16:06:00 2017 >> New Revision: 305850 >> >> URL:

Re: r305850 - Preserve CXX method overrides in ASTImporter

2017-06-26 Thread David Blaikie via cfe-commits
On Tue, Jun 20, 2017 at 2:06 PM Lang Hames via cfe-commits < cfe-commits@lists.llvm.org> wrote: > Author: lhames > Date: Tue Jun 20 16:06:00 2017 > New Revision: 305850 > > URL: http://llvm.org/viewvc/llvm-project?rev=305850=rev > Log: > Preserve CXX method overrides in ASTImporter > > Summary: >

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

2017-06-26 Thread David Blaikie via cfe-commits
On Mon, Jun 26, 2017 at 5:31 AM Serge Pavlov wrote: > 2017-06-26 4:05 GMT+07:00 David Blaikie : > >> Ah, I see now then. >> >> I have a symlink from the root of my source directory pointing to the >> compile_commands.json in my build directory. >> >> I

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

2017-06-25 Thread David Blaikie via cfe-commits
Ah, I see now then. I have a symlink from the root of my source directory pointing to the compile_commands.json in my build directory. I have this so that the vim YouCompleteMe plugin (& any other clang tools) can find it, as they usually should, for using tools with the llvm/clang project...

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

2017-06-24 Thread David Blaikie via cfe-commits
On Sat, Jun 24, 2017 at 10:08 AM Serge Pavlov wrote: > With CMAKE_EXPORT_COMPILE_COMMANDS the file compile_commands.json is > created in the directory > /tools/clang/tools/extra/test/clang-tidy/Output, > I'd be really surprised if this is the case - why would

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

2017-06-23 Thread David Blaikie via cfe-commits
Ping (+Manuel, perhaps he's got some ideas about this, given background in the tooling & compilation database work, or could point this to someone who does?) On Thu, Jun 15, 2017 at 10:40 AM David Blaikie wrote: >

Re: r305665 - clang-format: Add capability to format the diff on save in vim.

2017-06-19 Thread David Blaikie via cfe-commits
Ah, nevermind - I hadn't sync'd this change! Sorry for the noise. Thanks for the feature! On Mon, Jun 19, 2017 at 11:18 AM David Blaikie wrote: > On Mon, Jun 19, 2017 at 12:30 AM Daniel Jasper via cfe-commits < > cfe-commits@lists.llvm.org> wrote: > >> Author: djasper >>

Re: r305665 - clang-format: Add capability to format the diff on save in vim.

2017-06-19 Thread David Blaikie via cfe-commits
On Mon, Jun 19, 2017 at 12:30 AM Daniel Jasper via cfe-commits < cfe-commits@lists.llvm.org> wrote: > Author: djasper > Date: Mon Jun 19 02:30:04 2017 > New Revision: 305665 > > URL: http://llvm.org/viewvc/llvm-project?rev=305665=rev > Log: > clang-format: Add capability to format the diff on

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

2017-06-15 Thread David Blaikie via cfe-commits
On Mon, Jun 12, 2017 at 9:15 PM Dean Michael Berris via Phabricator < revi...@reviews.llvm.org> wrote: > dberris added a reviewer: dblaikie. > dberris added a subscriber: dblaikie. > dberris added a comment. > > @dblaikie -- do you have time to have a look? > Sure sure - no need to ping a patch

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,

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