Merge pull request #42329 from xedin/builder-var

[BuilderTransform] Replace use of `TypeExpr` with a special $builderSelf variable

Author: xedin

include/swift/AST/KnownIdentifiers.def

@@ -303,6 +303,7 @@ IDENTIFIER(InvocationEncoder)
 IDENTIFIER(whenLocal)
 IDENTIFIER(decodeNextArgument)
 IDENTIFIER(SerializationRequirement)
+IDENTIFIER_WITH_NAME(builderSelf, "$builderSelf")
 
 #undef IDENTIFIER
 #undef IDENTIFIER_

lib/Sema/BuilderTransform.cpp

@@ -73,6 +73,10 @@ class BuilderClosureVisitor
   Identifier buildOptionalId;
   llvm::SmallDenseMap<DeclName, bool> supportedOps;
 
+  /// The variable used as a base for all `build*` operations added
+  /// by this transform.
+  VarDecl *builderVar = nullptr;
+
   SkipUnhandledConstructInResultBuilder::UnhandledNode unhandledNode;
 
   /// Whether an error occurred during application of the builder closure,
@@ -94,32 +98,6 @@ class BuilderClosureVisitor
     if (!cs)
       return nullptr;
 
-    // FIXME: Setting a base on this expression is necessary in order
-    // to get diagnostics if something about this builder call fails,
-    // e.g. if there isn't a matching overload for `buildBlock`.
-    TypeExpr *typeExpr;
-    auto simplifiedTy = cs->simplifyType(builderType);
-    if (!simplifiedTy->hasTypeVariable()) {
-      typeExpr = TypeExpr::createImplicitHack(loc, simplifiedTy, ctx);
-    } else if (auto *decl = simplifiedTy->getAnyGeneric()) {
-      // HACK: If there's not enough information to completely resolve the
-      // builder type, but we have the base available to us, form an *explicit*
-      // TypeExpr pointing at it. We cannot form an implicit base without
-      // a fully-resolved concrete type. Really, whatever we put here has no
-      // bearing on the generated solution because we're going to use this node
-      // to stash the builder type and hand it back to the ambient
-      // constraint system.
-      typeExpr = TypeExpr::createForDecl(DeclNameLoc(loc), decl, dc);
-    } else {
-      // HACK: If there's not enough information in the constraint system,
-      // create a garbage base type to force it to diagnose
-      // this as an ambiguous expression.
-      // FIXME: We can also construct an UnresolvedMemberExpr here instead of
-      // an UnresolvedDotExpr and get a slightly better diagnostic.
-      typeExpr = TypeExpr::createImplicitHack(loc, ErrorType::get(ctx), ctx);
-    }
-    cs->setType(typeExpr, MetatypeType::get(builderType));
-
     SmallVector<Argument, 4> args;
     for (auto i : indices(argExprs)) {
       auto *expr = argExprs[i];
@@ -128,8 +106,11 @@ class BuilderClosureVisitor
       args.emplace_back(labelLoc, label, expr);
     }
 
+    auto *baseExpr = new (ctx) DeclRefExpr({builderVar}, DeclNameLoc(loc),
+                                           /*isImplicit=*/true);
+
     auto memberRef = new (ctx) UnresolvedDotExpr(
-        typeExpr, loc, DeclNameRef(fnName), DeclNameLoc(loc),
+        baseExpr, loc, DeclNameRef(fnName), DeclNameLoc(loc),
         /*implicit=*/true);
     memberRef->setFunctionRefKind(FunctionRefKind::SingleApply);
 
@@ -223,6 +204,16 @@ class BuilderClosureVisitor
       buildOptionalId = ctx.Id_buildOptional;
     else
       buildOptionalId = ctx.Id_buildIf;
+
+    // If we are about to generate constraints, let's establish builder
+    // variable for the base of `build*` calls.
+    if (cs) {
+      builderVar = new (ctx) VarDecl(
+          /*isStatic=*/false, VarDecl::Introducer::Let,
+          /*nameLoc=*/SourceLoc(), ctx.Id_builderSelf, dc);
+      builderVar->setImplicit();
+      cs->setType(builderVar, MetatypeType::get(cs->simplifyType(builderType)));
+    }
   }
 
   /// Apply the builder transform to the given statement.

lib/Sema/CSApply.cpp

