[cfe-commits] [PATCH] Implement C++0x deduced auto type

Douglas Gregor dgregor at apple.com
Fri Feb 18 09:52:20 PST 2011


On Feb 17, 2011, at 6:26 PM, Richard Smith wrote:

> Hi,
> 
> The attached patch implements the C++0x deduced 'auto' type feature. This
> should fix the following PRs:
> 
> Bug 8738 - C++'0x auto not implemented
> Bug 9060 - Late-specified return type accepted for parenthesized declarator
> Bug 9132 - clang accepts declarators other than a literal 'auto' as the
> return type of a function with a trailing-return-type
> 
> (The latter two were fixed in order to allow more comprehensive testing of
> deduced auto!)
> 
> The patch introduces a new type, AutoType, which carries a deduced type.
> AutoType is usually just sugar around the deduced type. However, if the
> type hasn't been deduced yet (either because the initializer has not been
> attached or because the auto is within a template and the initializer was
> type-dependent), the AutoType is canonical and is considered dependent.

Very cool!

[snip]
> 2) auto a = 0, b = { 1, 2, 3 }, (*f)() -> double;
> 
> To my reading of the standard, this is legal (it is the /deduced/ types
> which must match: in the initializer-list case, that's the type it's a
> list of). Since clang doesn't support initializer-lists yet, that part
> will need handling later. AutoType doesn't store enough information to
> diagnose that issue.

Interesting. I completely agree with your reading of the standard here, although I admit that my initial reaction was, "they can't possibly have meant that!?"

Your patch is *excellent*. This is precisely how I was hoping that 'auto' would fit into Clang, and the implementation is very clean (and surprisingly concise). I have a bunch of comments below, most of which are for relatively small issues or questions. In all cases, I'm perfectly happy to have this patch go in as-is (or with the tiny tweaks I've requested), and save some of the non-trivial things for follow-up commits. 

Specific comments on your patch:

Index: test/CXX/dcl.dcl/dcl.spec/dcl.type/dcl.spec.auto/p6.cpp
===================================================================
--- test/CXX/dcl.dcl/dcl.spec/dcl.type/dcl.spec.auto/p6.cpp	(revision 0)
+++ test/CXX/dcl.dcl/dcl.spec/dcl.type/dcl.spec.auto/p6.cpp	(revision 0)
@@ -0,0 +1,60 @@
[snip]
+  const auto **fail2(p); // desired-error {{variable 'fail2' with type 'auto const **' has incompatible initializer of type 'int **'}} expected-error {{cannot initialize}}
+}

The "desired-error" is cute, but please put a FIXME to indicate that the diagnostic here isn't any good. This is due to PR9233, right?

Index: include/clang/AST/Decl.h
===================================================================
--- include/clang/AST/Decl.h	(revision 125795)
+++ include/clang/AST/Decl.h	(working copy)
@@ -646,6 +646,10 @@
   /// \brief Whether this local variable could be allocated in the return
   /// slot of its function, enabling the named return value optimization (NRVO).
   bool NRVOVariable : 1;
+
+  /// \brief Whether this variable has a deduced C++0x auto type for which we're
+  /// currently parsing the initializer.
+  bool ParsingAutoInit : 1;

It's unfortunate that this temporary bit has to live forever in the AST. Did you consider an alternative, such as a SmallPtrSet inside Sema that tracks the (few, relatively rare) VarDecls that are waiting for their initializers? (I guess in the future, FieldDecls must also be considered).

I don't think this is a big deal either way.

Index: include/clang/AST/Type.h
===================================================================
--- include/clang/AST/Type.h	(revision 125795)
+++ include/clang/AST/Type.h	(working copy)
@@ -3014,6 +3015,45 @@
   static bool classof(const SubstTemplateTypeParmPackType *T) { return true; }
 };
 
