Author: george.karpenkov Date: Wed Mar 28 17:56:24 2018 New Revision: 328749
URL: http://llvm.org/viewvc/llvm-project?rev=328749&view=rev Log: [ast] Do not auto-initialize Objective-C for-loop variables in Objective-C++ in templatized code under ARC The AST for the fragment ``` @interface I @end template <typename> void decode(I *p) { for (I *k in p) {} } void decode(I *p) { decode<int>(p); } ``` differs heavily when templatized and non-templatized: ``` |-FunctionTemplateDecl 0x7fbfe0863940 <line:4:1, line:7:1> line:5:6 decode | |-TemplateTypeParmDecl 0x7fbfe0863690 <line:4:11> col:11 typename depth 0 index 0 | |-FunctionDecl 0x7fbfe08638a0 <line:5:1, line:7:1> line:5:6 decode 'void (I *__strong)' | | |-ParmVarDecl 0x7fbfe08637a0 <col:13, col:16> col:16 referenced p 'I *__strong' | | `-CompoundStmt 0x7fbfe0863b88 <col:19, line:7:1> | | `-ObjCForCollectionStmt 0x7fbfe0863b50 <line:6:3, col:20> | | |-DeclStmt 0x7fbfe0863a50 <col:8, col:13> | | | `-VarDecl 0x7fbfe08639f0 <col:8, col:11> col:11 k 'I *const __strong' | | |-ImplicitCastExpr 0x7fbfe0863a90 <col:16> 'I *' <LValueToRValue> | | | `-DeclRefExpr 0x7fbfe0863a68 <col:16> 'I *__strong' lvalue ParmVar 0x7fbfe08637a0 'p' 'I *__strong' | | `-CompoundStmt 0x7fbfe0863b78 <col:19, col:20> | `-FunctionDecl 0x7fbfe0863f80 <line:5:1, line:7:1> line:5:6 used decode 'void (I *__strong)' | |-TemplateArgument type 'int' | |-ParmVarDecl 0x7fbfe0863ef8 <col:13, col:16> col:16 used p 'I *__strong' | `-CompoundStmt 0x7fbfe0890cf0 <col:19, line:7:1> | `-ObjCForCollectionStmt 0x7fbfe0890cc8 <line:6:3, col:20> | |-DeclStmt 0x7fbfe0890c70 <col:8, col:13> | | `-VarDecl 0x7fbfe0890c00 <col:8, col:11> col:11 k 'I *__strong' callinit | | `-ImplicitValueInitExpr 0x7fbfe0890c60 <<invalid sloc>> 'I *__strong' | |-ImplicitCastExpr 0x7fbfe0890cb0 <col:16> 'I *' <LValueToRValue> | | `-DeclRefExpr 0x7fbfe0890c88 <col:16> 'I *__strong' lvalue ParmVar 0x7fbfe0863ef8 'p' 'I *__strong' | `-CompoundStmt 0x7fbfe0863b78 <col:19, col:20> ``` Note how in the instantiated version ImplicitValueInitExpr unexpectedly appears. While objects are auto-initialized under ARC, it does not make sense to have an initializer for a for-loop variable, and it makes even less sense to have such a different AST for instantiated and non-instantiated version. Digging deeper, I have found that there are two separate Sema* files for dealing with templates and for dealing with non-templatized code. In a non-templatized version, an initialization was performed only for variables which are not loop variables for an Objective-C loop and not variables for a C++ for-in loop: ``` if (FRI && (Tok.is(tok::colon) || isTokIdentifier_in())) { bool IsForRangeLoop = false; if (TryConsumeToken(tok::colon, FRI->ColonLoc)) { IsForRangeLoop = true; if (Tok.is(tok::l_brace)) FRI->RangeExpr = ParseBraceInitializer(); else FRI->RangeExpr = ParseExpression(); } Decl *ThisDecl = Actions.ActOnDeclarator(getCurScope(), D); if (IsForRangeLoop) Actions.ActOnCXXForRangeDecl(ThisDecl); Actions.FinalizeDeclaration(ThisDecl); D.complete(ThisDecl); return Actions.FinalizeDeclaratorGroup(getCurScope(), DS, ThisDecl); } SmallVector<Decl *, 8> DeclsInGroup; Decl *FirstDecl = ParseDeclarationAfterDeclaratorAndAttributes( D, ParsedTemplateInfo(), FRI); ``` However the code in SemaTemplateInstantiateDecl was inconsistent, guarding only against C++ for-in loops. rdar://38391075 Differential Revision: https://reviews.llvm.org/D44989 Added: cfe/trunk/test/SemaObjC/foreachtemplatized.mm Modified: cfe/trunk/include/clang/AST/Decl.h cfe/trunk/lib/Parse/ParseDecl.cpp cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp cfe/trunk/lib/Serialization/ASTReaderDecl.cpp cfe/trunk/lib/Serialization/ASTWriterDecl.cpp Modified: cfe/trunk/include/clang/AST/Decl.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/Decl.h?rev=328749&r1=328748&r2=328749&view=diff ============================================================================== --- cfe/trunk/include/clang/AST/Decl.h (original) +++ cfe/trunk/include/clang/AST/Decl.h Wed Mar 28 17:56:24 2018 @@ -937,6 +937,9 @@ protected: /// for-range statement. unsigned CXXForRangeDecl : 1; + /// Whether this variable is the for-in loop declaration in Objective-C. + unsigned ObjCForDecl : 1; + /// Whether this variable is an ARC pseudo-__strong /// variable; see isARCPseudoStrong() for details. unsigned ARCPseudoStrong : 1; @@ -1334,6 +1337,16 @@ public: NonParmVarDeclBits.CXXForRangeDecl = FRD; } + /// \brief Determine whether this variable is a for-loop declaration for a + /// for-in statement in Objective-C. + bool isObjCForDecl() const { + return NonParmVarDeclBits.ObjCForDecl; + } + + void setObjCForDecl(bool FRD) { + NonParmVarDeclBits.ObjCForDecl = FRD; + } + /// \brief Determine whether this variable is an ARC pseudo-__strong /// variable. A pseudo-__strong variable has a __strong-qualified /// type but does not actually retain the object written into it. Modified: cfe/trunk/lib/Parse/ParseDecl.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/ParseDecl.cpp?rev=328749&r1=328748&r2=328749&view=diff ============================================================================== --- cfe/trunk/lib/Parse/ParseDecl.cpp (original) +++ cfe/trunk/lib/Parse/ParseDecl.cpp Wed Mar 28 17:56:24 2018 @@ -2027,8 +2027,13 @@ Parser::DeclGroupPtrTy Parser::ParseDecl } Decl *ThisDecl = Actions.ActOnDeclarator(getCurScope(), D); - if (IsForRangeLoop) + if (IsForRangeLoop) { Actions.ActOnCXXForRangeDecl(ThisDecl); + } else { + // Obj-C for loop + if (auto *VD = dyn_cast_or_null<VarDecl>(ThisDecl)) + VD->setObjCForDecl(true); + } Actions.FinalizeDeclaration(ThisDecl); D.complete(ThisDecl); return Actions.FinalizeDeclaratorGroup(getCurScope(), DS, ThisDecl); Modified: cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp?rev=328749&r1=328748&r2=328749&view=diff ============================================================================== --- cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp (original) +++ cfe/trunk/lib/Sema/SemaTemplateInstantiateDecl.cpp Wed Mar 28 17:56:24 2018 @@ -4083,6 +4083,7 @@ void Sema::BuildVariableInstantiation( NewVar->setTSCSpec(OldVar->getTSCSpec()); NewVar->setInitStyle(OldVar->getInitStyle()); NewVar->setCXXForRangeDecl(OldVar->isCXXForRangeDecl()); + NewVar->setObjCForDecl(OldVar->isObjCForDecl()); NewVar->setConstexpr(OldVar->isConstexpr()); NewVar->setInitCapture(OldVar->isInitCapture()); NewVar->setPreviousDeclInSameBlockScope( @@ -4215,7 +4216,7 @@ void Sema::InstantiateVariableInitialize } // We'll add an initializer to a for-range declaration later. - if (Var->isCXXForRangeDecl()) + if (Var->isCXXForRangeDecl() || Var->isObjCForDecl()) return; ActOnUninitializedDecl(Var); Modified: cfe/trunk/lib/Serialization/ASTReaderDecl.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTReaderDecl.cpp?rev=328749&r1=328748&r2=328749&view=diff ============================================================================== --- cfe/trunk/lib/Serialization/ASTReaderDecl.cpp (original) +++ cfe/trunk/lib/Serialization/ASTReaderDecl.cpp Wed Mar 28 17:56:24 2018 @@ -1279,6 +1279,7 @@ ASTDeclReader::RedeclarableResult ASTDec VD->NonParmVarDeclBits.ExceptionVar = Record.readInt(); VD->NonParmVarDeclBits.NRVOVariable = Record.readInt(); VD->NonParmVarDeclBits.CXXForRangeDecl = Record.readInt(); + VD->NonParmVarDeclBits.ObjCForDecl = Record.readInt(); VD->NonParmVarDeclBits.ARCPseudoStrong = Record.readInt(); VD->NonParmVarDeclBits.IsInline = Record.readInt(); VD->NonParmVarDeclBits.IsInlineSpecified = Record.readInt(); Modified: cfe/trunk/lib/Serialization/ASTWriterDecl.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTWriterDecl.cpp?rev=328749&r1=328748&r2=328749&view=diff ============================================================================== --- cfe/trunk/lib/Serialization/ASTWriterDecl.cpp (original) +++ cfe/trunk/lib/Serialization/ASTWriterDecl.cpp Wed Mar 28 17:56:24 2018 @@ -920,6 +920,7 @@ void ASTDeclWriter::VisitVarDecl(VarDecl Record.push_back(D->isExceptionVariable()); Record.push_back(D->isNRVOVariable()); Record.push_back(D->isCXXForRangeDecl()); + Record.push_back(D->isObjCForDecl()); Record.push_back(D->isARCPseudoStrong()); Record.push_back(D->isInline()); Record.push_back(D->isInlineSpecified()); @@ -2031,6 +2032,7 @@ void ASTWriter::WriteDeclAbbrevs() { Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); // isExceptionVariable Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); // isNRVOVariable Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); // isCXXForRangeDecl + Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); // isObjCForDecl Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); // isARCPseudoStrong Abv->Add(BitCodeAbbrevOp(0)); // isInline Abv->Add(BitCodeAbbrevOp(0)); // isInlineSpecified Added: cfe/trunk/test/SemaObjC/foreachtemplatized.mm URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaObjC/foreachtemplatized.mm?rev=328749&view=auto ============================================================================== --- cfe/trunk/test/SemaObjC/foreachtemplatized.mm (added) +++ cfe/trunk/test/SemaObjC/foreachtemplatized.mm Wed Mar 28 17:56:24 2018 @@ -0,0 +1,15 @@ +// RUN: %clang_cc1 -fsyntax-only -fobjc-arc -Wno-objc-root-class -std=c++11 -ast-dump %s | FileCheck %s + +// CHECK-NOT: ImplicitValueInitExpr + +@interface I +@end + +template <typename T> +void decode(I *p) { + for (I *k in p) {} +} + +void decode(I *p) { + decode<int>(p); +} _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits