Maybe we should add a new warning in Clang for this. -Wshadow diagnosis's this but -Wshadow isn't a part of -Wall or -Wextra so it's of limited utility. A separate warning for shadowing 'x' caused by "T(x)" might be useful.
Do people actually use "T(x)" in the wild to default construct 'x'? /Eric On Wed, Jun 15, 2016 at 1:07 PM, Craig, Ben <ben.cr...@codeaurora.org> wrote: > Makes sense. Here's hoping parameter deduction for constructors makes it > in! > > (better link) > http://open-std.org/JTC1/SC22/WG21/docs/papers/2016/p0091r2.html > > On 6/15/2016 1:54 PM, Eric Fiselier wrote: > > > 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>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 >> >> > > -- > 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