+/// \brief Represents a C++0x auto type.
+///
+/// These types are usually a placeholder for a deduced type. However, within
+/// templates and before the initializer is attached, there is no deduced type
+/// and an auto type is type-dependent and canonical.
+class AutoType : public Type, public llvm::FoldingSetNode {
+  AutoType(QualType DeducedType)
+    : Type(Auto, DeducedType.isNull() ? QualType(this, 0) : DeducedType,
+           DeducedType.isNull() || DeducedType->isDependentType(),
+           /*VariablyModified=*/false, /*ContainsParameterPack=*/false) { }

The deduced type can never be dependent, because we shouldn't be trying to deduce anything from a type-dependent initializer. So, I think the "|| DeducedType->isDependentType()" be be removed and, instead, add an assert for DeducedType.isNull() || !DeducedType->isDependentType().

Index: lib/Sema/SemaDecl.cpp
===================================================================
--- lib/Sema/SemaDecl.cpp	(revision 125795)
+++ lib/Sema/SemaDecl.cpp	(working copy)
@@ -4500,6 +4525,24 @@
     return;
   }
 
+  if (TypeMayContainAuto && VDecl->getType()->getContainedAutoType()) {
+    VDecl->setParsingAutoInit(false);
+
+    QualType DeducedType;
+    if (!DeduceAutoType(VDecl->getType(), Init, DeducedType)) {
+      Diag(VDecl->getLocation(), diag::err_auto_var_deduction_failure)
+        << VDecl->getDeclName() << VDecl->getType() << Init->getType()
+        << Init->getSourceRange();
+      RealDecl->setInvalidDecl();
+      return;
+    }
+    VDecl->setType(DeducedType);
+
+    // If this is a redeclaration, check that the type we just deduced matches
+    // the previously declared type.
+    if (VarDecl *Old = VDecl->getPreviousDeclaration())
+      MergeVarDeclTypes(VDecl, Old);
+  }

It would be useful to cite the appropriate C++0x standard clause here.

Index: lib/Sema/SemaTemplateDeduction.cpp
===================================================================
--- lib/Sema/SemaTemplateDeduction.cpp	(revision 125795)
+++ lib/Sema/SemaTemplateDeduction.cpp	(working copy)
@@ -22,6 +22,7 @@
 #include "clang/AST/Expr.h"
 #include "clang/AST/ExprCXX.h"
 #include "llvm/ADT/BitVector.h"
