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