rsmith added a comment. Sorry for the long delay. This looks to be in good shape.
================ Comment at: include/clang/AST/Type.h:1210 @@ +1209,3 @@ +/// Which keyword(s) were used to create an AutoType. +enum class AutoTypeKeyword : unsigned char { + /// \brief auto ---------------- The explicit underlying type here doesn't seem necessary. ================ Comment at: include/clang/AST/Type.h:1438 @@ -1429,1 +1437,3 @@ + /// Was this placeholder type spelled as 'auto', 'decltype(auto)', or '__auto_type'? + AutoTypeKeyword Keyword : 2; }; ---------------- Use `unsigned` not `AutoTypeKeyword`. MSVC struct layout inserts extra padding if your bit-fields don't all have the same type. ================ Comment at: include/clang/Basic/DiagnosticSemaKinds.td:1678 @@ -1674,3 +1677,3 @@ "|in template argument|in typedef|in type alias|in function return type" - "|in conversion function type|here|in lambda parameter}1">; + "|in conversion function type|here|in lambda parameter|in 'new' argument}1">; def err_auto_not_allowed_var_inst : Error< ---------------- How about "in type allocated by `new`" or similar? The type operand isn't really an argument. ================ Comment at: lib/Sema/SemaTemplateDeduction.cpp:4007-4012 @@ -4006,4 +4006,8 @@ InitListExpr *InitList = dyn_cast<InitListExpr>(Init); if (InitList) { + if (!getLangOpts().CPlusPlus) { + Diag(Loc, diag::err_auto_init_list_from_c); + return DAR_Failed; + } for (unsigned i = 0, e = InitList->getNumInits(); i < e; ++i) { ---------------- Move this bailout to before we build the template parameter and substitute it into the type. ================ Comment at: lib/Sema/SemaTemplateDeduction.cpp:4011 @@ +4010,3 @@ + Diag(Loc, diag::err_auto_init_list_from_c); + return DAR_Failed; + } ---------------- Should be `DAR_FailedAlreadyDiagnosed` so the caller doesn't diagnose it again. ================ Comment at: lib/Sema/SemaType.cpp:1481 @@ -1479,3 +1480,3 @@ // being analyzed (which tracks the invented type template parameter). if (declarator.getContext() == Declarator::LambdaExprParameterContext) { sema::LambdaScopeInfo *LSI = S.getCurLambda(); ---------------- Can you move the `auto_type` case label lower to make this more obvious? (Put a `break` into the `if` and replace the `else` with fallthrough to avoid putting the label inside the `else`.) ================ Comment at: lib/Sema/SemaType.cpp:1517 @@ +1516,3 @@ + Result = Context.getAutoType(QualType(), + /*Keyword*/ AutoTypeKeyword::DecltypeAuto, + /*IsDependent*/ false); ---------------- The `/*Keyword*/` comment here doesn't add anything, thanks to your enumeration. Remove it and clang-format this line? ================ Comment at: lib/Sema/SemaType.cpp:2621 @@ -2620,3 +2620,3 @@ // C++14 In generic lambdas allow 'auto' in their parameters. - if (ContainsPlaceholderType && + if (D.getDeclSpec().containsPlaceholderType() && (!SemaRef.getLangOpts().CPlusPlus11 || !D.isFunctionDeclarator())) { ---------------- Does this still do the right thing for the `IK_ConversionFunctionId` case? It doesn't look like it will: in that case, the `auto` is in the //declarator-id//, not in the //decl-specifier-seq//. ================ Comment at: lib/Sema/SemaType.cpp:2626-2627 @@ -2625,4 +2625,4 @@ switch (D.getContext()) { case Declarator::KNRTypeListContext: llvm_unreachable("K&R type lists aren't allowed in C++"); case Declarator::LambdaExprContext: ---------------- This is now reachable. ================ Comment at: lib/Sema/SemaType.cpp:3756-3760 @@ -3743,2 +3755,7 @@ } + } else if (D.getDeclSpec().getTypeSpecType() == DeclSpec::TST_auto_type) { + S.Diag(D.getDeclSpec().getTypeSpecTypeLoc(), + diag::err_auto_not_allowed) + << 2 << 13 << D.getDeclSpec().getSourceRange(); + D.setInvalidType(true); } ---------------- Please deal with this in the "checking for auto" block at the top rather than repeating the diagnostic code here with magic numbers; maybe change (!SemaRef.getLangOpts().CPlusPlus11 || !D.isFunctionDeclarator())) { to (!SemaRef.getLangOpts().CPlusPlus11 || !D.isFunctionDeclarator() || D.getDeclSpec().getTypeSpecType() == DeclSpec::TST_auto_type)) { up there. http://reviews.llvm.org/D12686 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits