[prev in list] [next in list] [prev in thread] [next in thread]
List: cfe-commits
Subject: Re: [PATCH] D11194: Instantiate function declarations in instantiated functions.
From: Richard Smith via cfe-commits <cfe-commits () lists ! llvm ! org>
Date: 2015-08-19 1:25:07
Message-ID: CAOfiQq=2RaebzTu-orVUQttwW+uyPjo84N9RkDJg=jvrpfwJjw () mail ! gmail ! com
[Download RAW message or body]
[Attachment #2 (multipart/alternative)]
On Tue, Aug 18, 2015 at 10:05 AM, Serge Pavlov <sepavloff@gmail.com> wrote:
> sepavloff added inline comments.
>
> ================
> Comment at: lib/AST/DeclBase.cpp:273
> @@ +272,3 @@
> + return true;
> + if (const CXXRecordDecl *ClassD = dyn_cast<CXXRecordDecl>(LDC))
> + return ClassD->isLocalClass() && !ClassD->isLambda();;
> ----------------
> rsmith wrote:
> > It's not necessary for this change, but to match its documentation this
> function should handle other kinds of `TagDecl` too (enums, C structs).
> Something like:
> >
> > do {
> > if (LDC->isFunctionOrMethod())
> > return true;
> > if (!isa<TagDecl>(LDC))
> > return false;
> > LDC = LDC->getLexicalParent();
> > } while (LDC);
> > return false;
> >
> > ... maybe?
> The proposed code recognizes lambda as lexically contained within a
> function. As a result, the following test starts to fail (obtained from
> CXX\expr\expr.prim\expr.prim.lambda\default-arguments.cpp):
> ```
> struct NoDefaultCtor {
> NoDefaultCtor(const NoDefaultCtor&); // expected-note{{candidate
> constructor}}
> ~NoDefaultCtor();
> };
>
> template<typename T>
> void defargs_in_template_unused(T t) {
> auto l1 = [](const T& value = T()) { };
> l1(t);
> }
>
> template void defargs_in_template_unused(NoDefaultCtor);
> ```
> because default value for lambda argument cannot be instantiated. It is
> not clear whether instantiation of the lambda default argument must always
> occur similar to DR1484. I couldn't find appropriate place in the standard.
> According to spirit it shouldn't as lambda is not a separate declaration
> but a part of instantiated content.
I agree. The default argument must be instantiated as part of instantiating
the surrounding function.
> If so 14.6.4.1p2 is more likely to be applied and the above test must
> pass.
>
The call operator of the lambda is neither a function template nor a member
function of a class template. The relevant rule is 14.7.1/1: "Within a
template declaration, a local class or enumeration and the members
of a local class are never considered to be entities that can be separately
instantiated (this includes their
default arguments, exception-specifications, and non-static data member
initializers, if any)." The lambda expression defines a local class, so its
members' default arguments are instantiated with the surrounding function.
> Maybe we need to rename `isLexicallyWithinFunctionOrMethod` to
> `shouldBeAlwaysInstantiated` or something like that to avoid
> misunderstanding?
>
>
>
> http://reviews.llvm.org/D11194
>
>
>
>
[Attachment #5 (text/html)]
<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Tue, Aug 18, 2015 \
at 10:05 AM, Serge Pavlov <span dir="ltr"><<a href="mailto:sepavloff@gmail.com" \
target="_blank">sepavloff@gmail.com</a>></span> wrote:<br><blockquote \
class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc \
solid;padding-left:1ex">sepavloff added inline comments.<br> <span class=""><br>
================<br>
Comment at: lib/AST/DeclBase.cpp:273<br>
@@ +272,3 @@<br>
+ return true;<br>
+ if (const CXXRecordDecl *ClassD = dyn_cast<CXXRecordDecl>(LDC))<br>
+ return ClassD->isLocalClass() && !ClassD->isLambda();;<br>
----------------<br>
</span><span class="">rsmith wrote:<br>
> It's not necessary for this change, but to match its documentation this \
function should handle other kinds of `TagDecl` too (enums, C structs). Something \
like:<br> ><br>
> do {<br>
> if (LDC->isFunctionOrMethod())<br>
> return true;<br>
> if (!isa<TagDecl>(LDC))<br>
> return false;<br>
> LDC = LDC->getLexicalParent();<br>
> } while (LDC);<br>
> return false;<br>
><br>
> ... maybe?<br>
</span>The proposed code recognizes lambda as lexically contained within a function. \
As a result, the following test starts to fail (obtained from \
CXX\expr\expr.prim\expr.prim.lambda\default-arguments.cpp):<br> ```<br>
struct NoDefaultCtor {<br>
NoDefaultCtor(const NoDefaultCtor&); // expected-note{{candidate \
constructor}}<br> ~NoDefaultCtor();<br>
};<br>
<br>
template<typename T><br>
void defargs_in_template_unused(T t) {<br>
auto l1 = [](const T& value = T()) { };<br>
l1(t);<br>
}<br>
<br>
template void defargs_in_template_unused(NoDefaultCtor);<br>
```<br>
because default value for lambda argument cannot be instantiated. It is not clear \
whether instantiation of the lambda default argument must always occur similar to \
DR1484. I couldn't find appropriate place in the standard. According to spirit it \
shouldn't as lambda is not a separate declaration but a part of instantiated \
content.</blockquote><div><br></div><div>I agree. The default argument must be \
instantiated as part of instantiating the surrounding function.</div><div> \
</div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc \
solid;padding-left:1ex">If so 14.6.4.1p2 is more likely to be applied and the above \
test must pass.<br></blockquote><div><br></div><div>The call operator of the lambda \
is neither a function template nor a member function of a class template. The \
relevant rule is 14.7.1/1: "Within a template declaration, a local class or \
enumeration and the members</div><div>of a local class are never considered to be \
entities that can be separately instantiated (this includes their</div><div>default \
arguments, exception-specifications, and non-static data member initializers, if \
any)." The lambda expression defines a local class, so its members' default \
arguments are instantiated with the surrounding function.</div><div> \
</div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc \
solid;padding-left:1ex"> Maybe we need to rename `isLexicallyWithinFunctionOrMethod` \
to `shouldBeAlwaysInstantiated` or something like that to avoid misunderstanding?<br> \
<br> <br>
<br>
<a href="http://reviews.llvm.org/D11194" rel="noreferrer" \
target="_blank">http://reviews.llvm.org/D11194</a><br> <br>
<br>
<br>
</blockquote></div><br></div></div>
[Attachment #6 (text/plain)]
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic