Recover from missing 'typename' in sizeof(T::InnerType)

[rnk]

Summary

'sizeof' is a UnaryExprOrTypeTrait, and it can contain either a type or
an expression. This change threads a RecoveryTSI parameter through the
layers between TransformUnaryExprOrTypeTrait the point at which we look
up the type. If lookup finds a single type result after instantiation,
we now build TypeSourceInfo for it just like a normal transformation
would.

This fixes the last error in the hello world ATL app that I've been
working with, and it now links and runs with clang. Please try it and
file bugs!

[rsmith]

Looks good.

lib/Sema/TreeTransform.h

6871 ↗

(On Diff #10341)

It's sufficient to only test DRE here (you could also move the declaration of DRE into the if).

8282 ↗

(On Diff #10341)

My over-80-column sense is tingling...

[rnk]

Thanks!

lib/Sema/TreeTransform.h

6871 ↗

(On Diff #10341)

OK

8282 ↗

(On Diff #10341)

yeah

[rnk]

Closed by commit rL210855 (authored by @rnk).

cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td

def err_typename_nested_not_found_enable_if : Error<
   "no type named 'type' in %0; 'enable_if' cannot be used to disable "
   "this declaration">;
 def err_typename_nested_not_type : Error<
     "typename specifier refers to non-type member %0 in %1">;
 def note_typename_refers_here : Note<
     "referenced member %0 is declared here">;
 def err_typename_missing : Error<
   "missing 'typename' prior to dependent type name '%0%1'">;
-def warn_typename_missing : ExtWarn<
+def ext_typename_missing : ExtWarn<
   "missing 'typename' prior to dependent type name '%0%1'">,
   InGroup<DiagGroup<"typename-missing">>;
 def ext_typename_outside_of_template : ExtWarn<
   "'typename' occurs outside of a template">, InGroup<CXX11>;
 def warn_cxx98_compat_typename_outside_of_template : Warning<
   "use of 'typename' outside of a template is incompatible with C++98">,
   InGroup<CXX98Compat>, DefaultIgnore;
 def err_typename_refers_to_using_value_decl : Error<

cfe/trunk/include/clang/Sema/Sema.h

ExprResult BuildImplicitMemberExpr(const CXXScopeSpec &SS,
                                      SourceLocation TemplateKWLoc,
                                      LookupResult &R,
                                 const TemplateArgumentListInfo *TemplateArgs,
                                      bool IsDefiniteInstance);
   bool UseArgumentDependentLookup(const CXXScopeSpec &SS,
                                   const LookupResult &R,
                                   bool HasTrailingLParen);
 
-  ExprResult BuildQualifiedDeclarationNameExpr(CXXScopeSpec &SS,
-                                         const DeclarationNameInfo &NameInfo,
-                                               bool IsAddressOfOperand);
+  ExprResult BuildQualifiedDeclarationNameExpr(
+      CXXScopeSpec &SS, const DeclarationNameInfo &NameInfo,
+      bool IsAddressOfOperand, TypeSourceInfo **RecoveryTSI = nullptr);
+
   ExprResult BuildDependentDeclRefExpr(const CXXScopeSpec &SS,
                                        SourceLocation TemplateKWLoc,
                                 const DeclarationNameInfo &NameInfo,
                                 const TemplateArgumentListInfo *TemplateArgs);
 
   ExprResult BuildDeclarationNameExpr(const CXXScopeSpec &SS,
                                       LookupResult &R,
                                       bool NeedsADL);

cfe/trunk/lib/Sema/SemaDecl.cpp

bool Sema::DiagnoseUnknownTypeName(IdentifierInfo *&II,
   if (!SS || (!SS->isSet() && !SS->isInvalid()))
     Diag(IILoc, diag::err_unknown_typename) << II;
   else if (DeclContext *DC = computeDeclContext(*SS, false))
     Diag(IILoc, diag::err_typename_nested_not_found) 
       << II << DC << SS->getRange();
   else if (isDependentScopeSpecifier(*SS)) {
     unsigned DiagID = diag::err_typename_missing;
     if (getLangOpts().MSVCCompat && isMicrosoftMissingTypename(SS, S))
-      DiagID = diag::warn_typename_missing;
+      DiagID = diag::ext_typename_missing;
 
     Diag(SS->getRange().getBegin(), DiagID)
       << SS->getScopeRep() << II->getName()
       << SourceRange(SS->getRange().getBegin(), IILoc)
       << FixItHint::CreateInsertion(SS->getRange().getBegin(), "typename ");
     SuggestedType = ActOnTypenameType(S, SourceLocation(),
                                       *SS, *II, IILoc).get();
   } else {

cfe/trunk/lib/Sema/SemaExpr.cpp

 
 /// BuildQualifiedDeclarationNameExpr - Build a C++ qualified
 /// declaration name, generally during template instantiation.
 /// There's a large number of things which don't need to be done along
 /// this path.
 ExprResult
 Sema::BuildQualifiedDeclarationNameExpr(CXXScopeSpec &SS,
                                         const DeclarationNameInfo &NameInfo,
-                                        bool IsAddressOfOperand) {
+                                        bool IsAddressOfOperand,
+                                        TypeSourceInfo **RecoveryTSI) {
   DeclContext *DC = computeDeclContext(SS, false);
   if (!DC)
     return BuildDependentDeclRefExpr(SS, /*TemplateKWLoc=*/SourceLocation(),
                                      NameInfo, /*TemplateArgs=*/nullptr);
 
   if (RequireCompleteDeclContext(SS, DC))
     return ExprError();
 
   LookupResult R(*this, NameInfo, LookupOrdinaryName);
   LookupQualifiedName(R, DC);
 
   if (R.isAmbiguous())
     return ExprError();
 
   if (R.getResultKind() == LookupResult::NotFoundInCurrentInstantiation)
     return BuildDependentDeclRefExpr(SS, /*TemplateKWLoc=*/SourceLocation(),
                                      NameInfo, /*TemplateArgs=*/nullptr);
 
   if (R.empty()) {
     Diag(NameInfo.getLoc(), diag::err_no_member)
       << NameInfo.getName() << DC << SS.getRange();
     return ExprError();
   }
 
-  if (R.isSingleResult() && R.getAsSingle<TypeDecl>()) {
-    // Diagnose a missing typename if this resolved unambiguously to a type in a
-    // dependent context.
-    // FIXME: Issue a fixit and recover as though the user had written
-    // 'typename'.
-    Diag(SS.getBeginLoc(), diag::err_typename_missing)
-        << SS.getScopeRep() << NameInfo.getName().getAsString()
-        << SourceRange(SS.getBeginLoc(), NameInfo.getEndLoc());
-    return ExprError();
+  if (const TypeDecl *TD = R.getAsSingle<TypeDecl>()) {
+    // Diagnose a missing typename if this resolved unambiguously to a type in
+    // a dependent context.  If we can recover with a type, downgrade this to
+    // a warning in Microsoft compatibility mode.
+    unsigned DiagID = diag::err_typename_missing;
+    if (RecoveryTSI && getLangOpts().MSVCCompat)
+      DiagID = diag::ext_typename_missing;
+    SourceLocation Loc = SS.getBeginLoc();
+    auto D = Diag(Loc, DiagID);
+    D << SS.getScopeRep() << NameInfo.getName().getAsString()
+      << SourceRange(Loc, NameInfo.getEndLoc());
+
+    // Don't recover if the caller isn't expecting us to or if we're in a SFINAE
+    // context.
+    if (!RecoveryTSI)
+      return ExprError();
+
+    // Only issue the fixit if we're prepared to recover.
+    D << FixItHint::CreateInsertion(Loc, "typename ");
+
+    // Recover by pretending this was an elaborated type.
+    QualType Ty = Context.getTypeDeclType(TD);
+    TypeLocBuilder TLB;
+    TLB.pushTypeSpec(Ty).setNameLoc(NameInfo.getLoc());
+
+    QualType ET = getElaboratedType(ETK_None, SS, Ty);
+    ElaboratedTypeLoc QTL = TLB.push<ElaboratedTypeLoc>(ET);
+    QTL.setElaboratedKeywordLoc(SourceLocation());
+    QTL.setQualifierLoc(SS.getWithLocInContext(Context));
+
+    *RecoveryTSI = TLB.getTypeSourceInfo(Context, ET);
+
+    return ExprEmpty();
   }
 
   // Defend against this resolving to an implicit member access. We usually
   // won't get here if this might be a legitimate a class member (we end up in
   // BuildMemberReferenceExpr instead), but this can be valid if we're forming
   // a pointer-to-member or in an unevaluated context in C++11.
   if (!R.empty() && (*R.begin())->isCXXClassMember() && !IsAddressOfOperand)
     return BuildPossibleImplicitMemberExpr(SS,

cfe/trunk/lib/Sema/SemaTemplate.cpp

if (R.empty()) {
     Diag(NameInfo.getLoc(), diag::err_template_kw_refers_to_non_template)
       << NameInfo.getName() << SS.getRange();
     return ExprError();
   }
 
   if (ClassTemplateDecl *Temp = R.getAsSingle<ClassTemplateDecl>()) {
     Diag(NameInfo.getLoc(), diag::err_template_kw_refers_to_class_template)
       << SS.getScopeRep()
-      << NameInfo.getName() << SS.getRange();
+      << NameInfo.getName().getAsString() << SS.getRange();
     Diag(Temp->getLocation(), diag::note_referenced_class_template);
     return ExprError();
   }
 
   return BuildTemplateIdExpr(SS, TemplateKWLoc, R, /*ADL*/ false, TemplateArgs);
 }
 
 /// \brief Form a dependent template name.

cfe/trunk/lib/Sema/TreeTransform.h

ExprResult TransformLambdaScope(LambdaExpr *E, CXXMethodDecl *CallOperator,
        ArrayRef<InitCaptureInfoTy> InitCaptureExprsAndTypes);
 
   TemplateParameterList *TransformTemplateParameterList(
         TemplateParameterList *TPL) {
     return TPL;
   }
 
   ExprResult TransformAddressOfOperand(Expr *E);
+
   ExprResult TransformDependentScopeDeclRefExpr(DependentScopeDeclRefExpr *E,
-                                                bool IsAddressOfOperand);
+                                                bool IsAddressOfOperand,
+                                                TypeSourceInfo **RecoveryTSI);
+
+  ExprResult TransformParenDependentScopeDeclRefExpr(
+      ParenExpr *PE, DependentScopeDeclRefExpr *DRE, bool IsAddressOfOperand,
+      TypeSourceInfo **RecoveryTSI);
+
   StmtResult TransformOMPExecutableDirective(OMPExecutableDirective *S);
 
 // FIXME: We use LLVM_ATTRIBUTE_NOINLINE because inlining causes a ridiculous
 // amount of stack usage with clang.
 #define STMT(Node, Parent)                        \
   LLVM_ATTRIBUTE_NOINLINE \
   StmtResult Transform##Node(Node *S);
 #define EXPR(Node, Parent)                        \

   ///
   /// By default, performs semantic analysis to build the new expression.
   /// Subclasses may override this routine to provide different behavior.
   ExprResult RebuildDependentScopeDeclRefExpr(
                                           NestedNameSpecifierLoc QualifierLoc,
                                           SourceLocation TemplateKWLoc,
                                        const DeclarationNameInfo &NameInfo,
                               const TemplateArgumentListInfo *TemplateArgs,
-                                          bool IsAddressOfOperand) {
+                                          bool IsAddressOfOperand,
+                                          TypeSourceInfo **RecoveryTSI) {
     CXXScopeSpec SS;
     SS.Adopt(QualifierLoc);
 
     if (TemplateArgs || TemplateKWLoc.isValid())
-      return getSema().BuildQualifiedTemplateIdExpr(SS, TemplateKWLoc,
-                                                    NameInfo, TemplateArgs);
+      return getSema().BuildQualifiedTemplateIdExpr(SS, TemplateKWLoc, NameInfo,
+                                                    TemplateArgs);
 
-    return getSema().BuildQualifiedDeclarationNameExpr(SS, NameInfo,
-                                                       IsAddressOfOperand);
+    return getSema().BuildQualifiedDeclarationNameExpr(
+        SS, NameInfo, IsAddressOfOperand, RecoveryTSI);
   }
 
   /// \brief Build a new template-id expression.
   ///
   /// By default, performs semantic analysis to build the new expression.
   /// Subclasses may override this routine to provide different behavior.
   ExprResult RebuildTemplateIdExpr(const CXXScopeSpec &SS,
                                    SourceLocation TemplateKWLoc,

 
 /// \brief The operand of a unary address-of operator has special rules: it's
 /// allowed to refer to a non-static member of a class even if there's no 'this'
 /// object available.
 template<typename Derived>
 ExprResult
 TreeTransform<Derived>::TransformAddressOfOperand(Expr *E) {
   if (DependentScopeDeclRefExpr *DRE = dyn_cast<DependentScopeDeclRefExpr>(E))
-    return getDerived().TransformDependentScopeDeclRefExpr(DRE, true);
+    return getDerived().TransformDependentScopeDeclRefExpr(DRE, true, nullptr);
   else
     return getDerived().TransformExpr(E);
 }
 
 template<typename Derived>
 ExprResult
 TreeTransform<Derived>::TransformUnaryOperator(UnaryOperator *E) {
   ExprResult SubExpr;
TreeTransform<Derived>::TransformUnaryExprOrTypeTraitExpr(
   }
 
   // C++0x [expr.sizeof]p1:
   //   The operand is either an expression, which is an unevaluated operand
   //   [...]
   EnterExpressionEvaluationContext Unevaluated(SemaRef, Sema::Unevaluated,
                                                Sema::ReuseLambdaContextDecl);
 
-  ExprResult SubExpr = getDerived().TransformExpr(E->getArgumentExpr());
-  if (SubExpr.isInvalid())
+  // Try to recover if we have something like sizeof(T::X) where X is a type.
+  // Notably, there must be *exactly* one set of parens if X is a type.
+  TypeSourceInfo *RecoveryTSI = nullptr;
+  ExprResult SubExpr;
+  auto *PE = dyn_cast<ParenExpr>(E->getArgumentExpr());
+  if (auto *DRE =
+          PE ? dyn_cast<DependentScopeDeclRefExpr>(PE->getSubExpr()) : nullptr)
+    SubExpr = getDerived().TransformParenDependentScopeDeclRefExpr(
+        PE, DRE, false, &RecoveryTSI);
+  else
+    SubExpr = getDerived().TransformExpr(E->getArgumentExpr());
+
+  if (RecoveryTSI) {
+    return getDerived().RebuildUnaryExprOrTypeTrait(
+        RecoveryTSI, E->getOperatorLoc(), E->getKind(), E->getSourceRange());
+  } else if (SubExpr.isInvalid())
     return ExprError();
 
   if (!getDerived().AlwaysRebuild() && SubExpr.get() == E->getArgumentExpr())
     return E;
 
   return getDerived().RebuildUnaryExprOrTypeTrait(SubExpr.get(),
                                                   E->getOperatorLoc(),
                                                   E->getKind(),
ExprResult SubExpr;
     if (!getDerived().AlwaysRebuild() && SubExpr.get() == E->getQueriedExpression())
       return E;
   }
 
   return getDerived().RebuildExpressionTrait(
       E->getTrait(), E->getLocStart(), SubExpr.get(), E->getLocEnd());
 }
 
-template<typename Derived>
-ExprResult
-TreeTransform<Derived>::TransformDependentScopeDeclRefExpr(
-                                               DependentScopeDeclRefExpr *E) {
-  return TransformDependentScopeDeclRefExpr(E, /*IsAddressOfOperand*/false);
+template <typename Derived>
+ExprResult TreeTransform<Derived>::TransformParenDependentScopeDeclRefExpr(
+    ParenExpr *PE, DependentScopeDeclRefExpr *DRE, bool AddrTaken,
+    TypeSourceInfo **RecoveryTSI) {
+  ExprResult NewDRE = getDerived().TransformDependentScopeDeclRefExpr(
+      DRE, AddrTaken, RecoveryTSI);
+
+  // Propagate both errors and recovered types, which return ExprEmpty.
+  if (!NewDRE.isUsable())
+    return NewDRE;
+
+  // We got an expr, wrap it up in parens.
+  if (!getDerived().AlwaysRebuild() && NewDRE.get() == DRE)
+    return PE;
+  return getDerived().RebuildParenExpr(NewDRE.get(), PE->getLParen(),
+                                       PE->getRParen());
+}
+
+template <typename Derived>
+ExprResult TreeTransform<Derived>::TransformDependentScopeDeclRefExpr(
+  return TransformDependentScopeDeclRefExpr(E, /*IsAddressOfOperand=*/false,
+                                            nullptr);
 }
 
 template<typename Derived>
 ExprResult
 TreeTransform<Derived>::TransformDependentScopeDeclRefExpr(
                                                DependentScopeDeclRefExpr *E,
-                                               bool IsAddressOfOperand) {
+                                               bool IsAddressOfOperand,
+                                               TypeSourceInfo **RecoveryTSI) {
   assert(E->getQualifierLoc());
   NestedNameSpecifierLoc QualifierLoc
   = getDerived().TransformNestedNameSpecifierLoc(E->getQualifierLoc());
   if (!QualifierLoc)
     return ExprError();
   SourceLocation TemplateKWLoc = E->getTemplateKeywordLoc();
 
   // TODO: If this is a conversion-function-id, verify that the
   // destination type name (if present) resolves the same way after
   // instantiation as it did in the local scope.
 
   DeclarationNameInfo NameInfo
     = getDerived().TransformDeclarationNameInfo(E->getNameInfo());
   if (!NameInfo.getName())
     return ExprError();
 
   if (!E->hasExplicitTemplateArgs()) {
     if (!getDerived().AlwaysRebuild() &&
         QualifierLoc == E->getQualifierLoc() &&
         // Note: it is sufficient to compare the Name component of NameInfo:
         // if name has not changed, DNLoc has not changed either.
         NameInfo.getName() == E->getDeclName())
       return E;
 
-    return getDerived().RebuildDependentScopeDeclRefExpr(QualifierLoc,
-                                                        TemplateKWLoc,
-                                                        NameInfo,
-                                                        /*TemplateArgs*/nullptr,
-                                                        IsAddressOfOperand);
+    return getDerived().RebuildDependentScopeDeclRefExpr(
+        QualifierLoc, TemplateKWLoc, NameInfo, /*TemplateArgs=*/nullptr,
+        IsAddressOfOperand, RecoveryTSI);
   }
 
   TemplateArgumentListInfo TransArgs(E->getLAngleLoc(), E->getRAngleLoc());
   if (getDerived().TransformTemplateArguments(E->getTemplateArgs(),
                                               E->getNumTemplateArgs(),
                                               TransArgs))
     return ExprError();
 
-  return getDerived().RebuildDependentScopeDeclRefExpr(QualifierLoc,
-                                                       TemplateKWLoc,
-                                                       NameInfo,
-                                                       &TransArgs,
-                                                       IsAddressOfOperand);
+  return getDerived().RebuildDependentScopeDeclRefExpr(
+      QualifierLoc, TemplateKWLoc, NameInfo, &TransArgs, IsAddressOfOperand,
+      RecoveryTSI);
 }
 
 template<typename Derived>
 ExprResult
 TreeTransform<Derived>::TransformCXXConstructExpr(CXXConstructExpr *E) {
   // CXXConstructExprs other than for list-initialization and
   // CXXTemporaryObjectExpr are always implicit, so when we have
   // a 1-argument construction we just transform that argument.

cfe/trunk/test/SemaTemplate/ms-sizeof-missing-typename.cpp

+// RUN: %clang_cc1 -std=c++11 -fms-compatibility -fsyntax-only -verify %s
+
+// If we were even more clever, we'd tell the user to use one set of parens to
+// get the size of this type, so they don't get errors after inserting typename.
+
+namespace basic {
+template <typename T> int type_f() { return sizeof T::type; }  // expected-error {{missing 'typename' prior to dependent type name 'X::type'}}
+template <typename T> int type_g() { return sizeof(T::type); }  // expected-warning {{missing 'typename' prior to dependent type name 'X::type'}}
+template <typename T> int type_h() { return sizeof((T::type)); }  // expected-error {{missing 'typename' prior to dependent type name 'X::type'}}
+template <typename T> int value_f() { return sizeof T::not_a_type; }
+template <typename T> int value_g() { return sizeof(T::not_a_type); }
+template <typename T> int value_h() { return sizeof((T::not_a_type)); }
+struct X {
+  typedef int type;
+  static const int not_a_type;
+};
+int bar() {
+  return
+      type_f<X>() + // expected-note-re {{in instantiation {{.*}} requested here}}
+      type_g<X>() + // expected-note-re {{in instantiation {{.*}} requested here}}
+      type_h<X>() + // expected-note-re {{in instantiation {{.*}} requested here}}
+      value_f<X>() +
+      value_f<X>() +
+      value_f<X>();
+}
+}
+
+namespace nested_sizeof {
+template <typename T>
+struct Foo {
+  enum {
+    // expected-warning@+2 {{use 'template' keyword to treat 'InnerTemplate' as a dependent template name}}
+    // expected-warning@+1 {{missing 'typename' prior to dependent type name 'Bar::InnerType'}}
+    x1 = sizeof(typename T::/*template*/ InnerTemplate<sizeof(/*typename*/ T::InnerType)>),
+    // expected-warning@+1 {{missing 'typename' prior to dependent type name 'Bar::InnerType'}}
+    x2 = sizeof(typename T::template InnerTemplate<sizeof(/*typename*/ T::InnerType)>),
+    // expected-warning@+1 {{use 'template' keyword to treat 'InnerTemplate' as a dependent template name}}
+    y1 = sizeof(typename T::/*template*/ InnerTemplate<sizeof(T::InnerVar)>),
+    y2 = sizeof(typename T::template InnerTemplate<sizeof(T::InnerVar)>),
+    z = sizeof(T::template InnerTemplate<sizeof(T::InnerVar)>::x),
+  };
+};
+struct Bar {
+  template <int N>
+  struct InnerTemplate { int x[N]; };
+  typedef double InnerType;
+  static const int InnerVar = 42;
+};
+template struct Foo<Bar>; // expected-note-re {{in instantiation {{.*}} requested here}}
+}
+
+namespace ambiguous_missing_parens {
+// expected-error@+1 {{'Q::U' instantiated to a class template, not a function template}}
+template <typename T> void f() { int a = sizeof T::template U<0> + 4; }
+struct Q {
+  // expected-error@+1 {{class template declared here}}
+  template <int> struct U {};
+};
+// expected-note-re@+1 {{in instantiation {{.*}} requested here}}
+template void f<Q>();
+}