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

[David Blaikie]

[& 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/20111013/f1008716/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: pr9789.diff
Type: text/x-patch
Size: 4933 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20111013/f1008716/attachment.bin>