@@ -2926,6 +2926,20 @@ namespace {
     Expr *visitDeclRefExpr(DeclRefExpr *expr) {
       auto locator = cs.getConstraintLocator(expr);
 
+      // Check whether this is a reference to `__buildSelf`, and if so,
+      // replace it with a type expression with fully resolved type.
+      if (auto *var = dyn_cast<VarDecl>(expr->getDecl())) {
+        auto &ctx = cs.getASTContext();
+        if (var->getName() == ctx.Id_builderSelf) {
+          assert(expr->isImplicit() && var->isImplicit());
+          auto builderTy =
+            solution.getResolvedType(var)->getMetatypeInstanceType();
+
+          return cs.cacheType(
+            TypeExpr::createImplicitHack(expr->getLoc(), builderTy, ctx));
+        }
+      }
+
       // Find the overload choice used for this declaration reference.
       auto selected = solution.getOverloadChoice(locator);
       return buildDeclRef(selected, expr->getNameLoc(), locator,

lib/Sema/CSGen.cpp

@@ -1410,13 +1410,6 @@ namespace {
         type = CS.getInstanceType(CS.cacheType(E));
         assert(type && "Implicit type expr must have type set!");
         type = CS.replaceInferableTypesWithTypeVars(type, locator);
-      } else if (CS.hasType(E)) {
-        // If there's a type already set into the constraint system, honor it.
-        // FIXME: This supports the result builder transform, which sneakily
-        // stashes a type in the constraint system through a TypeExpr in order
-        // to pass it down to the rest of CSGen. This is a terribly
-        // unprincipled thing to do.
-        return CS.getType(E);
       } else {
         auto *repr = E->getTypeRepr();
         assert(repr && "Explicit node has no type repr!");

test/Constraints/result_builder_one_way.swift

@@ -49,17 +49,17 @@ func tuplify<C: Collection, T>(_ collection: C, @TupleBuilder body: (C.Element)
 }
 
 // CHECK: ---Connected components---
-// CHECK-NEXT:   1: $T10 depends on 0
-// CHECK-NEXT:   0: $T1 $T2 $T3 $T5 $T6 $T7 $T8 $T77 $T78 depends on 3
-// CHECK-NEXT:   3: $T12 $T17 $T28 $T42 $T53 $T54 $T55 $T56 $T57 $T58 $T59 $T60 $T61 $T62 $T63 $T64 $T65 $T66 $T68 $T69 $T70 $T71 $T72 $T73 $T74 $T75 $T76 depends on 2, 4, 5, 7, 10
-// CHECK-NEXT:   10: $T48 $T49 $T50 $T51 $T52 depends on 9
-// CHECK-NEXT:   9: $T43 $T44 $T45 $T46 $T47
-// CHECK-NEXT:   7: $T31 $T35 $T36 $T37 $T38 $T39 $T40 $T41 depends on 6, 8
-// CHECK-NEXT:   8: $T32 $T33 $T34
-// CHECK-NEXT:   6: $T30
-// CHECK-NEXT:   5: $T18 $T19 $T20 $T21 $T22 $T23 $T24 $T25 $T26 $T27
-// CHECK-NEXT:   4: $T15 $T16
-// CHECK-NEXT:   2: $T11
+// CHECK-NEXT: 1: $T10 depends on 0
+// CHECK-NEXT: 0: $T1 $T2 $T3 $T5 $T6 $T7 $T8 $T82 $T83 depends on 3
+// CHECK-NEXT: 3: $T12 $T17 $T28 $T43 $T55 $T57 $T58 $T59 $T60 $T61 $T63 $T64 $T65 $T66 $T67 $T68 $T70 $T71 $T73 $T74 $T75 $T76 $T77 $T78 $T79 $T80 $T81 depends on 2, 4, 5, 7, 10
+// CHECK-NEXT: 10: $T49 $T51 $T52 $T53 $T54 depends on 9
+// CHECK-NEXT: 9: $T44 $T45 $T46 $T47 $T48
+// CHECK-NEXT: 7: $T31 $T35 $T37 $T38 $T39 $T40 $T41 $T42 depends on 6, 8
+// CHECK-NEXT: 8: $T32 $T33 $T34
+// CHECK-NEXT: 6: $T30
+// CHECK-NEXT: 5: $T18 $T19 $T20 $T21 $T22 $T23 $T24 $T25 $T26 $T27
+// CHECK-NEXT: 4: $T15 $T16
+// CHECK-NEXT: 2: $T11
 let names = ["Alice", "Bob", "Charlie"]
 let b = true
 var number = 17