<html><head><meta http-equiv="Content-Type" content="text/html charset=us-ascii"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;"><div>Awesome. This will be so much better than my minimal patch last year.</div><div><br></div><br><div><div>On Mar 8, 2013, at 16:54 , John McCall <<a href="mailto:rjmccall@apple.com">rjmccall@apple.com</a>> wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite"><div style="letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-size-adjust: auto; -webkit-text-stroke-width: 0px;">Author: rjmccall<br>Date: Fri Mar 8 18:54:31 2013<br>New Revision: 176743<br><br>URL:<span class="Apple-converted-space"> </span><a href="http://llvm.org/viewvc/llvm-project?rev=176743&view=rev">http://llvm.org/viewvc/llvm-project?rev=176743&view=rev</a><br>Log:<br>Adjust the special non-C++ enum block return type inference<br>so that it looks through certain syntactic forms and applies<br>even if normal inference would have succeeded.<br><br>There is potential for source incompatibility from this<br>change, but overall we feel that it produces a much<br>cleaner and more defensible result, and the block<br>compatibility rules should curb a lot of the potential<br>for annoyance.<br><br><a href="rdar://13200889">rdar://13200889</a><br><br>Modified:<br> cfe/trunk/lib/Sema/SemaLambda.cpp<br> cfe/trunk/test/SemaObjC/blocks.m<br><br>Modified: cfe/trunk/lib/Sema/SemaLambda.cpp<br>URL:<span class="Apple-converted-space"> </span><a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaLambda.cpp?rev=176743&r1=176742&r2=176743&view=diff">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaLambda.cpp?rev=176743&r1=176742&r2=176743&view=diff</a><br>==============================================================================<br>--- cfe/trunk/lib/Sema/SemaLambda.cpp (original)<br>+++ cfe/trunk/lib/Sema/SemaLambda.cpp Fri Mar 8 18:54:31 2013<br>@@ -225,73 +225,153 @@ void Sema::addLambdaParameters(CXXMethod<br> }<br>}<br><br>-static bool checkReturnValueType(const ASTContext &Ctx, const Expr *E,<br>- QualType &DeducedType,<br>- QualType &AlternateType) {<br>- // Handle ReturnStmts with no expressions.<br>- if (!E) {<br>- if (AlternateType.isNull())<br>- AlternateType = Ctx.VoidTy;<br>-<br>- return Ctx.hasSameType(DeducedType, Ctx.VoidTy);<br>- }<br>-<br>- QualType StrictType = E->getType();<br>- QualType LooseType = StrictType;<br>-<br>- // In C, enum constants have the type of their underlying integer type,<br>- // not the enum. When inferring block return types, we should allow<br>- // the enum type if an enum constant is used, unless the enum is<br>- // anonymous (in which case there can be no variables of its type).<br>- if (!Ctx.getLangOpts().CPlusPlus) {<br>- const DeclRefExpr *DRE = dyn_cast<DeclRefExpr>(E->IgnoreParenImpCasts());<br>- if (DRE) {<br>- const Decl *D = DRE->getDecl();<br>- if (const EnumConstantDecl *ECD = dyn_cast<EnumConstantDecl>(D)) {<br>- const EnumDecl *Enum = cast<EnumDecl>(ECD->getDeclContext());<br>- if (Enum->getDeclName() || Enum->getTypedefNameForAnonDecl())<br>- LooseType = Ctx.getTypeDeclType(Enum);<br>- }<br>+/// If this expression is an enumerator-like expression of some type<br>+/// T, return the type T; otherwise, return null.<br>+///<br>+/// Pointer comparisons on the result here should always work because<br>+/// it's derived from either the parent of an EnumConstantDecl<br>+/// (i.e. the definition) or the declaration returned by<br>+/// EnumType::getDecl() (i.e. the definition).<br>+static EnumDecl *findEnumForBlockReturn(Expr *E) {<br>+ // An expression is an enumerator-like expression of type T if,<br>+ // ignoring parens and parens-like expressions:<br>+ E = E->IgnoreParens();<br>+<br>+ // - it is an enumerator whose enum type is T or<br>+ if (DeclRefExpr *DRE = dyn_cast<DeclRefExpr>(E)) {<br>+ if (EnumConstantDecl *D<br>+ = dyn_cast<EnumConstantDecl>(DRE->getDecl())) {<br>+ return cast<EnumDecl>(D->getDeclContext());<br> }<br>+ return 0;<br> }<br><br>- // Special case for the first return statement we find.<br>- // The return type has already been tentatively set, but we might still<br>- // have an alternate type we should prefer.<br>- if (AlternateType.isNull())<br>- AlternateType = LooseType;<br>-<br>- if (Ctx.hasSameType(DeducedType, StrictType)) {<br>- // FIXME: The loose type is different when there are constants from two<br>- // different enums. We could consider warning here.<br>- if (AlternateType != Ctx.DependentTy)<br>- if (!Ctx.hasSameType(AlternateType, LooseType))<br>- AlternateType = Ctx.VoidTy;<br>- return true;<br>- }<br>-<br>- if (Ctx.hasSameType(DeducedType, LooseType)) {<br>- // Use DependentTy to signal that we're using an alternate type and may<br>- // need to add casts somewhere.<br>- AlternateType = Ctx.DependentTy;<br>- return true;<br>- }<br>-<br>- if (Ctx.hasSameType(AlternateType, StrictType) ||<br>- Ctx.hasSameType(AlternateType, LooseType)) {<br>- DeducedType = AlternateType;<br>- // Use DependentTy to signal that we're using an alternate type and may<br>- // need to add casts somewhere.<br>- AlternateType = Ctx.DependentTy;<br>- return true;<br>+ // - it is a comma expression whose RHS is an enumerator-like<br>+ // expression of type T or<br>+ if (BinaryOperator *BO = dyn_cast<BinaryOperator>(E)) {<br>+ if (BO->getOpcode() == BO_Comma)<br>+ return findEnumForBlockReturn(BO->getRHS());<br>+ return 0;<br> }<br><br>- return false;<br>+ // - it is a statement-expression whose value expression is an<br>+ // enumerator-like expression of type T or<br>+ if (StmtExpr *SE = dyn_cast<StmtExpr>(E)) {<br>+ if (Expr *last = dyn_cast_or_null<Expr>(SE->getSubStmt()->body_back()))<br>+ return findEnumForBlockReturn(last);<br>+ return 0;<br>+ }<br>+<br>+ // - it is a ternary conditional operator (not the GNU ?:<br>+ // extension) whose second and third operands are<br>+ // enumerator-like expressions of type T or<br>+ if (ConditionalOperator *CO = dyn_cast<ConditionalOperator>(E)) {<br>+ if (EnumDecl *ED = findEnumForBlockReturn(CO->getTrueExpr()))<br>+ if (ED == findEnumForBlockReturn(CO->getFalseExpr()))<br>+ return ED;<br>+ return 0;<br>+ }<br>+<br>+ // (implicitly:)<br>+ // - it is an implicit integral conversion applied to an<br>+ // enumerator-like expression of type T or<br>+ if (ImplicitCastExpr *ICE = dyn_cast<ImplicitCastExpr>(E)) {<br>+ // We can only see integral conversions in valid enumerator-like<br>+ // expressions.<br>+ if (ICE->getCastKind() == CK_IntegralCast)<br>+ return findEnumForBlockReturn(ICE->getSubExpr());<br>+ return 0;<br>+ }<br>+<br>+ // - it is an expression of that formal enum type.<br>+ if (const EnumType *ET = E->getType()->getAs<EnumType>()) {<br>+ return ET->getDecl();<br>+ }<br>+<br>+ // Otherwise, nope.<br>+ return 0;<br>+}<br>+<br>+/// Attempt to find a type T for which the returned expression of the<br>+/// given statement is an enumerator-like expression of that type.<br>+static EnumDecl *findEnumForBlockReturn(ReturnStmt *ret) {<br>+ if (Expr *retValue = ret->getRetValue())<br>+ return findEnumForBlockReturn(retValue);<br>+ return 0;<br>+}<br>+<br>+/// Attempt to find a common type T for which all of the returned<br>+/// expressions in a block are enumerator-like expressions of that<br>+/// type.<br>+static EnumDecl *findCommonEnumForBlockReturns(ArrayRef<ReturnStmt*> returns) {<br>+ ArrayRef<ReturnStmt*>::iterator i = returns.begin(), e = returns.end();<br>+<br>+ // Try to find one for the first return.<br>+ EnumDecl *ED = findEnumForBlockReturn(*i);<br>+ if (!ED) return 0;<br>+<br>+ // Check that the rest of the returns have the same enum.<br>+ for (++i; i != e; ++i) {<br>+ if (findEnumForBlockReturn(*i) != ED)<br>+ return 0;<br>+ }<br>+<br>+ // Never infer an anonymous enum type.<br>+ if (!ED->hasNameForLinkage()) return 0;<br>+<br>+ return ED;<br>+}<br>+<br>+/// Adjust the given return statements so that they formally return<br>+/// the given type. It should require, at most, an IntegralCast.<br>+static void adjustBlockReturnsToEnum(Sema &S, ArrayRef<ReturnStmt*> returns,<br>+ QualType returnType) {<br>+ for (ArrayRef<ReturnStmt*>::iterator<br>+ i = returns.begin(), e = returns.end(); i != e; ++i) {<br>+ ReturnStmt *ret = *i;<br>+ Expr *retValue = ret->getRetValue();<br>+ if (S.Context.hasSameType(retValue->getType(), returnType))<br>+ continue;<br>+<br>+ // Right now we only support integral fixup casts.<br>+ assert(returnType->isIntegralOrUnscopedEnumerationType());<br>+ assert(retValue->getType()->isIntegralOrUnscopedEnumerationType());<br>+<br>+ ExprWithCleanups *cleanups = dyn_cast<ExprWithCleanups>(retValue);<br>+<br>+ Expr *E = (cleanups ? cleanups->getSubExpr() : retValue);<br>+ E = ImplicitCastExpr::Create(S.Context, returnType, CK_IntegralCast,<br>+ E, /*base path*/ 0, VK_RValue);<br>+ if (cleanups) {<br>+ cleanups->setSubExpr(E);<br>+ } else {<br>+ ret->setRetValue(E);<br>+ }<br>+ }<br>}<br><br>void Sema::deduceClosureReturnType(CapturingScopeInfo &CSI) {<br> assert(CSI.HasImplicitReturnType);<br><br>+ // C++ Core Issue #975, proposed resolution:<br>+ // If a lambda-expression does not include a trailing-return-type,<br>+ // it is as if the trailing-return-type denotes the following type:<br>+ // - if there are no return statements in the compound-statement,<br>+ // or all return statements return either an expression of type<br>+ // void or no expression or braced-init-list, the type void;<br>+ // - otherwise, if all return statements return an expression<br>+ // and the types of the returned expressions after<br>+ // lvalue-to-rvalue conversion (4.1 [conv.lval]),<br>+ // array-to-pointer conversion (4.2 [conv.array]), and<br>+ // function-to-pointer conversion (4.3 [conv.func]) are the<br>+ // same, that common type;<br>+ // - otherwise, the program is ill-formed.<br>+ //<br>+ // In addition, in blocks in non-C++ modes, if all of the return<br>+ // statements are enumerator-like expressions of some type T, where<br>+ // T has a name for linkage, then we infer the return type of the<br>+ // block to be that type.<br>+<br> // First case: no return statements, implicit void return type.<br> ASTContext &Ctx = getASTContext();<br> if (CSI.Returns.empty()) {<br>@@ -308,6 +388,17 @@ void Sema::deduceClosureReturnType(Captu<br> if (CSI.ReturnType->isDependentType())<br> return;<br><br>+ // Try to apply the enum-fuzz rule.<br>+ if (!getLangOpts().CPlusPlus) {<br>+ assert(isa<BlockScopeInfo>(CSI));<br>+ const EnumDecl *ED = findCommonEnumForBlockReturns(CSI.Returns);<br>+ if (ED) {<br>+ CSI.ReturnType = Context.getTypeDeclType(ED);<br>+ adjustBlockReturnsToEnum(*this, CSI.Returns, CSI.ReturnType);<br>+ return;<br>+ }<br>+ }<br>+<br> // Third case: only one return statement. Don't bother doing extra work!<br> SmallVectorImpl<ReturnStmt*>::iterator I = CSI.Returns.begin(),<br> E = CSI.Returns.end();<br>@@ -316,47 +407,25 @@ void Sema::deduceClosureReturnType(Captu<br><br> // General case: many return statements.<br> // Check that they all have compatible return types.<br>- // For now, that means "identical", with an exception for enum constants.<br>- // (In C, enum constants have the type of their underlying integer type,<br>- // not the type of the enum. C++ uses the type of the enum.)<br>- QualType AlternateType;<br><br> // We require the return types to strictly match here.<br>+ // Note that we've already done the required promotions as part of<br>+ // processing the return statement.<br> for (; I != E; ++I) {<br> const ReturnStmt *RS = *I;<br> const Expr *RetE = RS->getRetValue();<br>- if (!checkReturnValueType(Ctx, RetE, CSI.ReturnType, AlternateType)) {<br>- // FIXME: This is a poor diagnostic for ReturnStmts without expressions.<br>- Diag(RS->getLocStart(),<br>- diag::err_typecheck_missing_return_type_incompatible)<br>- << (RetE ? RetE->getType() : Ctx.VoidTy) << CSI.ReturnType<br>- << isa<LambdaScopeInfo>(CSI);<br>- // Don't bother fixing up the return statements in the block if some of<br>- // them are unfixable anyway.<br>- AlternateType = Ctx.VoidTy;<br>- // Continue iterating so that we keep emitting diagnostics.<br>- }<br>- }<br><br>- // If our return statements turned out to be compatible, but we needed to<br>- // pick a different return type, go through and fix the ones that need it.<br>- if (AlternateType == Ctx.DependentTy) {<br>- for (SmallVectorImpl<ReturnStmt*>::iterator I = CSI.Returns.begin(),<br>- E = CSI.Returns.end();<br>- I != E; ++I) {<br>- ReturnStmt *RS = *I;<br>- Expr *RetE = RS->getRetValue();<br>- if (RetE->getType() == CSI.ReturnType)<br>- continue;<br>-<br>- // Right now we only support integral fixup casts.<br>- assert(CSI.ReturnType->isIntegralOrUnscopedEnumerationType());<br>- assert(RetE->getType()->isIntegralOrUnscopedEnumerationType());<br>- ExprResult Casted = ImpCastExprToType(RetE, CSI.ReturnType,<br>- CK_IntegralCast);<br>- assert(Casted.isUsable());<br>- RS->setRetValue(Casted.take());<br>- }<br>+ QualType ReturnType = (RetE ? RetE->getType() : Context.VoidTy);<br>+ if (Context.hasSameType(ReturnType, CSI.ReturnType))<br>+ continue;<br>+<br>+ // FIXME: This is a poor diagnostic for ReturnStmts without expressions.<br>+ // TODO: It's possible that the *first* return is the divergent one.<br>+ Diag(RS->getLocStart(),<br>+ diag::err_typecheck_missing_return_type_incompatible)<br>+ << ReturnType << CSI.ReturnType<br>+ << isa<LambdaScopeInfo>(CSI);<br>+ // Continue iterating so that we keep emitting diagnostics.<br> }<br>}<br><br><br>Modified: cfe/trunk/test/SemaObjC/blocks.m<br>URL:<span class="Apple-converted-space"> </span><a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaObjC/blocks.m?rev=176743&r1=176742&r2=176743&view=diff">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaObjC/blocks.m?rev=176743&r1=176742&r2=176743&view=diff</a><br>==============================================================================<br>--- cfe/trunk/test/SemaObjC/blocks.m (original)<br>+++ cfe/trunk/test/SemaObjC/blocks.m Fri Mar 8 18:54:31 2013<br>@@ -75,10 +75,11 @@ void foo10() {<br><br><br>// In C, enum constants have the type of the underlying integer type, not the<br>-// enumeration they are part of. We pretend the constants have enum type when<br>-// they are mixed with other expressions of enum type.<br>+// enumeration they are part of. We pretend the constants have enum type if<br>+// all the returns seem to be playing along.<br>enum CStyleEnum {<br>- CSE_Value = 1<br>+ CSE_Value = 1,<br>+ CSE_Value2 = 2<br>};<br>enum CStyleEnum getCSE();<br>typedef enum CStyleEnum (^cse_block_t)();<br>@@ -92,7 +93,9 @@ void testCStyleEnumInference(bool arg) {<br> a = ^{ // expected-error {{incompatible block pointer types assigning to 'cse_block_t' (aka 'enum CStyleEnum (^)()') from 'int (^)(void)'}}<br> return 1;<br> };<br>- a = ^{ // expected-error {{incompatible block pointer types assigning to 'cse_block_t' (aka 'enum CStyleEnum (^)()') from 'int (^)(void)'}}<br>+<br>+ // No warning here.<br>+ a = ^{<br> return CSE_Value;<br> };<br><br>@@ -114,6 +117,15 @@ void testCStyleEnumInference(bool arg) {<br> else<br> return 1;<br> };<br>+<br>+ //<span class="Apple-converted-space"> </span><a href="rdar://13200889">rdar://13200889</a><br>+ extern void check_enum(void);<br>+ a = ^{<br>+ return (arg ? (CSE_Value) : (check_enum(), (!arg ? CSE_Value2 : getCSE())));<br>+ };<br>+ a = ^{<br>+ return (arg ? (CSE_Value) : ({check_enum(); CSE_Value2; }));<br>+ };<br>}<br><br><br><br><br>_______________________________________________<br>cfe-commits mailing list<br><a href="mailto:cfe-commits@cs.uiuc.edu">cfe-commits@cs.uiuc.edu</a><br><a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits</a></div></blockquote></div><br></body></html>