[PATCH] Don't parse non-template name as template name

[Richard Smith]

On Thu Jan 30 2014 at 2:38:44 PM, Ismail Pazarbasi <
ismail.pazarbasi at gmail.com> wrote:

> Hi rsmith,
>
> PR18127: misparse of 'new struct S < ...' where S is not a template-name
>
> If clang sees '<' token after an elaborated-type-specifier, it assumes
> elaborated-type-specifier refers to a template name, and then issues
> diagnostics concerning missing '>'.
>
> We try to find corresponding '>' for diagnostics location purpose, and
> perform name lookup. Parser knows that it is not parsing a template. If
> name lookup doesn't find a name, we will keep issuing `explicit
> instantiation/specialization of non-template class` error.
>
> http://llvm-reviews.chandlerc.com/D2662
>
> Files:
>   lib/Parse/ParseDeclCXX.cpp
>   test/Parser/cxx-template-argument.cpp
>
> Index: lib/Parse/ParseDeclCXX.cpp
> ===================================================================
> --- lib/Parse/ParseDeclCXX.cpp
> +++ lib/Parse/ParseDeclCXX.cpp
> @@ -24,6 +24,8 @@
>  #include "clang/Sema/Scope.h"
>  #include "clang/Sema/SemaDiagnostic.h"
>  #include "llvm/ADT/SmallString.h"
> +#include "clang/Sema/Lookup.h"
> +
>  using namespace clang;
>
>  /// ParseNamespace - We know that the current token is a namespace
> keyword. This
> @@ -1230,38 +1232,39 @@
>        // a class (or template thereof).
>        TemplateArgList TemplateArgs;
>        SourceLocation LAngleLoc, RAngleLoc;
> -      if (ParseTemplateIdAfterTemplateName(TemplateTy(), NameLoc, SS,
> -                                           true, LAngleLoc,
> -                                           TemplateArgs, RAngleLoc)) {
> -        // We couldn't parse the template argument list at all, so don't
> -        // try to give any location information for the list.
> -        LAngleLoc = RAngleLoc = SourceLocation();
> -      }
> -
> -      Diag(NameLoc, diag::err_explicit_spec_non_template)
> -          << (TemplateInfo.Kind == ParsedTemplateInfo::
> ExplicitInstantiation)
> -          << TagTokKind << Name << SourceRange(LAngleLoc, RAngleLoc);
> +      LAngleLoc = ConsumeToken();
> +      if (SkipUntil(tok::greater, StopBeforeMatch | StopAtSemi))
> +        RAngleLoc = ConsumeToken();
> +      // If a name was not found, this might be an explicit instantiation
> or
> +      // specialization attempt. If a name was found, that name does not
> refer
> +      // to a template name.
> +      LookupResult Result(Actions, Name, NameLoc, Sema::LookupTagName);
> +      Actions.LookupName(Result, getCurScope());
>

This is using too low-level an interface from the Parser into Sema. Also,
I'm not convinced this'll do the right thing if it finds an
injected-class-name for a class template.

Can you use Sema::isTemplateName here?


> +      if (Result.getResultKind() != LookupResult::Found)
> +        Diag(NameLoc, diag::err_explicit_spec_non_template)
>

This is the wrong diagnostic in many cases (including the "new X < blah"
case). This is fine if you have TemplateInfo telling you that we had
template-parameter-lists, but otherwise, you should stop parsing before the
'<'. In particular:

  bool k = new struct IDontExist < 0;

... should only diagnose allocation of an incomplete type, and

  struct A {
    template<typename T> operator T();
  };
  bool b = sizeof &A::operator struct Foo < 8;

... should be accepted without any diagnostics.

+            << (TemplateInfo.Kind == ParsedTemplateInfo::
> ExplicitInstantiation)
> +            << TagTokKind << Name << SourceRange(LAngleLoc, RAngleLoc);
>
>        // Strip off the last template parameter list if it was empty, since
>        // we've removed its template argument list.
>        if (TemplateParams && TemplateInfo.LastParameterListWasEmpty) {
>          if (TemplateParams && TemplateParams->size() > 1) {
>            TemplateParams->pop_back();
>          } else {
>            TemplateParams = 0;
> -          const_cast<ParsedTemplateInfo&>(TemplateInfo).Kind
> -            = ParsedTemplateInfo::NonTemplate;
> +          const_cast<ParsedTemplateInfo &>(TemplateInfo).Kind =
> +              ParsedTemplateInfo::NonTemplate;
>          }
> -      } else if (TemplateInfo.Kind
> -                                == ParsedTemplateInfo::ExplicitInstantiation)
> {
> +      } else if (TemplateInfo.Kind ==
> +                 ParsedTemplateInfo::ExplicitInstantiation) {
>          // Pretend this is just a forward declaration.
>          TemplateParams = 0;
> -        const_cast<ParsedTemplateInfo&>(TemplateInfo).Kind
> -          = ParsedTemplateInfo::NonTemplate;
> -        const_cast<ParsedTemplateInfo&>(TemplateInfo).TemplateLoc
> -          = SourceLocation();
> -        const_cast<ParsedTemplateInfo&>(TemplateInfo).ExternLoc
> -          = SourceLocation();
> +        const_cast<ParsedTemplateInfo &>(TemplateInfo).Kind =
> +            ParsedTemplateInfo::NonTemplate;
> +        const_cast<ParsedTemplateInfo &>(TemplateInfo).TemplateLoc =
> +            SourceLocation();
> +        const_cast<ParsedTemplateInfo &>(TemplateInfo).ExternLoc =
> +            SourceLocation();
>        }
>      }
>    } else if (Tok.is(tok::annot_template_id)) {
> Index: test/Parser/cxx-template-argument.cpp
> ===================================================================
> --- test/Parser/cxx-template-argument.cpp
> +++ test/Parser/cxx-template-argument.cpp
> @@ -106,3 +106,21 @@
>    { };
>
>  }
> +
> +namespace PR18127 {
> +    struct A {} *a;
> +    bool f = new struct A < 0;
> +    bool g = new struct A < a;
> +
> +    template<typename T>
> +    struct B { };
> +
> +    bool h = new struct B < 0;  // expected-error{{expected '>'}}
> +    bool i = new struct B<int> < 0;
> +    bool operator<(const struct O &, const struct O &);
> +    struct O { } *p;
> +    bool j = *(new struct O) < *p;
> +    bool k = new struct IDontExist < 0;  // expected-error{{explicit
> specialization of non-template struct 'IDontExist'}}
> +    // expected-error at -1{{allocation of incomplete type 'struct
> IDontExist'}}
> +    // expected-note at -2{{forward declaration of 'PR18127::IDontExist'}}
> +}
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140130/1042884f/attachment.html>