[PATCH] Addresses TemplateParameterDepth calculation of Local Member templates

[Richard Smith]

+  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>