+#include "TreeTransform.h"
 #include <algorithm>
 
 namespace clang {
@@ -2445,21 +2446,22 @@
     ParamType = ParamType.getLocalUnqualifiedType();
   const ReferenceType *ParamRefType = ParamType->getAs<ReferenceType>();
   if (ParamRefType) {
+    QualType PointeeType = ParamRefType->getPointeeType();
+
     //   [C++0x] If P is an rvalue reference to a cv-unqualified
     //   template parameter and the argument is an lvalue, the type
     //   "lvalue reference to A" is used in place of A for type
     //   deduction.
-    if (const RValueReferenceType *RValueRef
-                                   = dyn_cast<RValueReferenceType>(ParamType)) {
-      if (!RValueRef->getPointeeType().getQualifiers() &&
-          isa<TemplateTypeParmType>(RValueRef->getPointeeType()) &&
+    if (isa<RValueReferenceType>(ParamType)) {
+      if (!PointeeType.getQualifiers() &&
+          isa<TemplateTypeParmType>(PointeeType) &&
           Arg->Classify(S.Context).isLValue())
         ArgType = S.Context.getLValueReferenceType(ArgType);
     }
 
     //   [...] If P is a reference type, the type referred to by P is used
     //   for type deduction.
-    ParamType = ParamRefType->getPointeeType();
+    ParamType = PointeeType;
   }
 
   // Overload sets usually make this parameter an undeduced

This is unrelated to auto, right? It also seems slightly dangerous, since we're really looking for the specific written form "T&&" (hence the dyn_cast on ParamType), rather than something that might be a typedef of "T&&".

@@ -2946,6 +2948,88 @@
                                  QualType(), Specialization, Info);
 }
 
+namespace {
+  class SubstituteAutoTransform :
+    public TreeTransform<SubstituteAutoTransform> {
+    QualType Replacement;
+  public:
+    SubstituteAutoTransform(Sema &SemaRef, QualType Replacement) :
+      TreeTransform<SubstituteAutoTransform>(SemaRef), Replacement(Replacement) {
+    }

Please add a comment for the SubstituteAutoTransform class.

+    QualType TransformAutoType(TypeLocBuilder &TLB, AutoTypeLoc TL) {
+      // Some of the template deduction code expects to see unadorned
+      // TemplateTypeParmTypes, so don't wrap them in AutoType.
+      if (isa<TemplateTypeParmType>(Replacement)) {
+        QualType Result = Replacement;
+        TemplateTypeParmTypeLoc NewTL = TLB.push<TemplateTypeParmTypeLoc>(Result);
+        NewTL.setNameLoc(TL.getNameLoc());
+        return Result;
+      } else {
+        QualType Result = RebuildAutoType(Replacement);
+        AutoTypeLoc NewTL = TLB.push<AutoTypeLoc>(Result);
+        NewTL.setNameLoc(TL.getNameLoc());
+        return Result;
+      }
+    }
+  };
+}

Is the comment on the  "if" branch wrong? It seems that we wouldn't be performing template argument deduction using something that was originally an 'auto' type as the "P" in the deduction. However, it does seem that this "if" branch is critical for Sema::DeduceAutoType to work properly.

+/// \brief Deduce the type for an auto type-specifier (C++0x [dcl.spec.auto]p6)
+///
+/// \param Type the type pattern using the auto type-specifier.
+///
+/// \param Init the initializer for the variable whose type is to be deduced.
+///
+/// \param Result if type deduction was successful, this will be set to the
+/// deduced type. This may still contain undeduced autos if the type is
+/// dependent.
+///
+/// \returns true if deduction succeeded, false if it failed.
+bool
+Sema::DeduceAutoType(QualType Type, Expr *Init, QualType &Result) {
+  if (Init->isTypeDependent()) {
+    Result = Type;
+    return true;
+  }
+
+  SourceLocation Loc = Init->getExprLoc();
+
+  LocalInstantiationScope InstScope(*this);
+
+  // Build template<class TemplParam> void Func(FuncParam);
+  NamedDecl *TemplParam
+    = TemplateTypeParmDecl::Create(Context, 0, Loc, 0, 0, 0, false, false);
+  TemplateParameterList *TemplateParams
+    = TemplateParameterList::Create(Context, Loc, Loc, &TemplParam, 1, Loc);
+
+  QualType TemplArg = Context.getTemplateTypeParmType(0, 0, false);
+  QualType FuncParam =
+    SubstituteAutoTransform(*this, TemplArg).TransformType(Type);
+
+  // Deduce type of TemplParam in Func(Init)
+  llvm::SmallVector<DeducedTemplateArgument, 1> Deduced;
+  Deduced.resize(1);
+  QualType InitType = Init->getType();
+  unsigned TDF = 0;
+  if (AdjustFunctionParmAndArgTypesForDeduction(*this, TemplateParams,
+                                                FuncParam, InitType, Init,
+                                                TDF))
+    return false;
+
+  TemplateDeductionInfo Info(Context, Loc);
+  if (::DeduceTemplateArguments(*this, TemplateParams,
+                                FuncParam, InitType, Info, Deduced,
+                                TDF))
+    return false;
+
+  QualType DeducedType = Deduced[0].getAsType();
+  if (DeducedType.isNull())
+    return false;
+
+  Result = SubstituteAutoTransform(*this, DeducedType).TransformType(Type);
+  return true;
+}
+

1-1 mapping between standard and implementation, I like it! I do wonder if we could reduce the memory footprint a bit by, e.g., allocating the template parameter list and template type parameter on the stack rather than in the never-to-be-freed ASTContext. This could be a follow-up optimization.

Index: lib/Sema/SemaTemplate.cpp
===================================================================
--- lib/Sema/SemaTemplate.cpp	(revision 125795)
+++ lib/Sema/SemaTemplate.cpp	(working copy)
@@ -2778,6 +2778,10 @@
   return false;
 }
 
+bool UnnamedLocalNoLinkageFinder::VisitAutoType(const AutoType*) {
+  return false;
+}
+

Shouldn't this visit the deduced type, if there is one?

Index: lib/Sema/SemaType.cpp
===================================================================
--- lib/Sema/SemaType.cpp	(revision 125795)
+++ lib/Sema/SemaType.cpp	(working copy)
@@ -1631,6 +1615,32 @@
         D.setInvalidType(true);
       }
 
