I've had a change of heart. I think that lock_guard<> has some utility in
generic code, and I'm not sure removing it is a good idea. For example a
function like:
template <class Func, class ...Locks>
void ExecuteUnderLocks(Func&& fn, Locks&... locks) {
lock_guard<Locks...> g(locks...);
fn();
}
I checked the proposal and it's clear that "lock_guard<>" is expected to
compile and be default constructable. For this reason I'm not going to
remove "lock_guard<>", at least not without further discussion.
On Wed, Jun 15, 2016 at 12:47 PM, Craig, Ben <ben.cr...@codeaurora.org>
wrote:
> 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@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.
>
Exactly.
>
> 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?
>
It breaks the ABI any time "lock_guard<Mutex>" participates in the mangling
of some function or type. In addition to your example this will also break
any time "lock_guard<Mutex>" is used as a template parameter: ie
using T = MyType<lock_guard<Mutex>>;
MyFunction<lock_guard<Mutex>>();
The two different implementations are still layout compatible, so if
mangling were not an issue I think this change would have been safe.
> --
> Employee of Qualcomm Innovation Center, Inc.
> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux
> Foundation Collaborative Project
>
>
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
