From cb82a46e6eb416447d090c48494bf0e36aced500 Mon Sep 17 00:00:00 2001 From: Robert Widmann Date: Wed, 22 Jul 2020 10:46:13 -0700 Subject: [PATCH] [NFC] Stash Syntactic Information in EnumIsCaseExpr After the TypeLocs were removed here, the TypeRepr from the IsExpr was the only thing providing access to syntactic information from the parent IsExpr. In order to support this, it was possible to construct a bizarre ConditionalCheckedCastExpr that contained both semantic and syntactic information. This doesn't comport with the rest of the casting nodes, which force you to pick one or the other. Since we're rewriting an IsExpr into a EnumIsCaseExpr, let's just stash the syntactic information there. This unblocks a bit of cleanup. --- include/swift/AST/Expr.h | 22 ++++++++++------------ lib/AST/ASTWalker.cpp | 6 +++++- lib/AST/Expr.cpp | 11 ----------- lib/Sema/CSApply.cpp | 38 +++++++++++++++++++++++--------------- 4 files changed, 38 insertions(+), 39 deletions(-) diff --git a/include/swift/AST/Expr.h b/include/swift/AST/Expr.h index 21a3e2f45ffab..562f39ddd3650 100644 --- a/include/swift/AST/Expr.h +++ b/include/swift/AST/Expr.h @@ -4704,7 +4704,7 @@ class ForcedCheckedCastExpr final : public CheckedCastExpr { /// Represents an explicit conditional checked cast, which converts /// from a type to some subtype and produces an Optional value, which will be -/// .Some(x) if the cast succeeds, or .None if the cast fails. +/// .some(x) if the cast succeeds, or .none if the cast fails. /// Spelled 'a as? T' and produces a value of type 'T?'. class ConditionalCheckedCastExpr final : public CheckedCastExpr { SourceLoc QuestionLoc; @@ -4722,9 +4722,6 @@ class ConditionalCheckedCastExpr final : public CheckedCastExpr { static ConditionalCheckedCastExpr *createImplicit(ASTContext &ctx, Expr *sub, Type castTy); - static ConditionalCheckedCastExpr * - createImplicit(ASTContext &ctx, Expr *sub, TypeRepr *tyRepr, Type castTy); - /// Retrieve the location of the '?' that follows 'as'. SourceLoc getQuestionLoc() const { return QuestionLoc; } @@ -4911,20 +4908,21 @@ class IfExpr : public Expr { /// a particular case. class EnumIsCaseExpr : public Expr { Expr *SubExpr; + TypeRepr *CaseRepr; EnumElementDecl *Element; public: - EnumIsCaseExpr(Expr *SubExpr, EnumElementDecl *Element) - : Expr(ExprKind::EnumIsCase, /*implicit*/ true), - SubExpr(SubExpr), Element(Element) - {} - + EnumIsCaseExpr(Expr *SubExpr, TypeRepr *CaseRepr, EnumElementDecl *Element) + : Expr(ExprKind::EnumIsCase, /*implicit*/ true), SubExpr(SubExpr), + CaseRepr(CaseRepr), Element(Element) {} + Expr *getSubExpr() const { return SubExpr; } void setSubExpr(Expr *e) { SubExpr = e; } - + + TypeRepr *getCaseTypeRepr() const { return CaseRepr; } + EnumElementDecl *getEnumElement() const { return Element; } - void setEnumElement(EnumElementDecl *elt) { Element = elt; } - + SourceLoc getLoc() const { return SubExpr->getLoc(); } SourceLoc getStartLoc() const { return SubExpr->getStartLoc(); } SourceLoc getEndLoc() const { return SubExpr->getEndLoc(); } diff --git a/lib/AST/ASTWalker.cpp b/lib/AST/ASTWalker.cpp index b062f685fca66..be0fbc3a79914 100644 --- a/lib/AST/ASTWalker.cpp +++ b/lib/AST/ASTWalker.cpp @@ -951,7 +951,11 @@ class Traversal : public ASTVisitorsetSubExpr(Sub); } - + + if (auto *typerepr = E->getCaseTypeRepr()) + if (doIt(typerepr)) + return nullptr; + return E; } diff --git a/lib/AST/Expr.cpp b/lib/AST/Expr.cpp index ebf0ccfcb0ab7..c51d8590cedd4 100644 --- a/lib/AST/Expr.cpp +++ b/lib/AST/Expr.cpp @@ -1808,17 +1808,6 @@ ConditionalCheckedCastExpr::createImplicit(ASTContext &ctx, Expr *sub, return expr; } -ConditionalCheckedCastExpr * -ConditionalCheckedCastExpr::createImplicit(ASTContext &ctx, Expr *sub, - TypeRepr *tyRepr, Type castTy) { - auto *const expr = new (ctx) ConditionalCheckedCastExpr( - sub, SourceLoc(), SourceLoc(), new (ctx) TypeExpr(tyRepr)); - expr->setType(OptionalType::get(castTy)); - expr->setImplicit(); - expr->setCastType(castTy); - return expr; -} - IsExpr *IsExpr::create(ASTContext &ctx, SourceLoc isLoc, TypeRepr *tyRepr) { return new (ctx) IsExpr(nullptr, isLoc, new (ctx) TypeExpr(tyRepr)); } diff --git a/lib/Sema/CSApply.cpp b/lib/Sema/CSApply.cpp index 159c7c5025d33..ce49fd3c241f7 100644 --- a/lib/Sema/CSApply.cpp +++ b/lib/Sema/CSApply.cpp @@ -3574,12 +3574,14 @@ namespace { castKind == CheckedCastKind::ArrayDowncast || castKind == CheckedCastKind::DictionaryDowncast || castKind == CheckedCastKind::SetDowncast) { - auto *const cast = ConditionalCheckedCastExpr::createImplicit( - ctx, sub, castTypeRepr, toType); + auto *const cast = + ConditionalCheckedCastExpr::createImplicit(ctx, sub, toType); + cast->setType(OptionalType::get(toType)); + cast->setCastType(toType); cs.setType(cast, cast->getType()); // Type-check this conditional case. - Expr *result = handleConditionalCheckedCastExpr(cast, true); + Expr *result = handleConditionalCheckedCastExpr(cast, castTypeRepr); if (!result) return nullptr; @@ -3588,7 +3590,8 @@ namespace { // Match the optional value against its `Some` case. auto *someDecl = ctx.getOptionalSomeDecl(); - auto isSomeExpr = new (ctx) EnumIsCaseExpr(result, someDecl); + auto isSomeExpr = + new (ctx) EnumIsCaseExpr(result, castTypeRepr, someDecl); auto boolDecl = ctx.getBoolDecl(); if (!boolDecl) { @@ -4010,10 +4013,16 @@ namespace { } Expr *visitConditionalCheckedCastExpr(ConditionalCheckedCastExpr *expr) { + // Simplify and update the type we're casting to. + auto *const castTypeRepr = expr->getCastTypeRepr(); + const auto toType = simplifyType(cs.getType(castTypeRepr)); + expr->setCastType(toType); + cs.setType(castTypeRepr, toType); + // If we need to insert a force-unwrap for coercions of the form // 'as! T!', do so now. if (hasForcedOptionalResult(expr)) { - auto *coerced = handleConditionalCheckedCastExpr(expr); + auto *coerced = handleConditionalCheckedCastExpr(expr, castTypeRepr); if (!coerced) return nullptr; @@ -4021,29 +4030,28 @@ namespace { coerced, cs.getType(coerced)->getOptionalObjectType()); } - return handleConditionalCheckedCastExpr(expr); + return handleConditionalCheckedCastExpr(expr, castTypeRepr); } Expr *handleConditionalCheckedCastExpr(ConditionalCheckedCastExpr *expr, - bool isInsideIsExpr = false) { + TypeRepr *castTypeRepr) { + assert(castTypeRepr && + "cast requires TypeRepr; implicit casts are superfluous"); + // The subexpression is always an rvalue. auto &ctx = cs.getASTContext(); auto sub = cs.coerceToRValue(expr->getSubExpr()); expr->setSubExpr(sub); // Simplify and update the type we're casting to. - auto *const castTypeRepr = expr->getCastTypeRepr(); - const auto fromType = cs.getType(sub); - const auto toType = simplifyType(cs.getType(castTypeRepr)); - expr->setCastType(toType); - cs.setType(castTypeRepr, toType); + const auto toType = expr->getCastType(); bool isSubExprLiteral = isa(sub); auto castContextKind = - (SuppressDiagnostics || isInsideIsExpr || isSubExprLiteral) - ? CheckedCastContextKind::None - : CheckedCastContextKind::ConditionalCast; + (SuppressDiagnostics || expr->isImplicit() || isSubExprLiteral) + ? CheckedCastContextKind::None + : CheckedCastContextKind::ConditionalCast; auto castKind = TypeChecker::typeCheckCheckedCast( fromType, toType, castContextKind, cs.DC, expr->getLoc(), sub,