+      // Check for auto functions and trailing return type and adjust the
+      // return type accordingly.
+      if (getLangOptions().CPlusPlus0x && !D.isInvalidType()) {
+        // trailing-return-type is only required if we're declaring a function,
+        // and not, for instance, a pointer to a function.
+        if (D.getDeclSpec().getTypeSpecType() == DeclSpec::TST_auto &&
+            !FTI.TrailingReturnType && chunkIndex == 0) {
+          Diag(D.getDeclSpec().getTypeSpecTypeLoc(),
+               diag::err_auto_missing_trailing_return);
+          T = Context.IntTy;
+          D.setInvalidType(true);
+        } else if (FTI.TrailingReturnType) {
+          if (T.hasQualifiers() || !isa<AutoType>(T)) {
+            // T must be exactly 'auto' at this point. See CWG issue 681.
+            Diag(D.getDeclSpec().getTypeSpecTypeLoc(),
+                 diag::err_trailing_return_without_auto)
+              << T << D.getDeclSpec().getSourceRange();
+            D.setInvalidType(true);
+          }
+
+          T = GetTypeFromParser(
+            ParsedType::getFromOpaquePtr(FTI.TrailingReturnType),
+            &ReturnTypeInfo);
+        }
+      }
+

Only a trivial comment here: I know the check was there before, there's no need to check the CPlusPlus0x flag, since we won't see TST_auto or FTI.TrailingReturnType unless the parser wanted it. By not checking for C++0x here, we have the option of teaching the parser to be smarter about dialect issues, e.g., if it figures out that "auto" is being used as a type-specifier in C++98/03 code, it could warn and then parse it as a type-specifier.

Index: lib/AST/ItaniumMangle.cpp
===================================================================
--- lib/AST/ItaniumMangle.cpp	(revision 125795)
+++ lib/AST/ItaniumMangle.cpp	(working copy)
@@ -1313,9 +1313,6 @@
     assert(false &&
            "Overloaded and dependent types shouldn't get to name mangling");
     break;
-  case BuiltinType::UndeducedAuto:
-    assert(0 && "Should not see undeduced auto here");
-    break;
   case BuiltinType::ObjCId: Out << "11objc_object"; break;
   case BuiltinType::ObjCClass: Out << "10objc_class"; break;
   case BuiltinType::ObjCSel: Out << "13objc_selector"; break;
@@ -1648,6 +1645,12 @@
   Out << 'E';
 }
 
+void CXXNameMangler::mangleType(const AutoType *T) {
+  QualType D = T->getDeducedType();
+  assert(!D.isNull() && "can't mangle undeduced auto type");
+  mangleType(D);
+}

Please make this an error (via the Diagnostics subsystem) rather than an assertion... I can think of evil ways to write code that would trip up here, which we should (but don't) diagnose as errors. I'd rather fail gracefully.

Index: lib/AST/ASTContext.cpp
===================================================================
--- lib/AST/ASTContext.cpp	(revision 125795)
+++ lib/AST/ASTContext.cpp	(working copy)
@@ -2674,6 +2677,14 @@
   return QualType(dt, 0);
 }
 
+/// getAutoType - Unlike many "get<Type>" functions, we don't unique
+/// AutoType AST's.
+QualType ASTContext::getAutoType(QualType DeducedType) const {
+  AutoType *at = new (*this, TypeAlignment) AutoType(DeducedType);
+  Types.push_back(at);
+  return QualType(at, 0);
+}

Why not unique 'auto' types when we have a deduced type?

 I can understand not uniquing when we have a dependent initializer (so the 'auto' has no deduced type for a "long" time), since we don't want 'x' and 'y' to look like they have the same type in:

  template<typename T, typename U>
  void f() {
    auto x = T();
    auto y = U();
  }

If we could unique in the common cases---where we have a deduced type, or where we have no deduced type and we know we're not anywhere inside a template---we could save ourselves some memory.

None of the tests seem to cover the pointer-to-member case, e.g.,

struct X {
  int &f(int);
};

void test(X &x) {
  auto X::*fp = &X::f;
  int &ir = (x.*fp)(17);
}

even though it seems to work perfectly well.

	- Doug
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20110218/aa5f86a0/attachment.html>


More information about the cfe-commits mailing list