[PATCH] Addresses TemplateParameterDepth calculation of Local Member templates
Richard Smith
richard at metafoo.co.uk
Sat Apr 27 15:52:31 PDT 2013
+ assert( Actions.getDiagnostics().hasErrorOccurred() ||
+ (!dyn_cast<FunctionTemplateDecl>(LM.D) ||
+ (dyn_cast<FunctionTemplateDecl>(LM.D)->getTemplateParameters()->
+ getDepth() < TemplateParameterDepth)) &&
"TemplateParameterDepth"
+ " should be greater than the depth of current template being"
+ " instantiated!");
No space after (, extra space on the next line, and this will generate a ||
versus && precedence warning. Also, would be more consistent to reflow the
string literal to start on its own line.
+ // getNumTemplateParameterLists returns the number of TPLs
+ // minus the TPL of the actual function being instantiated
+ // i.e. consider a nested member class template with
+ // a template member of a function defined out of class ... its
+ // associated TPLs
+ // Therefore we add 1 to the depth for the Declarator itself
+ // and the rest for the outer TPLs returned by getNumTPLs
Something seems to be missing from the first part of this comment. In any
case, IIRC there's a patch in flight which removes this code and makes it
call ActOnReenterTemplateScope the right number of times.
+ CurTemplateDepthTracker.addDepth( 1 + NumTPLs );
No spaces inside parens.
A if (Tok.is(tok::kw_try)) {
Random 'A' in the patch?
+ assert( (!FunTmplD ||
+ (FunTmplD->getTemplateParameters()->getDepth()
+ < TemplateParameterDepth)) && "TemplateParameterDepth should
"
+ " be greater than the depth of current template being "
+ "instantiated!");
No space inside parens, and please start the string literal on a new line.
On Thu, Apr 25, 2013 at 12:08 PM, Faisal Vali <faisalv at gmail.com> wrote:
> Hi Richard,
> let me know what you think of this patch to address the template
> parameter depth tracking issue.
> thanks!
>
> Faisal Vali
>
>
>
> On Mon, Apr 22, 2013 at 8:01 AM, Richard Smith <richard at metafoo.co.uk>wrote:
>
>> Hi Faisal,
>>
>> I'd prefer putting the TemplateParameterDepthRAII code next to the calls
>> to ActOnReenterTemplateScope (unless there's some reason for them to be
>> separated?). If you increase the depth once for each
>> ActOnReenterTemplateScope call, there shouldn't ever be a need to bump it
>> by more than one. That should also handle the cases of a generic lambda in
>> a default argument in a method or class template, and a generic lambda /
>> local class w/member template in a non-template function in a class
>> template, which I think the current patch won't get right.
>>
>> + // ok here we need to make sure that the depth of the template
>> parameter list
>> + // of a potential member function template is adjusted for the template
>> + // parameter depth - in case we have local classes with member
>> templates
>>
>> Please start comments with a capital letter and end them with a full
>> stop. Also drop the "ok" :)
>>
>> + TemplateParameterDepthRAII
>> CurTemplateDepthTracker(TemplateParameterDepth);
>> + FunctionTemplateDecl *FTD = 0;
>> + if (LM.TemplateScope && (FTD = dyn_cast<FunctionTemplateDecl>(LM.D))) {
>> + TemplateParameterList *TPL = FTD->getTemplateParameters();
>> + unsigned MemberTemplateDepth = TPL->getDepth();
>> + while( TemplateParameterDepth <= MemberTemplateDepth )
>>
>> Space after "while", no spaces immediately inside parens.
>>
>> Index: test/CXX/generic-lambdas/fv_print.h
>>
>> Subdirectories inside test/CXX should correspond to sections of the
>> standard. The test itself should be cleaned up prior to commit: remove the
>> printf-based testing mechanism, and don't use lli. If you really want to
>> test CodeGen rather than just Sema (and I think you can avoid that), you
>> should FileCheck the -emit-llvm output. Otherwise, try to craft a -verify
>> test, and put it in test/SemaTemplate.
>>
>> Thanks!
>>
>> On Mon, Apr 22, 2013 at 4:40 AM, Faisal Vali <faisalv at gmail.com> wrote:
>>
>>> Hi Doug,
>>> I had mentioned this issue to you briefly in Bristol having to do
>>> tangentially with generic lambdas: the template parameter depth does not
>>> get calculated correctly when local member templates are parsed. This
>>> patch addresses that issue. I know local member templates are not
>>> standard, but some of the machinery I will be using to implement
>>> incrementing depth appropriately within generic lambdas is included. This
>>> only addresses the issue of template argument deduction for local member
>>> templates - other issues (and i don't remember what they are exactly now -
>>> i think you had showed me an example in Portland...) will probably get
>>> addressed as the generic lambda implementation moves along (if we care to
>>> fix local member templates entirely that is).
>>>
>>> Will refine the patch based on feedback.
>>>
>>> Thanks!
>>>
>>> Faisal Vali
>>>
>>>
>>> _______________________________________________
>>> cfe-commits mailing list
>>> cfe-commits at cs.uiuc.edu
>>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>>>
>>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130427/8f97112b/attachment.html>
More information about the cfe-commits
mailing list