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.

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

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

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

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 cmake/ninja/makefiles put the compil

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

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, but the tests > from /llvm/tools/clang/tools/extra/test/clang-tidy run in the > directory /tool

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

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: > https://sarcasm.github.io/notes/dev/compilation-database.html#cmake > > If you ena

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

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 >> Date: Mon Jun 19 02:30:

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

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

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 m

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

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

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 >> compile_commands.json file). >> >> Looks like what hap

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

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 for

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

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

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. /usr/local/google/home/blaikie/dev/llvm/src/tools/clang/tools/e

Re: r305182 - Revert r305164/5/7.

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

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.

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&view=rev > Log: > Add regressio

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

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 testc

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

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

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

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: > > - We have gmock headers now,

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

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 t

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

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 wrote: > > > > > > On Tue, Jun 6, 2017 at 3:5

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

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? Weth

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

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&view=rev based on a > > compiler wa

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

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 fa

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

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

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

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&view=rev > Log: > [clang-tidy] Add `const

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

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 w

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

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: > test/CodeGenCoroutines/coro-await-domination.cp

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

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 Phab

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

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&view=rev > Log: > Diagnose attempts to build a preprocessed module th

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

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 > test/SemaCXX/coreturn.cpp > test/SemaCXX

r304456 - Add compatibility alias for -Wno-#warnings

Author: dblaikie Date: Thu Jun 1 14:08:34 2017 New Revision: 304456 URL: http://llvm.org/viewvc/llvm-project?rev=304456&view=rev Log: Add compatibility alias for -Wno-#warnings GCC uses -Wno-cpp for this, so seems reasonable to add an alias to match. Modified: cfe/trunk/include/clang/Basic/

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

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 email - hope everybody sees

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

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 throu

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

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: "CurCoro.Data->FinalJD =

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

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 acts as a > jump target fo

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

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 ch

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

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

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

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 < cfe-commi

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

Author: dblaikie Date: Sun Apr 30 21:11:39 2017 New Revision: 301786 URL: http://llvm.org/viewvc/llvm-project?rev=301786&view=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: cfe/trunk/test/SemaCXX/c

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

Author: dblaikie Date: Fri Apr 28 15:50:25 2017 New Revision: 301684 URL: http://llvm.org/viewvc/llvm-project?rev=301684&view=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 pas

Re: r301470 - Fix API breaks

t the time seems fine :) > Reverted my revert in r301472. > Thanks! > > vedant > > > > > > > On Wed, Apr 26, 2017 at 2:14 PM Vedant Kumar wrote: > > Hi David, > > > > It looks like this conflicts with my revert. I will revert my revert,

Re: r301470 - Fix API breaks

ant > > > 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-project?rev

r301470 - Fix API breaks

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

[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

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

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

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

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

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

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.

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/

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

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

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

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

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

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

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] 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: 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: 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(),

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

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

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] 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: [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

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

[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

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

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

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

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

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

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

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

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

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

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

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+

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

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

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

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

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

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