[libcxx] r272634 - Implement variadic lock_guard.

Craig, Ben via cfe-commits cfe-commits at lists.llvm.org
Wed Jun 15 11:47:29 PDT 2016


On 6/15/2016 1:15 PM, Eric Fiselier wrote:
> On Wed, Jun 15, 2016 at 11:45 AM, Craig, Ben via cfe-commits 
> <cfe-commits at lists.llvm.org <mailto:cfe-commits at lists.llvm.org>> wrote:
>
>     Does this change (and the paper) permit declarations like the
>     following?
>
>         lock_guard<> guard();
>
>     If that syntax is allowed, then this is also likely allowed...
>
>         lock_guard<>(guard);
>
>     I would really like the prior two examples to not compile.  Here
>     is a common bug that I see in the wild...
>
>         unique_guard<mutex>(some_member_mutex);
>
>     That defines a new, default constructed unique_guard named
>     "some_member_mutex", that likely shadows the member variable
>     some_member_mutex.  It is almost never what users want.
>
>
> I had no idea that syntax did that. I would have assumed it created an 
> unnamed temporary. I can see how that would cause bugs.
It's also strong rationale for deduced constructor templates. 
(http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2015/p0091r0.html)
auto guard = unique_guard(some_member_mutex);
You don't need to repeat types there, and it's very difficult to forget 
to name the guard variable.

>     Is it possible to have the empty template remain undefined, and
>     let the one element lock_guard be the base case of the recursion? 
>     Does that help any with the mangling?
>
> Nothing in the spec says the empty template should be undefined. The 
> default constructor on the empty template is technically implementing 
> "lock_guard(MutexTypes...)" for an empty pack.
> However your example provides ample motivation to make it undefined. 
> I'll go ahead and make that change and I'll file a LWG defect to 
> change the standard.
>
> There is actually no recursion in the variadic lock_guard 
> implementation, so the change is trivial.
>
> As for mangling I'm not sure what you mean? It definitely doesn't 
> change the fact that this change is ABI breaking. (Note this change is 
> not enabled by default for that reason).
My thought regarding the mangling was that you could still provide a one 
argument lock_guard, as well as a variadic lock_guard.  The one argument 
lock_guard would have the same mangling as before.  I think some of your 
other comments have convinced me that that won't work, as I think the 
variadic lock_guard has to be made the primary template, and I think the 
primary template dictates the mangling.

I'm also going to guess that throwing inline namespaces at the problem 
won't help, as that would probably cause compile-time ambiguity.

If I'm not mistaken, this only breaks ABI for those foolish enough to 
pass a lock_guard reference or pointer as a parameter across a libcxx 
version boundary.  Does that sound accurate?

-- 
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20160615/061429ff/attachment.html>


More information about the cfe-commits mailing list