<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Tue, May 14, 2013 at 1:35 AM, Richard Smith <span dir="ltr"><<a href="mailto:richard@metafoo.co.uk" target="_blank">richard@metafoo.co.uk</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div>On Mon, May 13, 2013 at 10:25 PM, Faisal Vali <span dir="ltr"><<a href="mailto:faisalv@gmail.com" target="_blank">faisalv@gmail.com</a>></span> wrote:<br>
</div><div class="gmail_quote"><div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<div dir="ltr">First off, this patch is far from ready for committing - it does not include<br>enough tests - includes commented out code - and even some dead<br>code. I shall try to address all those issues in the future - but for now<br>
I just wanted to put this out there for some early feedback (before I go<br> any deeper down a rabbit hole I need not have crawled into)<br></div></blockquote><div><br></div></div><div>Just a couple of thoughts below, I've not had time to study the patch itself yet.</div>
<div>
<div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr">The patch implements the following (an incomplete list):<br> - converting auto parameters to template parameters and creating a<br>
function call operator template<br> - It does this by replacing the typesourceinfo of the parameter -<br> Should I avoid doing this? Is there a way to transform the auto<br> into a template type parameter, so that it subsequently behaves<br>
as a template type parameter, but remembers that it was<br> once an auto (for error messages only, right?)<br></div></blockquote><div><br></div></div><div>Perhaps an AutoType whose deduced type is the template parameter would work? However, note this comment from SubstituteAutoTransform:</div>
<div><br></div><div><div> // If we're building the type pattern to deduce against, don't wrap the</div><div> // substituted type in an AutoType. Certain template deduction rules</div><div> // apply only when a template type parameter appears directly (and not if</div>
<div> // the parameter is found through desugaring). For instance:</div><div> // auto &&lref = lvalue;</div><div> // must transform into "rvalue reference to T" not "rvalue reference to</div>
<div> // auto type deduced as T" in order for [temp.deduct.call]p3 to apply.</div></div><div><br></div><div>If we wanted that to work, we'd need to teach some parts of template deduction to walk over the AutoType node.</div>
<div><br></div><div>How significant is the diagnostic impact of performing the substitution? Where, and how often, does it show up?</div><div><div></div></div></div></blockquote><div><br><br></div><div>It shows up if deduction fails - for e.g:<br>
<br>auto L = [](auto *a) -> void { };<br>L(1);<br><br>f:\clang-trunk\clang-trunk-fv\tests\test-lambda-captures-lab.cpp:77:3: <br>error: no matching function for call to object of type '<lambda at<br> f:\clang-trunk\clang-trunk-fv\tests\test-lambda-captures-lab.cpp:76:12>'<br>
L(1);<br> ^<br>f:\clang-trunk\clang-trunk-fv\tests\test-lambda-captures-lab.cpp:76:12: <br>note: candidate template ignored: could not match 'type-parameter-0-0 *' <br>against 'int'<br> auto L = [](auto *a) -> void { };<br>
</div><div><br><br></div><div>I could live with that - but if we didn't want to - asides from considering the approach <br>you presented, what are your thoughts about the following approach :<br>when emitting the error, check to see if it is a parameter of a generic lambda <br>
and sneak the auto back into the diagnositc - pros and cons?<br>
<br> </div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div class="gmail_quote"><div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<div dir="ltr"> - it makes TransformLambdaExpr and InstantiateFunctionDefinition<br> generic lambda aware (to allow for transformation of nested lambdas)<br>
so that the <br> - Regarding capturing for generic lambdas:<br> 1) A generic lambda (nested or not) will only have its captures<br> computed/finalized when its enclosing context is non-dependent <br> (and if an empty capture list, then no captures).<br>
2) When a generic lambda is invoked (i.e a specialization is instantiated) <br> any variable from an outer scope that is odr-used, but not already <br> captured by the lambda expression, results in an arror. <br>
3) If an outer lambda that is enclosed in a non-dependent context,<br> can't tell whether an inner lambda (with an implicit capture) <br> or a tree of inner lambdas (all with implicit captures) <br> might need to capture the variable for some<br>
instantiation of that inner lambda, the outer lambda will<br> capture that variable (if it has a capture default).<br> - this is implemented by hooking into ActOnCallExpr<br> (is it enough to assume that the only cases where an inner<br>
lambda might potentially need to capture a variable is <br> if it is used in a dependent function call (ctor, operator) <br> of the inner lambda, in a non-unevaluated context?)<br></div>
</blockquote><div><br></div></div><div>Per the proposal, you need captures in other situations:</div><div><br></div><div>struct S {</div><div> constexpr S() {}</div><div> S(const S&) { puts("captured"); }</div>
<div>
};</div><div>void f() {</div><div> constexpr S s {};</div><div> [=] (auto x) {</div><div> // captures 's' (even though no instantiation can need it),</div><div> // because the full-expression depends on 'x'. Therefore</div>
<div> // we must print "captured".</div><div> (void)x, s;</div><div><div> };</div><div>}</div><div><br></div></div></div></blockquote><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<div class="gmail_quote">
<div><div></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<div dir="ltr"> - Within ActOnCallExpr, I dig into each argument, and as<br>
long as they are not in an unevaluated context (currently<br> I only check for sizeof), and are from a scope outside<br> the outer lambda, I capture the variable within the outer <br> lambda (if both the inner and outer lambda have a cap default,<br>
and the outer lambda is enclosed by a non-dependent context).<br> - The reason I do this in ActOnCallExpr is because until then<br> I do not know if the function call is still dependent, perhaps<br>
there is a better place to do this?</div></blockquote><div><br></div></div><div>Here's how I was imagining this working when we were in CWG:</div><div><br></div><div>The captures for a lambda are not finalized until the outermost context of the reaching scope is non-dependent. At that point, for each call to ActOnFinishFullExpr within the lambda, we check whether the full-expression is instantiation-dependent. If it is, we scan it for entities which it might need to capture, and capture all of them. There may be ways of optimizing this (maybe keep a list of the potentially-captured variables for each full-expression as we build it).</div>
<div><div>
<div style="text-align:left"><u><br></u></div></div></div></div></blockquote><div><br></div><div>Aah! ActOnFinishfullExpr was exactly the hook I was looking for - thanks! <br>So the new patch implements it as you describe (and it does do the optimization <br>
- I introduced an array of potential captures in the LambdaScopeInfo) so we <br></div><div>don't have to walk the expression tree each time. It seems to work for your <br>case and my previous tests.<br></div><div><br>
</div><div>While we're on the topic, can i ask you to clarify a few capture and constexpr <br>questions below:<br><br><div>struct S {</div><div> constexpr S() {}</div><div> constexpr operator int() const { return 0; }<br>
</div><div>};<br></div><div>void fooS(S s) { }<br></div><div><br>void fooInt(int i) { }<br><br></div><div>void f() {</div><div> constexpr S s {};</div><div> auto L = [] (int x) {</div> (void) s; // 1<br></div><div>
fooS(s); // 2<br></div><div> fooInt(s); // 3<br></div><div><div> };<br></div><div>}<br><br></div><div>Should the above be ok in spite of L not having a default capture?<br></div>Currently clang errors on all of them individually - is that the right behavior?<br>
<br>Also, how would you want me to start submitting patches for commit - do you want <br>me to break up the patch into smaller patches? - and if so, do you have any thoughts <br>on how I might want to break up the functionality per patch?<br>
<br></div><div>thanks!<br></div><div><br></div><div><br></div></div></div></div>