[Patch] Clang: allow scoped_lockable to acquire multiple locks

Delesley Hutchins delesley at google.com
Tue Sep 2 15:03:15 PDT 2014


You're okay to submit.  I will probably move the declarations around
slightly in a separate patch.  :-)

  -DeLesley



On Tue, Sep 2, 2014 at 2:19 PM, Ed Schouten <ed at 80386.nl> wrote:

> Hi Delesley, Aaron,
>
> On 2 September 2014 22:44, Delesley Hutchins <delesley at google.com> wrote:
> > (1) FactEntry needs a virtual destructor to avoid memory leaks.
>
> Done!
>
> > (2) I would like the different FactEntry classes to be declared together
> (at
> > the top of the file), rather than split as they are currently.
>
> The problem is that the *LockableFactEntry classes depend on FactSet
> and FactManager, whereas the FactSet and FactManager depend on the
> FactEntry.
>
> I could possibly declare them together, but in that case, the
> implementations of handle*() would need to go elsewhere. I'm usually
> not a big fan of that approach. Be sure to let me know if you'd prefer
> that anyway.
>
> On 2 September 2014 22:49, Aaron Ballman <aaron at aaronballman.com> wrote:
> > I would not reformat the entire file at this point (that tends to be
> > pretty noisy for little gain). Instead, you can run clang-format over
> > the patch file itself. There's a pretty simple script for doing this
> > described in the documentation:
> > http://clang.llvm.org/docs/ClangFormat.html
>
> That seems to work perfectly. I've just reformatted the code. Updated
> patch:
>
> http://80386.nl/pub/20140902-clang-scoped_lockable-2.txt
>
> Be sure to let me know if there's anything else that you'd like to see
> differently. I'll probably push in the change some time tomorrow
> morning/afternoon.
>
> Thanks,
> --
> Ed Schouten <ed at 80386.nl>
>



-- 
DeLesley Hutchins | Software Engineer | delesley at google.com | 505-206-0315
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140902/ad42dc15/attachment.html>


More information about the cfe-commits mailing list