[cfe-commits] [patch] pr9786 assert-on-invalid non-final parameter packs

[David Blaikie]

[bump]

On Thu, Oct 13, 2011 at 10:09 PM, David Blaikie <dblaikie at gmail.com> wrote:

> [& of course I forget to include the patch. Attached now]
>
>
> On Thu, Oct 13, 2011 at 10:09 PM, David Blaikie <dblaikie at gmail.com>wrote:
>
>> From the bug:
>>
>> template<int ...Values, int After> struct X0nt;
>> X0nt<42> f();
>>
>>
>> Causes clang to fail an assertion (SemaTemplate.cpp:4868 - "Converted
>> template argument list is too short!"). This fix causes
>> CheckTemplateArgumentList to fail if there are parameter packs anywhere
>> other than the last argument in a non-partial application of the template
>> arguments (partial applications include function templates for which
>> non-final parameter packs may be valid. This change does not regress that
>> behavior as far as I know/as far as the test cases already cover it).
>>
>> I've included a test for this fix.
>>
>> As an added bonus, I've also fixed a minor quirk in the output of the
>> diagnostic that appears here ("template parameter pack must be the last
>> template parameter") - currently/without my patch, Clang produces this
>> diagnostic for every parameter after a parameter pack. So in the case of
>> "template<typename ...A, typename B, typename C> struct foo;" the error is
>> emitted twice (though, strangely, the second type it is produced it does not
>> provide the context/line location, though the line/col numbers are the same
>> & still correct). I've fixed that issue & modified a test case to catch it
>> too.
>>
>> Let me know if this looks good & I'll check it in,
>>
>> - David
>>
>> [for the record - gcc does something rather different with this invalid
>> construct. It produces special "invalid type" template arguments for
>> non-final parameter packs & then any instantiation of the template fails to
>> match any argument to that parameter & you get an extra diagnostic there,
>> too. I'm not entirely sure how Clang's error recovery works on the failure
>> path (returning Invalid from CheckTemplateArgumentList) works in this case
>> or how it compares to GCC's behavior.
>>
>> I was considering having the original template parsing code actually
>> modify the template parameter list when it hit this invalid case - moving
>> the parameter pack to the end (or removing it in the case of multiple
>> parameter packs) for example - but I wasn't sure how to mutate the template
>> parameter list & whether that would be valid. Not to mention it could
>> produce some rather strange diagnostics later on. Though it would be more
>> indicative of a legitimate 'error recovery' scenario (the compiler assuming
>> some valid interpretation of the invalid code & then continuing on its way -
>> rather than just glossing over the details of this invalid template whenever
>> it's used) & could be supported by appropriate FixItHints... (might be nice
>> to add the FixIt anyway at some point (to move the parameter pack to the
>> end))]
>>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20111017/95c93e27/attachment.html>