Re: r279485 - Module debug info: Don't assert when encountering an incomplete definition

Generally I'd expect a test case to exercise/demonstrate the behavior that was previously hidden behind an assertion. (put another way: "we expect something specific to happen, not just for clang not to assert/crash") On Mon, Aug 22, 2016 at 3:32 PM Adrian Prantl via cfe-commits < cfe-commits@list

Re: r279485 - Module debug info: Don't assert when encountering an incomplete definition

Might be good to mention in an email on this thread what the revision number of the missing test is (& hopefully that test commit also mentions the revision number of this change as well - so people can track through in either direction) On Tue, Aug 23, 2016 at 8:31 AM Adrian Prantl wrote: > Yes

r279651 - DebugInfo: Add flag to CU to disable emission of inline debug info into the skeleton CU

Author: dblaikie Date: Wed Aug 24 13:29:58 2016 New Revision: 279651 URL: http://llvm.org/viewvc/llvm-project?rev=279651&view=rev Log: DebugInfo: Add flag to CU to disable emission of inline debug info into the skeleton CU In cases where .dwo/.dwp files are guaranteed to be available, skipping t

r279687 - DebugInfo: Let -gsplit-dwarf and -gmlt compose if -fno-split-dwarf-inlining is used

Author: dblaikie Date: Wed Aug 24 18:22:36 2016 New Revision: 279687 URL: http://llvm.org/viewvc/llvm-project?rev=279687&view=rev Log: DebugInfo: Let -gsplit-dwarf and -gmlt compose if -fno-split-dwarf-inlining is used If the inline info is not duplicated into the skeleton CU, then there's value

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

I don't really follow how fixing uninitialized local variable warnings would be blocking plugin fixes for MSVC? On Sun, Aug 28, 2016 at 10:24 AM Sergio Martins wrote: > iamsergio added a subscriber: iamsergio. > iamsergio added a comment. > > @ariccio, are you blocked on this or waiting for revi

Re: r279765 - Omit column info for CodeView by default

On Thu, Aug 25, 2016 at 11:32 AM Adrian McCarthy via cfe-commits < cfe-commits@lists.llvm.org> wrote: > Author: amccarth > Date: Thu Aug 25 13:24:35 2016 > New Revision: 279765 > > URL: http://llvm.org/viewvc/llvm-project?rev=279765&view=rev > Log: > Omit column info for CodeView by default > > Cl

Re: r279765 - Omit column info for CodeView by default

ah, fair enough - only saw the diff, not the rest of the file On Mon, Aug 29, 2016 at 9:59 AM Adrian McCarthy wrote: > Thanks. I was trying to be consistent with how the prefixes were used in > this particular file, where the prefix was modeled after the case-sensitive > flags being tested. > >

r280290 - DebugInfo: Fix -gsplit-dwarf + -fno-split-dwarf-inlining

Author: dblaikie Date: Wed Aug 31 15:54:35 2016 New Revision: 280290 URL: http://llvm.org/viewvc/llvm-project?rev=280290&view=rev Log: DebugInfo: Fix -gsplit-dwarf + -fno-split-dwarf-inlining I tested the cases involving split-dwarf + gmlt + no-split-dwarf-inlining, but didn't verify the simpler

Re: [PATCH] D24152: Support the overloadable attribute with _Generic expressions

dblaikie added inline comments. Comment at: lib/Sema/SemaOverload.cpp:12996 @@ +12995,3 @@ + // selection expression. + std::vector AssocExprs(GSE->getAssocExprs().vec()); + unsigned ResultIdx = GSE->getResultIndex(); george.burgess.iv wrote: > Is t

Re: [PATCH] D24152: Support the overloadable attribute with _Generic expressions

On Fri, Sep 2, 2016 at 10:57 AM Aaron Ballman wrote: > aaron.ballman added inline comments. > > > Comment at: lib/Sema/SemaOverload.cpp:12996 > @@ +12995,3 @@ > + // selection expression. > + std::vector AssocExprs(GSE->getAssocExprs().vec()); > + unsigned ResultId

Re: r281198 - Add virtual destructor (necessary due to the switch to shared_ptr).

