<div dir="ltr">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.<div>A separate warning for shadowing 'x' caused by "T(x)" might be useful.</div><div><br></div><div><div><div>Do people actually use "T(x)" in the wild to default construct 'x'?<br><div><br></div><div>/Eric<br><div><br></div></div></div></div></div></div><div class="gmail_extra"><br><div class="gmail_quote">On Wed, Jun 15, 2016 at 1:07 PM, Craig, Ben <span dir="ltr"><<a href="mailto:ben.craig@codeaurora.org" target="_blank">ben.craig@codeaurora.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
  
    
  
  <div bgcolor="#FFFFFF" text="#000000">
    <p>Makes sense.  Here's hoping parameter deduction for constructors
      makes it in!</p>
    <p>(better link)
      <a href="http://open-std.org/JTC1/SC22/WG21/docs/papers/2016/p0091r2.html" target="_blank">http://open-std.org/JTC1/SC22/WG21/docs/papers/2016/p0091r2.html</a><br>
    </p><div><div class="h5">
    <br>
    <div>On 6/15/2016 1:54 PM, Eric Fiselier
      wrote:<br>
    </div>
    <blockquote type="cite">
      <div dir="ltr"><br>
        <div>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:
          <div><br>
          </div>
          <div>template <class Func, class ...Locks></div>
          <div>void ExecuteUnderLocks(Func&& fn, Locks&...
            locks) {</div>
          <div>  lock_guard<Locks...> g(locks...);</div>
          <div>  fn();</div>
          <div>}</div>
          <div><br>
          </div>
          <div>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.</div>
          <div> </div>
        </div>
        <div class="gmail_extra"><br>
          <div class="gmail_quote">On Wed, Jun 15, 2016 at 12:47 PM,
            Craig, Ben <span dir="ltr"><<a href="mailto:ben.craig@codeaurora.org" target="_blank">ben.craig@codeaurora.org</a>></span>
            wrote:<br>
            <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
              <div bgcolor="#FFFFFF" text="#000000"><span> On
                  6/15/2016 1:15 PM, Eric Fiselier wrote:<br>
                  <blockquote type="cite">
                    <div dir="ltr">On Wed, Jun 15, 2016 at 11:45 AM,
                      Craig, Ben via cfe-commits <span dir="ltr"><<a href="mailto:cfe-commits@lists.llvm.org" target="_blank"></a><a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a>></span>
                      wrote:<br>
                      <div class="gmail_extra">
                        <div class="gmail_quote">
                          <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
                            <div bgcolor="#FFFFFF" text="#000000">
                              <p>Does this change (and the paper) permit
                                declarations like the following?</p>
                              <p>    lock_guard<> guard();</p>
                              <p>If that syntax is allowed, then this is
                                also likely allowed...</p>
                              <p>    lock_guard<>(guard);</p>
                              <p>I would really like the prior two
                                examples to not compile.  Here is a
                                common bug that I see in the wild...</p>
                              <p>   
                                unique_guard<mutex>(some_member_mutex);</p>
                              <p>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.<br>
                              </p>
                            </div>
                          </blockquote>
                          <div><br>
                          </div>
                          <div>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.</div>
                        </div>
                      </div>
                    </div>
                  </blockquote>
                </span> It's also strong rationale for deduced
                constructor templates. (<a href="http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2015/p0091r0.html" target="_blank">http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2015/p0091r0.html</a>)<br>
                auto guard = unique_guard(some_member_mutex); <br>
                You don't need to repeat types there, and it's very
                difficult to forget to name the guard variable.<span><br>
                  <br>
                  <blockquote type="cite">
                    <div dir="ltr">
                      <div class="gmail_extra">
                        <div class="gmail_quote">
                          <div> </div>
                          <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
                            <div bgcolor="#FFFFFF" text="#000000">
                              <p> </p>
                              <p>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?<br>
                              </p>
                            </div>
                          </blockquote>
                          <div>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.</div>
                          <div>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.</div>
                          <div><br>
                          </div>
                          <div>There is actually no recursion in the
                            variadic lock_guard implementation, so the
                            change is trivial.</div>
                          <div><br>
                          </div>
                          <div>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).</div>
                        </div>
                      </div>
                    </div>
                  </blockquote>
                </span> 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.<br>
              </div>
            </blockquote>
            <div><br>
            </div>
            <div>Exactly.</div>
            <div> </div>
            <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
              <div bgcolor="#FFFFFF" text="#000000"> <br>
                I'm also going to guess that throwing inline namespaces
                at the problem won't help, as that would probably cause
                compile-time ambiguity.<br>
                <br>
                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?</div>
            </blockquote>
            <div><br>
            </div>
            <div>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</div>
            <div><br>
            </div>
            <div>using T = MyType<lock_guard<Mutex>>;</div>
            <div>MyFunction<lock_guard<Mutex>>();</div>
            <div><br>
            </div>
            <div>The two different implementations are still layout
              compatible, so if mangling were not an issue I think this
              change would have been safe.</div>
            <div><br>
            </div>
            <div> </div>
            <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
              <div bgcolor="#FFFFFF" text="#000000"> <span>
                  <pre cols="72">-- 
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project
</pre>
                </span></div>
            </blockquote>
          </div>
          <br>
        </div>
      </div>
    </blockquote>
    <br>
    <pre cols="72">-- 
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project
</pre>
  </div></div></div>

</blockquote></div><br></div>