Merge pull request #16250 from slavapestov/start-removing-shallow-type-checking

Start removing typeCheckExpressionShallow()

Author: slavapestov

lib/Sema/CSApply.cpp

@@ -131,17 +131,17 @@ void Solution::computeSubstitutions(
 /// \param diag The diagnostic to emit if the protocol definition doesn't
 /// have a requirement with the given name.
 ///
-/// \returns The named witness, or nullptr if no witness could be found.
-template <typename DeclTy>
-static DeclTy *findNamedWitnessImpl(
+/// \returns The named witness, or an empty ConcreteDeclRef if no witness
+/// could be found.
+ConcreteDeclRef findNamedWitnessImpl(
                         TypeChecker &tc, DeclContext *dc, Type type,
                         ProtocolDecl *proto, DeclName name,
                         Diag<> diag,
                         Optional<ProtocolConformanceRef> conformance = None) {
   // Find the named requirement.
-  DeclTy *requirement = nullptr;
+  ValueDecl *requirement = nullptr;
   for (auto member : proto->getMembers()) {
-    auto d = dyn_cast<DeclTy>(member);
+    auto d = dyn_cast<ValueDecl>(member);
     if (!d || !d->hasName())
       continue;
 
@@ -159,7 +159,7 @@ static DeclTy *findNamedWitnessImpl(
   // Find the member used to satisfy the named requirement.
   if (!conformance) {
     conformance = tc.conformsToProtocol(type, proto, dc,
-                                          ConformanceCheckFlags::InExpression);
+                                        ConformanceCheckFlags::InExpression);
     if (!conformance)
       return nullptr;
   }
@@ -169,8 +169,7 @@ static DeclTy *findNamedWitnessImpl(
   if (!conformance->isConcrete())
     return requirement;
   auto concrete = conformance->getConcrete();
-  // FIXME: Dropping substitutions here.
-  return cast_or_null<DeclTy>(concrete->getWitnessDecl(requirement, &tc));
+  return concrete->getWitnessDeclRef(requirement, &tc);
 }
 
 static bool shouldAccessStorageDirectly(Expr *base, VarDecl *member,
@@ -2377,18 +2376,21 @@ namespace {
       DeclName name(tc.Context, DeclBaseName::createConstructor(),
                     { tc.Context.Id_stringInterpolation });
       auto member
-        = findNamedWitnessImpl<ConstructorDecl>(
+        = findNamedWitnessImpl(
             tc, dc, type,
             interpolationProto, name,
             diag::interpolation_broken_proto);
 
       DeclName segmentName(tc.Context, DeclBaseName::createConstructor(),
                            { tc.Context.Id_stringInterpolationSegment });
       auto segmentMember
-        = findNamedWitnessImpl<ConstructorDecl>(
+        = findNamedWitnessImpl(
             tc, dc, type, interpolationProto, segmentName,
             diag::interpolation_broken_proto);
-      if (!member || !segmentMember)
+      if (!member ||
+          !segmentMember ||
+          !isa<ConstructorDecl>(member.getDecl()) ||
+          !isa<ConstructorDecl>(segmentMember.getDecl()))
         return nullptr;
 
       // Build a reference to the init(stringInterpolation:) initializer.
@@ -2399,7 +2401,7 @@ namespace {
       Expr *memberRef =
         new (tc.Context) MemberRefExpr(typeRef,
                                        expr->getStartLoc(),
-                                       member,
+                                       member.getDecl(),
                                        DeclNameLoc(expr->getStartLoc()),
                                        /*Implicit=*/true);
       cs.cacheSubExprTypes(memberRef);
@@ -3298,11 +3300,7 @@ namespace {
       auto cond
         = solution.convertBooleanTypeToBuiltinI1(expr->getCondExpr(),
                                                  cs.getConstraintLocator(expr));
-      if (!cond) {
-        cs.setType(expr->getCondExpr(), ErrorType::get(resultTy));
-      } else {
-        expr->setCondExpr(cond);
-      }
+      expr->setCondExpr(cond);
 
       // Coerce the then/else branches to the common type.
       expr->setThenExpr(coerceToType(expr->getThenExpr(), resultTy,
@@ -7013,45 +7011,38 @@ static Type adjustSelfTypeForMember(Type baseTy, ValueDecl *member,
                                     AccessSemantics semantics,
                                     DeclContext *UseDC) {
   auto baseObjectTy = baseTy->getWithoutSpecifierType();
-  if (auto func = dyn_cast<AbstractFunctionDecl>(member)) {
+
+  if (isa<ConstructorDecl>(member))
+    return baseObjectTy;
+
+  if (auto func = dyn_cast<FuncDecl>(member)) {
     // If 'self' is an inout type, turn the base type into an lvalue
     // type with the same qualifiers.
-    auto selfParam = func->getInterfaceType()->getAs<AnyFunctionType>()->getParams();
-    assert(selfParam.size() == 1 && "found invalid arity of self param");
-    if (selfParam[0].getParameterFlags().isInOut()) {
-      // Unless we're looking at a nonmutating existential member.  In which
-      // case, the member will be modeled as an inout but ExistentialMemberRef
-      // and ArchetypeMemberRef want to take the base as an rvalue.
-      if (auto *fd = dyn_cast<FuncDecl>(func))
-        if (!fd->isMutating() && baseObjectTy->is<ArchetypeType>())
-          return baseObjectTy;
-      
+    if (func->isMutating())
       return InOutType::get(baseObjectTy);
-    }
 
     // Otherwise, return the rvalue type.
     return baseObjectTy;
   }
 
   // If the base of the access is mutable, then we may be invoking a getter or
   // setter that requires the base to be mutable.
-  if (auto *SD = dyn_cast<AbstractStorageDecl>(member)) {
-    bool isSettableFromHere = SD->isSettable(UseDC)
-      && (!UseDC->getASTContext().LangOpts.EnableAccessControl
-          || SD->isSetterAccessibleFrom(UseDC));
-
-    // If neither the property's getter nor its setter are mutating, the base
-    // can be an rvalue.
-    if (!SD->isGetterMutating()
-        && (!isSettableFromHere || !SD->isSetterMutating()))
-      return baseObjectTy;
-
-    // If we're calling an accessor, keep the base as an inout type, because the
-    // getter may be mutating.
-    if (SD->hasAccessorFunctions() && baseTy->is<InOutType>() &&
-        semantics != AccessSemantics::DirectToStorage)
-      return InOutType::get(baseObjectTy);
-  }
+  auto *SD = cast<AbstractStorageDecl>(member);
+  bool isSettableFromHere = SD->isSettable(UseDC)
+    && (!UseDC->getASTContext().LangOpts.EnableAccessControl
+        || SD->isSetterAccessibleFrom(UseDC));
+
+  // If neither the property's getter nor its setter are mutating, the base
+  // can be an rvalue.
+  if (!SD->isGetterMutating()
+      && (!isSettableFromHere || !SD->isSetterMutating()))
+    return baseObjectTy;
+
+  // If we're calling an accessor, keep the base as an inout type, because the
+  // getter may be mutating.
+  if (SD->hasAccessorFunctions() && baseTy->is<InOutType>() &&
+      semantics != AccessSemantics::DirectToStorage)
+    return InOutType::get(baseObjectTy);
 
   // Accesses to non-function members in value types are done through an @lvalue
   // type.
@@ -7287,11 +7278,11 @@ Expr *ExprRewriter::convertLiteralInPlace(Expr *literal,
 
     // Find the witness that we'll use to initialize the type via a builtin
     // literal.
-    auto witness = findNamedWitnessImpl<AbstractFunctionDecl>(
+    auto witness = findNamedWitnessImpl(
                      tc, dc, type->getRValueType(), builtinProtocol,
                      builtinLiteralFuncName, brokenBuiltinProtocolDiag,
                      *builtinConformance);
-    if (!witness)
+    if (!witness || !isa<AbstractFunctionDecl>(witness.getDecl()))
       return nullptr;
 
     // Form a reference to the builtin conversion function.
@@ -7301,7 +7292,7 @@ Expr *ExprRewriter::convertLiteralInPlace(Expr *literal,
 
     Expr *unresolvedDot = new (tc.Context) UnresolvedDotExpr(
                                              base, SourceLoc(),
-                                             witness->getFullName(),
+                                             witness.getDecl()->getFullName(),
                                              DeclNameLoc(base->getEndLoc()),
                                              /*Implicit=*/true);
     (void)tc.typeCheckExpression(unresolvedDot, dc);
@@ -7352,11 +7343,11 @@ Expr *ExprRewriter::convertLiteralInPlace(Expr *literal,
   }
 
   // Find the witness that we'll use to initialize the literal value.
-  auto witness = findNamedWitnessImpl<AbstractFunctionDecl>(
+  auto witness = findNamedWitnessImpl(
                    tc, dc, type->getRValueType(), protocol,
                    literalFuncName, brokenProtocolDiag,
                    conformance);
-  if (!witness)
+  if (!witness || !isa<AbstractFunctionDecl>(witness.getDecl()))
     return nullptr;
 
   // Form a reference to the conversion function.
@@ -7366,7 +7357,7 @@ Expr *ExprRewriter::convertLiteralInPlace(Expr *literal,
 
   Expr *unresolvedDot = new (tc.Context) UnresolvedDotExpr(
                                            base, SourceLoc(),
-                                           witness->getFullName(),
+                                           witness.getDecl()->getFullName(),
                                            DeclNameLoc(base->getEndLoc()),
                                            /*Implicit=*/true);
   (void)tc.typeCheckExpression(unresolvedDot, dc);
@@ -8367,18 +8358,20 @@ Expr *TypeChecker::callWitness(Expr *base, DeclContext *dc,
   if (auto metaType = type->getAs<AnyMetatypeType>())
     type = metaType->getInstanceType();
 
-  auto witness = findNamedWitnessImpl<AbstractFunctionDecl>(
+  auto witness = findNamedWitnessImpl(
                    *this, dc, type->getRValueType(), protocol,
                    name, brokenProtocolDiag);
-  if (!witness)
+  if (!witness || !isa<AbstractFunctionDecl>(witness.getDecl()))
     return nullptr;
 
+  auto *witnessFn = cast<AbstractFunctionDecl>(witness.getDecl());
+
   // Form a syntactic expression that describes the reference to the
   // witness.
   // FIXME: Egregious hack.
   auto unresolvedDot = new (Context) UnresolvedDotExpr(
                                        base, SourceLoc(),
-                                       witness->getFullName(),
+                                       witness.getDecl()->getFullName(),
                                        DeclNameLoc(base->getEndLoc()),
                                        /*Implicit=*/true);
   unresolvedDot->setFunctionRefKind(FunctionRefKind::SingleApply);
@@ -8387,7 +8380,7 @@ Expr *TypeChecker::callWitness(Expr *base, DeclContext *dc,
   // Form a reference to the witness itself.
   Type openedFullType, openedType;
   std::tie(openedFullType, openedType)
-    = cs.getTypeOfMemberReference(base->getType(), witness, dc,
+    = cs.getTypeOfMemberReference(base->getType(), witness.getDecl(), dc,
                                   /*isDynamicResult=*/false,
                                   FunctionRefKind::DoubleApply,
                                   dotLocator);
@@ -8400,9 +8393,9 @@ Expr *TypeChecker::callWitness(Expr *base, DeclContext *dc,
   // FIXME: Standardize all callers to always provide all argument names,
   // rather than hack around this.
   CallExpr *call;
-  auto argLabels = witness->getFullName().getArgumentNames();
+  auto argLabels = witness.getDecl()->getFullName().getArgumentNames();
   if (arguments.size() == 1 &&
-      (isVariadicWitness(witness) ||
+      (isVariadicWitness(witnessFn) ||
        argumentNamesMatch(cs.getType(arguments[0]), argLabels))) {
     call = CallExpr::create(Context, unresolvedDot, arguments[0], {}, {},
                             /*hasTrailingClosure=*/false,
@@ -8453,7 +8446,7 @@ Expr *TypeChecker::callWitness(Expr *base, DeclContext *dc,
                         /*suppressDiagnostics=*/false);
 
   auto choice =
-      OverloadChoice(openedFullType, witness, FunctionRefKind::SingleApply);
+      OverloadChoice(openedFullType, witnessFn, FunctionRefKind::SingleApply);
   auto memberRef = rewriter.buildMemberRef(
       base, openedFullType, base->getStartLoc(), choice,
       DeclNameLoc(base->getEndLoc()), openedType, dotLocator, dotLocator,
@@ -8502,45 +8495,48 @@ Solution::convertBooleanTypeToBuiltinI1(Expr *expr,
   // Find the builtin method.
   if (members.size() != 1) {
     tc.diagnose(expr->getLoc(), diag::broken_bool);
-    return nullptr;
+    return expr;
   }
   auto *builtinMethod = dyn_cast<FuncDecl>(members[0].getValueDecl());
   if (!builtinMethod) {
     tc.diagnose(expr->getLoc(), diag::broken_bool);
-    return nullptr;
+    return expr;
   }
 
-  // Form a reference to the builtin method.
-  Expr *memberRef = new (ctx) MemberRefExpr(expr, SourceLoc(),
-                                            builtinMethod,
-                                            DeclNameLoc(expr->getLoc()),
-                                            /*Implicit=*/true);
-  cs.cacheSubExprTypes(memberRef);
-  cs.setSubExprTypes(memberRef);
-  bool failed = tc.typeCheckExpressionShallow(memberRef, cs.DC);
-  cs.cacheExprTypes(memberRef);
-  assert(!failed && "Could not reference witness?");
-  (void)failed;
+  // The method is not generic, so there are no substitutions.
+  auto builtinMethodType = builtinMethod->getInterfaceType()
+    ->castTo<FunctionType>();
+
+  // Form an unbound reference to the builtin method.
+  auto *declRef = new (ctx) DeclRefExpr(builtinMethod,
+                                        DeclNameLoc(expr->getLoc()),
+                                        /*Implicit=*/true);
+  declRef->setFunctionRefKind(FunctionRefKind::DoubleApply);
+  cs.setType(declRef, builtinMethodType);
 
-  // Call the builtin method.
   auto getType = [&](const Expr *E) -> Type {
     return cs.getType(E);
   };
 
-  expr = CallExpr::createImplicit(ctx, memberRef, { }, { }, getType);
-  cs.cacheSubExprTypes(expr);
-  cs.setSubExprTypes(expr);
-  failed = tc.typeCheckExpressionShallow(expr, cs.DC);
-  cs.cacheExprTypes(expr);
-  assert(!failed && "Could not call witness?");
-  (void)failed;
+  // Apply 'self' to get the method value.
+  auto *methodRef = new (ctx) DotSyntaxCallExpr(declRef,
+                                                SourceLoc(),
+                                                expr);
+  cs.setType(methodRef, builtinMethodType->getResult());
+
+  // Apply the empty argument list to get the final result.
+  auto *result = CallExpr::createImplicit(ctx, methodRef,
+                                          { }, { }, getType);
+  cs.setType(result, builtinMethodType->getResult()
+                  ->castTo<FunctionType>()->getResult());
+  cs.setType(result->getArg(), ctx.TheEmptyTupleType);
 
-  if (expr && !cs.getType(expr)->isBuiltinIntegerType(1)) {
+  if (!cs.getType(result)->isBuiltinIntegerType(1)) {
     tc.diagnose(expr->getLoc(), diag::broken_bool);
-    return nullptr;
+    return result;
   }
 
-  return expr;
+  return result;
 }
 
 Expr *Solution::convertOptionalToBool(Expr *expr,

lib/Sema/TypeCheckConstraints.cpp

@@ -2613,7 +2613,7 @@ bool TypeChecker::typeCheckCondition(Expr *&expr, DeclContext *dc) {
 
     // Convert the result to a Builtin.i1.
     Expr *appliedSolution(constraints::Solution &solution,
-                                  Expr *expr) override {
+                          Expr *expr) override {
       auto &cs = solution.getConstraintSystem();
 
       auto converted =

lib/Sema/TypeCheckREPL.cpp

@@ -268,9 +268,9 @@ void REPLChecker::generatePrintOfExpression(StringRef NameStr, Expr *E) {
   CE->setBody(Body, false);
   TC.typeCheckClosureBody(CE);
 
-  Expr *TheCall = CallExpr::createImplicit(Context, CE, { E }, { });
-  if (TC.typeCheckExpressionShallow(TheCall, Arg->getDeclContext()))
-    return ;
+  auto *TheCall = CallExpr::createImplicit(Context, CE, { E }, { });
+  TheCall->getArg()->setType(ParenType::get(Context, E->getType()));
+  TheCall->setType(Context.TheEmptyTupleType);
 
   // Inject the call into the top level stream by wrapping it with a TLCD.
   auto *BS = BraceStmt::create(Context, Loc, ASTNode(TheCall),

test/NameBinding/scope_map_lookup.swift

@@ -1,4 +1,4 @@
-// RUN: %target-typecheck-verify-swift %s -enable-astscope-lookup
+// RUN: %target-typecheck-verify-swift -enable-astscope-lookup
 
 // Name binding in default arguments
 

validation-test/compiler_crashers_2_fixed/0117-rdar33433087.swift

@@ -1,4 +1,4 @@
-// RUN: %target-typecheck-verify-swift %s
+// RUN: %target-typecheck-verify-swift
 
 class C {
   private init() {} // expected-note {{declared here}}

validation-test/compiler_crashers_2_fixed/0118-rdar31529413.swift

@@ -1,4 +1,4 @@
-// RUN: %target-typecheck-verify-swift %s
+// RUN: %target-typecheck-verify-swift
 
 enum E { case A }
 

validation-test/compiler_crashers_2_fixed/0119-rdar33613329.swift

@@ -1,4 +1,4 @@
-// RUN: %target-typecheck-verify-swift %s
+// RUN: %target-typecheck-verify-swift
 
 precedencegroup BindingPrecedence {
   higherThan: DefaultPrecedence

validation-test/compiler_crashers_2_fixed/0146-rdar38309176.swift

@@ -1,4 +1,4 @@
-// RUN: %target-typecheck-verify-swift %s
+// RUN: %target-typecheck-verify-swift
 
 func foo(_ msg: Int) {}
 func foo(_ msg: Double) {}

validation-test/compiler_crashers_2_fixed/0147-rdar38505436.swift

@@ -1,4 +1,4 @@
-// RUN: not %target-typecheck-verify-swift %s
+// RUN: not %target-typecheck-verify-swift
 
 protocol P1 {
   class N1 {

validation-test/compiler_crashers_2_fixed/0148-rdar35773761.swift

@@ -1,4 +1,4 @@
-// RUN: %target-typecheck-verify-swift %s
+// RUN: %target-typecheck-verify-swift
 
 let b: () -> Void = withoutActuallyEscaping({ print("hello crash") }, do: { $0() })
 // expected-error@-1 {{cannot convert value of type '()' to specified type '() -> Void'}}

validation-test/compiler_crashers_2_fixed/0149-rdar34852808.swift

@@ -1,4 +1,4 @@
-// RUN: %target-typecheck-verify-swift %s
+// RUN: %target-typecheck-verify-swift
 
 func foo(optional: Int?) {
   _ = { [value = optional ?? 0]