Does that actually need a virtual dtor? The type erasure handling in shared_ptr would handle it so long as each instance is constructed with the derived type (looks like it probably is - make_shared used consistently) & we should get a warning (or could make the base dtor protected non-virtual to e

Re: r281198 - Add virtual destructor (necessary due to the switch to shared_ptr).

wer to the MSVC-compatible engines. On Mon, Sep 12, 2016 at 10:57 AM Richard Smith wrote: > On Mon, Sep 12, 2016 at 10:55 AM, David Blaikie via cfe-commits < > cfe-commits@lists.llvm.org> wrote: > > Does that actually need a virtual dtor? The type erasure handling in > shared

Re: r281056 - Make -fstandalone-debug and -flimit-debug-info available in clang-cl

standalone and limit-debug-info are the same functionality - -fstandalone-debug is the more modern and apt description, so perhaps just keep that one & drop/don't provide -flimit-debug-info (since there's no real need to be backwards compatible in the flag support in clang-cl, I expect)? On Fri, S

Re: r281057 - [DebugInfo] Ensure complete type is emitted with -fstandalone-debug

Looks like this test could be a bit simpler: struct foo; foo *f; // produces a forward decl struct foo { virtual ~foo(); }; foo f; // requires the type to be complete A cursory glance looks like this reproduces the issue, but I may've missed something. Also the bracing in the new condition you

Re: [PATCH] D24426: DebugInfo: Pass non-zero alignment to DIBuilder only if aligment was forced

dblaikie added a comment. Like the backend patch, should/could this be broken up into separate patches for the different places/features that have alignment? (probably just passing zero for alignment in general could be a simple cleanup patch to start, if it's otherwise unused) ==

Re: [PATCH] D24426: DebugInfo: Pass non-zero alignment to DIBuilder only if aligment was forced

On Mon, Sep 12, 2016 at 5:00 PM Paul Robinson wrote: > probinson added a subscriber: probinson. > > > Comment at: lib/CodeGen/CGDebugInfo.cpp:3691 > @@ -3635,1 +3690,3 @@ > + if (D->hasAttr()) > +AlignInBits = D->getMaxAlignment(); >StringRef DeclName, LinkageName; > ---

Re: r281278 - [DebugInfo] Deduplicate debug info limiting logic

On Mon, Sep 12, 2016 at 5:09 PM Reid Kleckner via cfe-commits < cfe-commits@lists.llvm.org> wrote: > Author: rnk > Date: Mon Sep 12 19:01:23 2016 > New Revision: 281278 > > URL: http://llvm.org/viewvc/llvm-project?rev=281278&view=rev > Log: > [DebugInfo] Deduplicate debug info limiting logic > > W

Re: [PATCH] D24426: DebugInfo: Pass non-zero alignment to DIBuilder only if aligment was forced

e true? - David > --paulr > > P.S. The committee is hoping to get a draft out for public comment Real > Soon Now. > Looking forward to it :) > > > *From:* cfe-commits [mailto:cfe-commits-boun...@lists.llvm.org] *On > Behalf Of *David Blaikie via cfe-commits > *Sent

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

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&view=rev > Log: > Use arg_begin() instead of getArgumentList().begin(),

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

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 getArgumentList for simple t

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

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. > zhiz

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

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 via

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

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 an

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

Author: dblaikie Date: Wed Apr 5 11:50:19 2017 New Revision: 299561 URL: http://llvm.org/viewvc/llvm-project?rev=299561&view=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

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();

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

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 commen

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

Author: dblaikie Date: Tue Apr 11 15:46:34 2017 New Revision: 299982 URL: http://llvm.org/viewvc/llvm-project?rev=299982&view=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

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

Author: dblaikie Date: Tue Apr 11 16:13:37 2017 New Revision: 299987 URL: http://llvm.org/viewvc/llvm-project?rev=299987&view=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 te

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

Author: dblaikie Date: Wed Apr 12 15:58:33 2017 New Revision: 300104 URL: http://llvm.org/viewvc/llvm-project?rev=300104&view=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

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

