It would be awesome if this kind of shadowing warning could be put into
-Wall. My recollection on the last set of -Wshadow reviews is that most
shadowing warnings are from ctor arguments being used to initialize
members. Here's the last discussion / review regarding shadowing
http://reviews.llvm.org/D18271
On 6/15/2016 2:22 PM, Eric Fiselier wrote:
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
<mailto: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 <mailto: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
<mailto: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
--
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
