[cfe-commits] [PATCH][MS][Review request] - Optional template keyword in Microsoft mode.

[Francois Pichet]

On Sun, Mar 27, 2011 at 6:09 AM, Douglas Gregor <dgregor at apple.com> wrote:
>
> On Mar 24, 2011, at 5:47 AM, Francois Pichet wrote:
>
>> Hi,
>>
>> Small parser hack to be able to parse type dependent template name
>> without the "template" keyword in Microsoft mode.
>> This patch fixes about a dozen compile errors when parsing MSVC STL
>> headers with clang.
>>
>> For example:
>> =========
>> template <class A>
>> class C1 {
>> public:
>>  template <int B>
>>  class Iterator {
>>  };
>> };
>>
>> template<class T>
>> class C2  {
>>  // template is optional in microsoft mode.
>>  typename C1<T>:: /*template*/  Iterator<0> Mypos;
>> };
>>
>> ok to commit?
>
> This particular parser recovery would be great, not just for Microsoft mode but in general. So, I'd like to see this handled by making the parser recover well from a missing 'template' keyword (diagnostic + Fix-It) and then either downgrading the diagnostic (to a warning) or omitting it entirely for Microsoft mode.
>
> +    // Microsoft mode parser hack to be able to parse type dependent
> +    // template name without the "template" keyword.
> +    bool IsImplicitTemplate = getLang().Microsoft && IsTypename &&
> +                              Tok.is(tok::identifier) &&
> +                              NextToken().is(tok::less) &&
> +                              (getCurScope()->getFlags() & Scope::DeclScope) &&
> +                              (HasScopeSpecifier || ObjectType);
> +
>     // nested-name-specifier:
>     //   nested-name-specifier 'template'[opt] simple-template-id '::'
>
>     // Parse the optional 'template' keyword, then make sure we have
>     // 'identifier <' after it.
> -    if (Tok.is(tok::kw_template)) {
> +    if (Tok.is(tok::kw_template) || IsImplicitTemplate) {
>       // If we don't have a scope specifier or an object type, this isn't a
>       // nested-name-specifier, since they aren't allowed to start with
>       // 'template'.
>
> I think this is the wrong place to perform the recovery, because we're forcing Clang to go down the dependent template-name path even in cases where we might be looking at a non-dependent template name.
>
> Instead, I suggest improving recovery down in the identifier case, by improving the heuristics for the current diagnostic that inserts a dependent 'template' keyword:
>
>      if (MemberOfUnknownSpecialization && (ObjectType || SS.isSet()) &&
>          IsTemplateArgumentList(1)) {
>        // We have something like t::getAs<T>, where getAs is a
>        // member of an unknown specialization. However, this will only
>        // parse correctly as a template, so suggest the keyword 'template'
>        // before 'getAs' and treat this as a dependent template name.
>        Diag(Tok.getLocation(), diag::err_missing_dependent_template_keyword)
>          << II.getName()
>          << FixItHint::CreateInsertion(Tok.getLocation(), "template ");
>
> It might be as easy as turning that last condition into "(IsTypename || IsTemplateArgumentList(1))".
>

Ok this works fine, committed in 128387