Author: dblaikie Date: Wed Apr 12 16:09:34 2017 New Revision: 300105 URL: http://llvm.org/viewvc/llvm-project?rev=300105&view=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: cfe/trunk/

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

Author: dblaikie Date: Wed Apr 12 16:14:04 2017 New Revision: 300106 URL: http://llvm.org/viewvc/llvm-project?rev=300106&view=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.

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

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: > Author

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

Author: dblaikie Date: Mon Apr 17 12:16:19 2017 New Revision: 300461 URL: http://llvm.org/viewvc/llvm-project?rev=300461&view=rev Log: Use default ref capture to simplify local lambdas, use a template to avoid std::function overhead, other cleanup Modified: cfe/trunk/lib/AST/ExternalASTMerge

r291919 - unique_ptrify createDriverOptTable

Author: dblaikie Date: Fri Jan 13 11:34:15 2017 New Revision: 291919 URL: http://llvm.org/viewvc/llvm-project?rev=291919&view=rev Log: unique_ptrify createDriverOptTable Modified: cfe/trunk/include/clang/Driver/Driver.h cfe/trunk/include/clang/Driver/Options.h cfe/trunk/lib/Driver/Dri

r291938 - unique_ptrify Driver::ToolChains

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

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

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

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

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

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

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 that reaches the maili

Re: [PATCH] D28845: Prototype of modules codegen

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 bee

Re: [PATCH] D28845: Prototype of modules codegen

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 n

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

Author: dblaikie Date: Wed Jan 18 15:15:18 2017 New Revision: 292439 URL: http://llvm.org/viewvc/llvm-project?rev=292439&view=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 b

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

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

Re: r292632 - Fix actually-reachable llvm_unreachable.

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

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

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

Re: [PATCH] D28845: Prototype of modules codegen

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. One

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

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". LLVM's been this way for

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

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

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

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. > mehdi_amini added a comment. > This revision i

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

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 quality improvement, so > may be worth considering in the 4.0 branc

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

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

r293456 - Prototype of modules codegen

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

Re: [PATCH] D28845: Prototype of modules codegen

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 > +ExternalAST

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

Author: dblaikie Date: Sun Jan 29 23:33:51 2017 New Revision: 293457 URL: http://llvm.org/viewvc/llvm-project?rev=293457&view=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: cfe/tr

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

Author: dblaikie Date: Mon Jan 30 00:36:08 2017 New Revision: 293462 URL: http://llvm.org/viewvc/llvm-project?rev=293462&view=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 attempti

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

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&view=re

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

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" o

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

Message- > > 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 &

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

Author: dblaikie Date: Tue Jan 31 15:28:19 2017 New Revision: 293692 URL: http://llvm.org/viewvc/llvm-project?rev=293692&view=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 l

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

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. > > > A

r294512 - Initialize builtins during modular codegen

Author: dblaikie Date: Wed Feb 8 14:51:11 2017 New Revision: 294512 URL: http://llvm.org/viewvc/llvm-project?rev=294512&view=rev Log: Initialize builtins during modular codegen Added: cfe/trunk/test/Modules/Inputs/codegen-opt/ cfe/trunk/test/Modules/Inputs/codegen-opt/bar.h - copie

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

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 > c

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

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

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

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

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

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 have this so that the vim YouCompleteMe plug

Re: r305850 - Preserve CXX method overrides in ASTImporter

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&view=rev > Log: > Preserve CXX method overrides in ASTImporter > > Summar

Re: r305850 - Preserve CXX method overrides in ASTImporter

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: http://llvm.org/viewvc/llvm-projec

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

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: r306809 - Ambiguity might be also uninitialized. Use llvm::Optional.

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 whethe

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

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 l

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

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: http://llv

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

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 new PM has the expected

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

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: [clang-tools-extra] r303735 - Modify test so that it looks for patterns in stderr as well

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 add a test for this behavior anyway. >>

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

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 optim

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

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 sa

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

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 guess that GDB would

Re: r322405 - Disable test for Windows to fix Windows buildbots.

might be worth having an explanation (probably in both the commit message and in a comment in the source) about why this test isn't supported on windows? On Fri, Jan 12, 2018 at 1:50 PM Richard Trieu via cfe-commits < cfe-commits@lists.llvm.org> wrote: > Author: rtrieu > Date: Fri Jan 12 13:49:20

