On Mar 29, 2012, at 6:13 PM, Eli Friedman wrote: > Author: efriedma > Date: Thu Mar 29 20:13:43 2012 > New Revision: 153716 > > URL: http://llvm.org/viewvc/llvm-project?rev=153716&view=rev > Log: > Make sure we perform the relevant implied conversions correctly for ObjC > methods with related result types. PR12384. > > > Modified: > cfe/trunk/lib/Sema/SemaStmt.cpp > cfe/trunk/test/CodeGenObjC/arc.m > cfe/trunk/test/SemaObjC/instancetype.m > cfe/trunk/test/SemaObjC/related-result-type-inference.m > > Modified: cfe/trunk/lib/Sema/SemaStmt.cpp > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaStmt.cpp?rev=153716&r1=153715&r2=153716&view=diff > ============================================================================== > --- cfe/trunk/lib/Sema/SemaStmt.cpp (original) > +++ cfe/trunk/lib/Sema/SemaStmt.cpp Thu Mar 29 20:13:43 2012 > @@ -1945,24 +1945,21 @@ > return ActOnCapScopeReturnStmt(ReturnLoc, RetValExp); > > QualType FnRetType; > - QualType DeclaredRetType; > + QualType RelatedRetType; > if (const FunctionDecl *FD = getCurFunctionDecl()) { > FnRetType = FD->getResultType(); > - DeclaredRetType = FnRetType; > if (FD->hasAttr<NoReturnAttr>() || > FD->getType()->getAs<FunctionType>()->getNoReturnAttr()) > Diag(ReturnLoc, diag::warn_noreturn_function_has_return_expr) > << FD->getDeclName(); > } else if (ObjCMethodDecl *MD = getCurMethodDecl()) { > - DeclaredRetType = MD->getResultType(); > + FnRetType = MD->getResultType(); > if (MD->hasRelatedResultType() && MD->getClassInterface()) { > // In the implementation of a method with a related return type, the > // type used to type-check the validity of return statements within the > // method body is a pointer to the type of the class being implemented. > - FnRetType = Context.getObjCInterfaceType(MD->getClassInterface()); > - FnRetType = Context.getObjCObjectPointerType(FnRetType); > - } else { > - FnRetType = DeclaredRetType; > + RelatedRetType = Context.getObjCInterfaceType(MD->getClassInterface()); > + RelatedRetType = Context.getObjCObjectPointerType(RelatedRetType); > } > } else // If we don't have a function/method context, bail. > return StmtError(); > @@ -2045,6 +2042,21 @@ > if (!FnRetType->isDependentType() && !RetValExp->isTypeDependent()) { > // we have a non-void function with an expression, continue checking > > + if (!RelatedRetType.isNull()) { > + // If we have a related result type, perform an extra conversion > here. > + // FIXME: The diagnostics here don't really describe what is > happening. > + InitializedEntity Entity = > + InitializedEntity::InitializeTemporary(RelatedRetType); > + > + ExprResult Res = PerformCopyInitialization(Entity, SourceLocation(), > + RetValExp); > + if (Res.isInvalid()) { > + // FIXME: Cleanup temporaries here, anyway? > + return StmtError(); > + } > + RetValExp = Res.takeAs<Expr>(); > + } > + > // C99 6.8.6.4p3(136): The return statement is not an assignment. The > // overlap restriction of subclause 6.5.16.1 does not apply to the case > of > // function return. > @@ -2068,17 +2080,6 @@ > } > > if (RetValExp) { > - // If we type-checked an Objective-C method's return type based > - // on a related return type, we may need to adjust the return > - // type again. Do so now. > - if (DeclaredRetType != FnRetType) { > - ExprResult result = PerformImplicitConversion(RetValExp, > - DeclaredRetType, > - AA_Returning); > - if (result.isInvalid()) return StmtError(); > - RetValExp = result.take(); > - } > - > CheckImplicitConversions(RetValExp, ReturnLoc); > RetValExp = MaybeCreateExprWithCleanups(RetValExp); > } > > Modified: cfe/trunk/test/CodeGenObjC/arc.m > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenObjC/arc.m?rev=153716&r1=153715&r2=153716&view=diff > ============================================================================== > --- cfe/trunk/test/CodeGenObjC/arc.m (original) > +++ cfe/trunk/test/CodeGenObjC/arc.m Thu Mar 29 20:13:43 2012 > @@ -640,9 +640,7 @@ > // CHECK-NEXT: store i8* {{%.*}}, i8** [[CMD]] > // CHECK-NEXT: [[T0:%.*]] = load [[TEST27]]** [[SELF]] > // CHECK-NEXT: [[T1:%.*]] = bitcast [[TEST27]]* [[T0]] to i8* > -// CHECK-NEXT: [[T2:%.*]] = call i8* @objc_retain(i8* [[T1]]) > -// CHECK-NEXT: [[T3:%.*]] = bitcast i8* [[T2]] > -// CHECK-NEXT: [[RET:%.*]] = bitcast > +// CHECK-NEXT: [[RET:%.*]] = call i8* @objc_retain(i8* [[T1]]) > // CHECK-NEXT: store i32 {{[0-9]+}}, i32* [[DEST]] > // CHECK-NEXT: [[T0:%.*]] = load [[TEST27]]** [[SELF]] > // CHECK-NEXT: [[T1:%.*]] = bitcast [[TEST27]]* [[T0]] to i8* > @@ -706,9 +704,7 @@ > // Return statement. > // CHECK-NEXT: [[T2:%.*]] = bitcast i8* [[CALL]] > // CHECK-NEXT: [[CALL:%.*]] = bitcast > -// CHECK-NEXT: [[T1:%.*]] = call i8* @objc_retain(i8* [[CALL]]) nounwind > -// CHECK-NEXT: [[T2:%.*]] = bitcast i8* [[T1]] > -// CHECK-NEXT: [[RET:%.*]] = bitcast > +// CHECK-NEXT: [[RET:%.*]] = call i8* @objc_retain(i8* [[CALL]]) nounwind > // CHECK-NEXT: store i32 1, i32* [[CLEANUP]] > > // Cleanup. > @@ -762,9 +758,7 @@ > // Return statement. > // CHECK-NEXT: [[T0:%.*]] = load [[TEST29]]** [[SELF]] > // CHECK-NEXT: [[T1:%.*]] = bitcast [[TEST29]]* [[T0]] to i8* > -// CHECK-NEXT: [[T2:%.*]] = call i8* @objc_retain(i8* [[T1]]) nounwind > -// CHECK-NEXT: [[T3:%.*]] = bitcast i8* [[T2]] > -// CHECK-NEXT: [[RET:%.*]] = bitcast > +// CHECK-NEXT: [[RET:%.*]] = call i8* @objc_retain(i8* [[T1]]) nounwind > // CHECK-NEXT: store i32 1, i32* [[CLEANUP]] > > // Cleanup. > @@ -819,9 +813,7 @@ > // Return. > // CHECK-NEXT: [[T0:%.*]] = load [[TEST30]]** [[SELF]] > // CHECK-NEXT: [[T1:%.*]] = bitcast [[TEST30]]* [[T0]] to i8* > -// CHECK-NEXT: [[T2:%.*]] = call i8* @objc_retain(i8* [[T1]]) > -// CHECK-NEXT: [[T3:%.*]] = bitcast i8* [[T2]] > -// CHECK-NEXT: [[RET:%.*]] = bitcast > +// CHECK-NEXT: [[RET:%.*]] = call i8* @objc_retain(i8* [[T1]]) > // CHECK-NEXT: store i32 1 > > // Cleanup. > > Modified: cfe/trunk/test/SemaObjC/instancetype.m > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaObjC/instancetype.m?rev=153716&r1=153715&r2=153716&view=diff > ============================================================================== > --- cfe/trunk/test/SemaObjC/instancetype.m (original) > +++ cfe/trunk/test/SemaObjC/instancetype.m Thu Mar 29 20:13:43 2012 > @@ -143,7 +143,7 @@ > > @implementation Subclass4 > + (id)alloc { > - return self; // expected-warning{{incompatible pointer types returning > 'Class' from a function with result type 'Subclass4 *'}} > + return self; // expected-warning{{incompatible pointer types casting > 'Class' to type 'Subclass4 *'}} > } > > - (Subclass3 *)init { return 0; } // don't complain: we lost the related > return type > @@ -166,12 +166,12 @@ > @implementation Subclass2 > - (instancetype)initSubclass2 { > Subclass1 *sc1 = [[Subclass1 alloc] init]; > - return sc1; // expected-warning{{incompatible pointer types returning > 'Subclass1 *' from a function with result type 'Subclass2 *'}} > + return sc1; // expected-warning{{incompatible pointer types casting > 'Subclass1 *' to type 'Subclass2 *'}} > } > - (void)methodOnSubclass2 {} > - (id)self { > Subclass1 *sc1 = [[Subclass1 alloc] init]; > - return sc1; // expected-warning{{incompatible pointer types returning > 'Subclass1 *' from a function with result type 'Subclass2 *'}} > + return sc1; // expected-warning{{incompatible pointer types casting > 'Subclass1 *' to type 'Subclass2 *'}}
Side-effect of these changes is causing new warning which is far less clear than the one it replaces. It will be matter of time before we hear from users complaining. - Fariborz _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
