Merge pull request #36523 from ahoppen/pr/revert-remove-unreachable-code

Revert "Sema: Remove some unreachable code from CSApply"

Author: ahoppen

include/swift/AST/DiagnosticsSema.def

@@ -225,6 +225,13 @@ ERROR(cannot_apply_lvalue_binop_to_subelement,none,
 ERROR(cannot_apply_lvalue_binop_to_rvalue,none,
       "left side of mutating operator has immutable type %0", (Type))
 
+ERROR(cannot_subscript_base,none,
+      "cannot subscript a value of type %0",
+      (Type))
+
+ERROR(cannot_subscript_ambiguous_base,none,
+      "cannot subscript a value of incorrect or ambiguous type", ())
+
 ERROR(cannot_subscript_nil_literal,none,
       "cannot subscript a nil literal value", ())
 ERROR(conditional_cast_from_nil,none,

include/swift/AST/Expr.h

@@ -3148,9 +3148,24 @@ class LoadExpr : public ImplicitConversionExpr {
   static bool classof(const Expr *E) { return E->getKind() == ExprKind::Load; }
 };
 
+/// This is a conversion from an expression of UnresolvedType to an arbitrary
+/// other type, and from an arbitrary type to UnresolvedType.  This node does
+/// not appear in valid code, only in code involving diagnostics.
+class UnresolvedTypeConversionExpr : public ImplicitConversionExpr {
+public:
+  UnresolvedTypeConversionExpr(Expr *subExpr, Type type)
+  : ImplicitConversionExpr(ExprKind::UnresolvedTypeConversion, subExpr, type) {}
+
+  static bool classof(const Expr *E) {
+    return E->getKind() == ExprKind::UnresolvedTypeConversion;
+  }
+};
+
 /// FunctionConversionExpr - Convert a function to another function type,
 /// which might involve renaming the parameters or handling substitutions
 /// of subtypes (in the return) or supertypes (in the input).
+///
+/// FIXME: This should be a CapturingExpr.
 class FunctionConversionExpr : public ImplicitConversionExpr {
 public:
   FunctionConversionExpr(Expr *subExpr, Type type)

include/swift/AST/ExprNodes.def

@@ -151,6 +151,7 @@ ABSTRACT_EXPR(Apply, Expr)
 ABSTRACT_EXPR(ImplicitConversion, Expr)
   EXPR(Load, ImplicitConversionExpr)
   EXPR(DestructureTuple, ImplicitConversionExpr)
+  EXPR(UnresolvedTypeConversion, ImplicitConversionExpr)
   EXPR(FunctionConversion, ImplicitConversionExpr)
   EXPR(CovariantFunctionConversion, ImplicitConversionExpr)
   EXPR(CovariantReturnConversion, ImplicitConversionExpr)

lib/AST/ASTDumper.cpp

@@ -2270,6 +2270,11 @@ class PrintExpr : public ExprVisitor<PrintExpr> {
     printRec(E->getResultExpr());
     PrintWithColorRAII(OS, ParenthesisColor) << ')';
   }
+  void visitUnresolvedTypeConversionExpr(UnresolvedTypeConversionExpr *E) {
+    printCommon(E, "unresolvedtype_conversion_expr") << '\n';
+    printRec(E->getSubExpr());
+    PrintWithColorRAII(OS, ParenthesisColor) << ')';
+  }
   void visitFunctionConversionExpr(FunctionConversionExpr *E) {
     printCommon(E, "function_conversion_expr") << '\n';
     printRec(E->getSubExpr());

lib/AST/Expr.cpp

@@ -338,6 +338,7 @@ ConcreteDeclRef Expr::getReferencedDecl(bool stopAtParenExpr) const {
 
   PASS_THROUGH_REFERENCE(Load, getSubExpr);
   NO_REFERENCE(DestructureTuple);
+  NO_REFERENCE(UnresolvedTypeConversion);
   PASS_THROUGH_REFERENCE(FunctionConversion, getSubExpr);
   PASS_THROUGH_REFERENCE(CovariantFunctionConversion, getSubExpr);
   PASS_THROUGH_REFERENCE(CovariantReturnConversion, getSubExpr);
@@ -662,6 +663,7 @@ bool Expr::canAppendPostfixExpression(bool appendingPostfixOperator) const {
 
   case ExprKind::Load:
   case ExprKind::DestructureTuple:
+  case ExprKind::UnresolvedTypeConversion:
   case ExprKind::FunctionConversion:
   case ExprKind::CovariantFunctionConversion:
   case ExprKind::CovariantReturnConversion:

lib/SILGen/SILGenExpr.cpp

@@ -446,6 +446,8 @@ namespace {
     RValue visitConditionalBridgeFromObjCExpr(ConditionalBridgeFromObjCExpr *E,
                                               SGFContext C);
     RValue visitArchetypeToSuperExpr(ArchetypeToSuperExpr *E, SGFContext C);
+    RValue visitUnresolvedTypeConversionExpr(UnresolvedTypeConversionExpr *E,
+                                             SGFContext C);
     RValue visitFunctionConversionExpr(FunctionConversionExpr *E,
                                        SGFContext C);
     RValue visitCovariantFunctionConversionExpr(
@@ -956,6 +958,12 @@ RValue RValueEmitter::visitSuperRefExpr(SuperRefExpr *E, SGFContext C) {
   return RValue(SGF, E, result);
 }
 
+RValue RValueEmitter::
+visitUnresolvedTypeConversionExpr(UnresolvedTypeConversionExpr *E,
+                                  SGFContext C) {
+  llvm_unreachable("invalid code made its way into SILGen");
+}
+
 RValue RValueEmitter::visitOtherConstructorDeclRefExpr(
                                 OtherConstructorDeclRefExpr *E, SGFContext C) {
   // This should always be a child of an ApplyExpr and so will be emitted by

lib/Sema/CSApply.cpp

@@ -2894,7 +2894,8 @@ namespace {
       auto type = simplifyType(openedType);
       cs.setType(expr, type);
 
-      assert(!type->is<UnresolvedType>());
+      if (type->is<UnresolvedType>())
+        return expr;
 
       auto &ctx = cs.getASTContext();
 
@@ -2917,11 +2918,14 @@ namespace {
       ConcreteDeclRef witness = conformance.getWitnessByName(
           conformingType->getRValueType(), constrName);
 
-      auto selectedOverload = solution.getOverloadChoice(
+      auto selectedOverload = solution.getOverloadChoiceIfAvailable(
           cs.getConstraintLocator(expr, ConstraintLocator::ConstructorMember));
 
+      if (!selectedOverload)
+        return nullptr;
+
       auto fnType =
-          simplifyType(selectedOverload.openedType)->castTo<FunctionType>();
+          simplifyType(selectedOverload->openedType)->castTo<FunctionType>();
 
       auto newArg = coerceCallArguments(
           expr->getArg(), fnType, witness,
@@ -3221,8 +3225,18 @@ namespace {
       // Determine the declaration selected for this overloaded reference.
       auto memberLocator = cs.getConstraintLocator(expr,
                                                    ConstraintLocator::Member);
-      auto selected = solution.getOverloadChoice(memberLocator);
+      auto selectedElt = solution.getOverloadChoiceIfAvailable(memberLocator);
+
+      if (!selectedElt) {
+        // If constraint solving resolved this to an UnresolvedType, then we're
+        // in an ambiguity tolerant mode used for diagnostic generation.  Just
+        // leave this as whatever type of member reference it already is.
+        Type resultTy = simplifyType(cs.getType(expr));
+        cs.setType(expr, resultTy);
+        return expr;
+      }
 
+      auto selected = *selectedElt;
       if (!selected.choice.getBaseType()) {
         // This is one of the "outer alternatives", meaning the innermost
         // methods didn't work out.
@@ -3454,19 +3468,40 @@ namespace {
     Expr *visitSubscriptExpr(SubscriptExpr *expr) {
       auto *memberLocator =
           cs.getConstraintLocator(expr, ConstraintLocator::SubscriptMember);
-      auto overload = solution.getOverloadChoice(memberLocator);
+      auto overload = solution.getOverloadChoiceIfAvailable(memberLocator);
+
+      // Handles situation where there was a solution available but it didn't
+      // have a proper overload selected from subscript call, might be because
+      // solver was allowed to return free or unresolved types, which can
+      // happen while running diagnostics on one of the expressions.
+      if (!overload) {
+        auto *base = expr->getBase();
+        auto &de = cs.getASTContext().Diags;
+        auto baseType = cs.getType(base);
 
-      if (overload.choice.getKind() ==
+        if (auto errorType = baseType->getAs<ErrorType>()) {
+          de.diagnose(base->getLoc(), diag::cannot_subscript_base,
+                      errorType->getOriginalType())
+              .highlight(base->getSourceRange());
+        } else {
+          de.diagnose(base->getLoc(), diag::cannot_subscript_ambiguous_base)
+              .highlight(base->getSourceRange());
+        }
+
+        return nullptr;
+      }
+
+      if (overload->choice.getKind() ==
               OverloadChoiceKind::KeyPathDynamicMemberLookup) {
         return buildDynamicMemberLookupRef(
             expr, expr->getBase(), expr->getIndex()->getStartLoc(), SourceLoc(),
-            overload, memberLocator);
+            *overload, memberLocator);
       }
 
       return buildSubscript(
           expr->getBase(), expr->getIndex(), expr->getArgumentLabels(),
           expr->hasTrailingClosure(), cs.getConstraintLocator(expr),
-          expr->isImplicit(), expr->getAccessSemantics(), overload);
+          expr->isImplicit(), expr->getAccessSemantics(), *overload);
     }
 
     /// "Finish" an array expression by filling in the semantic expression.
@@ -4337,7 +4372,8 @@ namespace {
       expr->getSubPattern()->forEachVariable([](VarDecl *VD) {
         VD->setInvalid();
       });
-      if (!SuppressDiagnostics) {
+      if (!SuppressDiagnostics
+          && !cs.getType(simplified)->is<UnresolvedType>()) {
         auto &de = cs.getASTContext().Diags;
         de.diagnose(simplified->getLoc(), diag::pattern_in_expr,
                     expr->getSubPattern()->getKind());
@@ -4816,12 +4852,18 @@ namespace {
         // If this is an unresolved link, make sure we resolved it.
         if (kind == KeyPathExpr::Component::Kind::UnresolvedProperty ||
             kind == KeyPathExpr::Component::Kind::UnresolvedSubscript) {
-          auto foundDecl = solution.getOverloadChoice(locator);
+          auto foundDecl = solution.getOverloadChoiceIfAvailable(locator);
+          if (!foundDecl) {
+            // If we couldn't resolve the component, leave it alone.
+            resolvedComponents.push_back(origComponent);
+            componentTy = origComponent.getComponentType();
+            continue;
+          }
 
           isDynamicMember =
-              foundDecl.choice.getKind() ==
+              foundDecl->choice.getKind() ==
                   OverloadChoiceKind::DynamicMemberLookup ||
-              foundDecl.choice.getKind() ==
+              foundDecl->choice.getKind() ==
                   OverloadChoiceKind::KeyPathDynamicMemberLookup;
 
           // If this was a @dynamicMemberLookup property, then we actually
@@ -4852,9 +4894,11 @@ namespace {
         case KeyPathExpr::Component::Kind::OptionalChain: {
           didOptionalChain = true;
           // Chaining always forces the element to be an rvalue.
-          assert(!componentTy->hasUnresolvedType());
           auto objectTy =
               componentTy->getWithoutSpecifierType()->getOptionalObjectType();
+          if (componentTy->hasUnresolvedType() && !objectTy) {
+            objectTy = componentTy;
+          }
           assert(objectTy);
 
           auto loc = origComponent.getLoc();
@@ -4906,8 +4950,8 @@ namespace {
       }
 
       // Wrap a non-optional result if there was chaining involved.
-      assert(!componentTy->hasUnresolvedType());
       if (didOptionalChain && componentTy &&
+          !componentTy->hasUnresolvedType() &&
           !componentTy->getWithoutSpecifierType()->isEqual(leafTy)) {
         assert(leafTy->getOptionalObjectType()->isEqual(
             componentTy->getWithoutSpecifierType()));
@@ -4926,8 +4970,8 @@ namespace {
 
       // The final component type ought to line up with the leaf type of the
       // key path.
-      assert(!componentTy->hasUnresolvedType());
-      assert(componentTy->getWithoutSpecifierType()->isEqual(leafTy));
+      assert(!componentTy || componentTy->hasUnresolvedType()
+             || componentTy->getWithoutSpecifierType()->isEqual(leafTy));
 
       if (!isFunctionType)
         return E;
@@ -5038,13 +5082,18 @@ namespace {
 
       // Unwrap the last component type, preserving @lvalue-ness.
       auto optionalTy = components.back().getComponentType();
-      assert(!optionalTy->hasUnresolvedType());
       Type objectTy;
       if (auto lvalue = optionalTy->getAs<LValueType>()) {
         objectTy = lvalue->getObjectType()->getOptionalObjectType();
+        if (optionalTy->hasUnresolvedType() && !objectTy) {
+          objectTy = optionalTy;
+        }
         objectTy = LValueType::get(objectTy);
       } else {
         objectTy = optionalTy->getOptionalObjectType();
+        if (optionalTy->hasUnresolvedType() && !objectTy) {
+          objectTy = optionalTy;
+        }
       }
       assert(objectTy);
 
@@ -5473,9 +5522,11 @@ Expr *ExprRewriter::coerceExistential(Expr *expr, Type toType) {
   Type toInstanceType = toType;
 
   // Look through metatypes
-  while (fromInstanceType->is<AnyMetatypeType>() &&
+  while ((fromInstanceType->is<UnresolvedType>() ||
+          fromInstanceType->is<AnyMetatypeType>()) &&
          toInstanceType->is<ExistentialMetatypeType>()) {
-    fromInstanceType = fromInstanceType->castTo<AnyMetatypeType>()->getInstanceType();
+    if (!fromInstanceType->is<UnresolvedType>())
+      fromInstanceType = fromInstanceType->castTo<AnyMetatypeType>()->getInstanceType();
     toInstanceType = toInstanceType->castTo<ExistentialMetatypeType>()->getInstanceType();
   }
 
@@ -5675,6 +5726,11 @@ Expr *ExprRewriter::coerceCallArguments(
         LocatorPathElt::ApplyArgToParam(argIdx, paramIdx, flags));
   };
 
+  bool matchCanFail =
+      llvm::any_of(params, [](const AnyFunctionType::Param &param) {
+        return param.getPlainType()->hasUnresolvedType();
+      });
+
   // Determine whether this application has curried self.
   bool skipCurriedSelf = apply ? hasCurriedSelf(cs, callee, apply) : true;
   // Determine the parameter bindings.
@@ -5732,7 +5788,9 @@ Expr *ExprRewriter::coerceCallArguments(
       args, params, paramInfo, unlabeledTrailingClosureIndex,
       /*allowFixes=*/false, listener, trailingClosureMatching);
 
-  assert(callArgumentMatch && "Call arguments did not match up?");
+  assert((matchCanFail || callArgumentMatch) &&
+         "Call arguments did not match up?");
+  (void)matchCanFail;
 
   auto parameterBindings = std::move(callArgumentMatch->parameterBindings);
 
@@ -6469,7 +6527,8 @@ Expr *ExprRewriter::coerceToType(Expr *expr, Type toType,
   if (knownRestriction != solution.ConstraintRestrictions.end()) {
     switch (knownRestriction->second) {
     case ConversionRestrictionKind::DeepEquality: {
-      assert(!toType->hasUnresolvedType());
+      if (toType->hasUnresolvedType())
+        break;
 
       // HACK: Fix problem related to Swift 4 mode (with assertions),
       // since Swift 4 mode allows passing arguments with extra parens
@@ -7030,8 +7089,9 @@ Expr *ExprRewriter::coerceToType(Expr *expr, Type toType,
     break;
   }
 
-  assert(!fromType->hasUnresolvedType());
-  assert(!toType->hasUnresolvedType());
+  // Unresolved types come up in diagnostics for lvalue and inout types.
+  if (fromType->hasUnresolvedType() || toType->hasUnresolvedType())
+    return cs.cacheType(new (ctx) UnresolvedTypeConversionExpr(expr, toType));
 
   // Use an opaque type to abstract a value of the underlying concrete type.
   if (toType->getAs<OpaqueTypeArchetypeType>()) {
@@ -7608,14 +7668,19 @@ Expr *ExprRewriter::finishApply(ApplyExpr *apply, Type openedType,
   // FIXME: handle unwrapping everywhere else
   assert(!unwrapResult);
 
-  assert(!cs.getType(fn)->is<UnresolvedType>());
+  // If this is an UnresolvedType in the system, preserve it.
+  if (cs.getType(fn)->is<UnresolvedType>()) {
+    cs.setType(apply, cs.getType(fn));
+    return apply;
+  }
 
   // We have a type constructor.
   auto metaTy = cs.getType(fn)->castTo<AnyMetatypeType>();
   auto ty = metaTy->getInstanceType();
 
   // If we're "constructing" a tuple type, it's simply a conversion.
   if (auto tupleTy = ty->getAs<TupleType>()) {
+    // FIXME: Need an AST to represent this properly.
     return coerceToType(apply->getArg(), tupleTy, locator);
   }
 
@@ -7624,14 +7689,19 @@ Expr *ExprRewriter::finishApply(ApplyExpr *apply, Type openedType,
   auto *ctorLocator =
       cs.getConstraintLocator(locator, {ConstraintLocator::ApplyFunction,
                                         ConstraintLocator::ConstructorMember});
-  auto selected = solution.getOverloadChoice(ctorLocator);
+  auto selected = solution.getOverloadChoiceIfAvailable(ctorLocator);
+  if (!selected) {
+    assert(ty->hasError() || ty->hasUnresolvedType());
+    cs.setType(apply, ty);
+    return apply;
+  }
 
   assert(ty->getNominalOrBoundGenericNominal() || ty->is<DynamicSelfType>() ||
          ty->isExistentialType() || ty->is<ArchetypeType>());
 
   // Consider the constructor decl reference expr 'implicit', but the
   // constructor call expr itself has the apply's 'implicitness'.
-  Expr *declRef = buildMemberRef(fn, /*dotLoc=*/SourceLoc(), selected,
+  Expr *declRef = buildMemberRef(fn, /*dotLoc=*/SourceLoc(), *selected,
                                  DeclNameLoc(fn->getEndLoc()), locator,
                                  ctorLocator, /*implicit=*/true,
                                  AccessSemantics::Ordinary);

validation-test/IDE/crashers_2_fixed/rdar75200446.swift

@@ -0,0 +1,3 @@
+// RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=COMPLETE
+
+.undefined() #^COMPLETE^#

validation-test/IDE/crashers_2_fixed/rdar75219757.swift

@@ -0,0 +1,3 @@
+// RUN: %target-swift-ide-test -code-completion -source-filename %s -code-completion-token=COMPLETE
+
+0odd.foo.bar #^COMPLETE^#