Re: r322350 - [ODRHash] Don't hash friend functions.

I'm surprised this problem is unique to friend functions with definitions inline and the friend declaration site - doesn't a similar issue occur with member functions of templates that are not instantiated in some (similar) contexts? Is there a common solution that could be used for both cases? O

Re: r322065 - Avoid assumption that lit tests are writable (in a couple more places). NFC

I feel like this sort of change will regress quite quickly - in the sense that most 'cp' in tests is probably there to put it somewhere writable (in some cases its to put it in a particular location, not about writability). Any way we could fix 'cp' in lit to do the right thing here? (I doubt any t

r323167 - NewPM: Improve/fix GCOV - which needs to run early in the pass pipeline.

Author: dblaikie Date: Mon Jan 22 17:25:24 2018 New Revision: 323167 URL: http://llvm.org/viewvc/llvm-project?rev=323167&view=rev Log: NewPM: Improve/fix GCOV - which needs to run early in the pass pipeline. Using a new extension point in the new PM, register GCOV at the start of the pipeline rat

Re: [clang-tools-extra] r323149 - [clangd] Drop ~destructor completions - rarely helpful and work inconsistently

Any chance of making this a really low priority completion? (maybe just leaving in a FIXME or expected-fail check of some kind if it's not practical to implement it right now) For those handful of times when you actually want to do this. On Mon, Jan 22, 2018 at 1:06 PM Sam McCall via cfe-commits <

Re: [clang-tools-extra] r323149 - [clangd] Drop ~destructor completions - rarely helpful and work inconsistently

Fair - maybe just a comment in the test case (and/or the source code that handles this) saying "hey, let's make sure dtors aren't here - but if/when there's support for a low priority completion group, these should appear there" or the like. On Mon, Jan 29, 2018 at 9:49 AM Sam McCall wrote: > On

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

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

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

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

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

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 v

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

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 *already* required to be globally

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

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 > > URL

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

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 moves helper functions that ar

r296386 - Remove unused variable

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

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

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 = FormatTest.BreaksLongD

r297322 - Defensively ensure that GetExternalDeclStmt protects itself from nested deserialization

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

Re: [PATCH] Warning for main returning a bool.

On Tue, Nov 15, 2016 at 4:50 PM Richard Smith via cfe-commits < cfe-commits@lists.llvm.org> wrote: > On Tue, Nov 15, 2016 at 2:55 PM, Aaron Ballman via cfe-commits < > cfe-commits@lists.llvm.org> wrote: > > On Tue, Nov 15, 2016 at 5:44 PM, Hal Finkel wrote: > > - Original Message - > >> F

Re: r289285 - Fix unused variable warnings. NFCI.

On Fri, Dec 9, 2016 at 2:55 PM Simon Pilgrim via cfe-commits < cfe-commits@lists.llvm.org> wrote: > Author: rksimon > Date: Fri Dec 9 16:45:21 2016 > New Revision: 289285 > > URL: http://llvm.org/viewvc/llvm-project?rev=289285&view=rev > Log: > Fix unused variable warnings. NFCI. > > Modified: >

Re: [PATCH] D27597: [DebugInfo] Restore test case for long double constants.

While it's possible to do arbitrary script things - we prefer nto to to ensure the tests are portable. (we have some custom implementations of common unix utilities for portability of those). In this case, can you xfail this on platforms that don't have the feature you want? rather than trying to

Re: [PATCH] D27597: [DebugInfo] Restore test case for long double constants.

On Mon, Dec 12, 2016 at 1:11 PM David Gross wrote: > I looked at what's supported by "requires", and couldn't find anything > appropriate. > > The problem is that I want the test to be sensitive to the size of long > double -- either no greater than 64 bits, or greater than 64 bits. It does > no

r300741 - Parse backend options during thinlto backend compile actions

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

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

Author: dblaikie Date: Fri Apr 21 18:35:36 2017 New Revision: 301063 URL: http://llvm.org/viewvc/llvm-project?rev=301063&view=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

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

\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> wrote

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

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

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