Implementation for SE-0228: Fix ExpressibleByStringInterpolation (#20214)

* [CodeCompletion] Restrict ancestor search to brace

This change allows ExprParentFinder to restrict certain searches for parents to just AST nodes within the nearest surrounding BraceStmt. In the string interpolation rework, BraceStmts can appear in new places in the AST; this keeps code completion from looking at irrelevant context.

NFC in this commit, but keeps code completion from crashing once TapExpr is introduced.

* Remove test relying on ExpressibleByStringInterpolation being deprecated

Since soon enough, it won’t be anymore.

* [AST] Introduce TapExpr

TapExpr allows a block of code to to be inserted between two expressions, accessing and potentially mutating the result of its subexpression before giving it to its parent expression. It’s roughly equivalent to this function:

  func _tap<T>(_ value: T, do body: (inout T) throws -> Void) rethrows -> T {
    var copy = value
    try body(&copy)
    return copy
  }

Except that it doesn’t use a closure, so no variables are captured and no call frame is (even notionally) added.

This commit does not include tests because nothing in it actually uses TapExpr yet. It will be used by string interpolation.

* SE-0228: Fix ExpressibleByStringInterpolation

This is the bulk of the implementation of the string interpolation rework. It includes a redesigned AST node, new parsing logic, new constraints and post-typechecking code generation, and new standard library types and members.

* [Sema] Rip out typeCheckExpressionShallow()

With new string interpolation in place, it is no longer used by anything in the compiler.

* [Sema] Diagnose invalid StringInterpolationProtocols

StringInterpolationProtocol informally requires conforming types to provide at least one method with the base name “appendInterpolation” with no (or a discardable) return value and visibility at least as broad as the conforming type’s. This change diagnoses an error when a conforming type does not have a method that meets those criteria.

* [Stdlib] Fix map(String.init) source break

Some users, including some in the source compatibility suite, accidentally used init(stringInterpolationSegment:) by writing code like `map(String.init)`. Now that these intializers have been removed, the remaining initializers often end up tying during overload resolution. This change adds several overloads of `String.init(describing:)` which will break these ties in cases where the compiler previously selected `String.init(stringInterpolationSegment:)`.

* [Sema] Make callWitness() take non-mutable arrays

It doesn’t actually need to mutate them.

* [Stdlib] Improve floating-point interpolation performance

This change avoids constructing a String when interpolating a Float, Double, or Float80. Instead, we write the characters to a fixed-size buffer and then append them directly to the string’s storage.

This seems to improve performance for all three types, but especially for Double and Float80, which cannot always fit into a small string when stringified.

* [NameLookup] Improve MemberLookupTable invalidation

In rare cases usually involving generated code, an overload added by an extension in the middle of a file would not be visible below it if the type had lazy members and the same base name had already been referenced above the extension. This change essentially dirties a type’s member lookup table whenever an extension is added to it, ensuring the entries in it will be updated.

This change also includes some debugging improvements for NameLookup.

* [SILOptimizer] XFAIL dead object removal failure

The DeadObjectRemoval pass in SILOptimizer does not currently remove reworked string interpolations as well as the old design because their effects cannot be described by @_effects(readonly). That causes a test failure on Linux. This change temporarily silences that test. The SILOptimizer issue has been filed as SR-9008.

* Confess string interpolation’s source stability sins

* [Parser] Parse empty interpolations

Previously, the parser had an odd asymmetry which caused the same function to accept foo(), but reject “\()”. This change fixes the issue.

Already tested by test/Parse/try.swift, which uses this construct in one of its throwing interpolation tests.

* [Sema] Fix batch-mode-only lazy var bug

The temporary variable used by string interpolation needs to be recontextualized when it’s inserted into a synthesized getter. Fixes a compilation failure in Alamofire.

I’ll probably follow up on this bug a bit more after merging.

Author: beccadax

docs/Literals.rst

@@ -136,8 +136,7 @@ to jump through.
 The default array literal type is always Array, and the default dictionary
 literal type is always Dictionary.
 
-String interpolations are a bit different: they try to individually convert
-each element of the interpolation to the type that adopts
-_ExpressibleByStringInterpolation, then calls the variadic
-``convertFromStringInterpolation`` to put them all together. The default type
+String interpolations are a bit different: they create an instance of 
+``T.StringInterpolation`` and append each segment to it, then initialize
+an instance of ``T`` with that instance. The default type
 for an interpolated literal without context is also ``StringLiteralType``.

include/swift/AST/Decl.h

@@ -3051,16 +3051,26 @@ class NominalTypeDecl : public GenericTypeDecl, public IterableDeclContext {
   /// The table itself is lazily constructed and updated when
   /// lookupDirect() is called. The bit indicates whether the lookup
   /// table has already added members by walking the declarations in
-  /// scope.
+  /// scope; it should be manipulated through \c isLookupTablePopulated()
+  /// and \c setLookupTablePopulated().
   llvm::PointerIntPair<MemberLookupTable *, 1, bool> LookupTable;
 
   /// Prepare the lookup table to make it ready for lookups.
   void prepareLookupTable(bool ignoreNewExtensions);
 
+  /// True if the entries in \c LookupTable are complete--that is, if a
+  /// name is present, it contains all members with that name.
+  bool isLookupTablePopulated() const;
+  void setLookupTablePopulated(bool value);
+
   /// Note that we have added a member into the iterable declaration context,
   /// so that it can also be added to the lookup table (if needed).
   void addedMember(Decl *member);
 
+  /// Note that we have added an extension into the nominal type,
+  /// so that its members can eventually be added to the lookup table.
+  void addedExtension(ExtensionDecl *ext);
+
   /// \brief A lookup table used to find the protocol conformances of
   /// a given nominal type.
   mutable ConformanceLookupTable *ConformanceTable = nullptr;

include/swift/AST/DiagnosticsParse.def

@@ -1221,6 +1221,16 @@ ERROR(expected_type_after_as,none,
 ERROR(string_interpolation_extra,none,
       "extra tokens after interpolated string expression", ())
 
+// Interpolations with parameter labels or multiple values
+WARNING(string_interpolation_list_changing,none,
+      "interpolating multiple values will not form a tuple in Swift 5", ())
+NOTE(string_interpolation_list_insert_parens,none,
+      "insert parentheses to keep current behavior", ())
+WARNING(string_interpolation_label_changing,none,
+      "labeled interpolations will not be ignored in Swift 5", ())
+NOTE(string_interpolation_remove_label,none,
+      "remove %0 label to keep current behavior", (Identifier))
+
 // Keypath expressions.
 ERROR(expr_keypath_expected_lparen,PointsToFirstBadToken,
       "expected '(' following '#keyPath'", ())

include/swift/AST/DiagnosticsSema.def

@@ -1904,6 +1904,22 @@ NOTE(req_near_match_access,none,
      "make %0 %select{ERROR|private|private|non-public|non-public}1 to silence this "
      "warning", (DeclName, AccessLevel))
 
+// appendInterpolation methods
+ERROR(missing_append_interpolation,none,
+      "type conforming to 'StringInterpolationProtocol' does not implement "
+      "a valid 'appendInterpolation' method", ())
+WARNING(append_interpolation_static,none,
+      "'appendInterpolation' method will never be used because it is static",
+      ())
+WARNING(append_interpolation_void_or_discardable,none,
+      "'appendInterpolation' method does not return 'Void' or have a "
+      "discardable result", ())
+WARNING(append_interpolation_access_control,none,
+      "'appendInterpolation' method is %select{private|fileprivate|internal|"
+      "public|open}0, but %1 is %select{private|fileprivate|internal|public|"
+      "open}2",
+      (AccessLevel, DeclName, AccessLevel))
+
 // Protocols and existentials
 ERROR(assoc_type_outside_of_protocol,none,
       "associated type %0 can only be used with a concrete type or "
@@ -2747,10 +2763,10 @@ ERROR(optional_used_as_boolean,none,
      "test for '!= nil' instead", (Type))
 
 ERROR(interpolation_missing_proto,none,
-      "string interpolation requires the protocol '_ExpressibleByStringInterpolation' to be defined",
+      "string interpolation requires the protocol 'ExpressibleByStringInterpolation' to be defined",
       ())
 ERROR(interpolation_broken_proto,none,
-      "protocol '_ExpressibleByStringInterpolation' is broken",
+      "protocol 'ExpressibleByStringInterpolation' is broken",
       ())
 
 ERROR(object_literal_broken_proto,none,
@@ -3362,6 +3378,8 @@ ERROR(throw_in_illegal_context,none,
 
 ERROR(throwing_operator_without_try,none,
       "operator can throw but expression is not marked with 'try'", ())
+ERROR(throwing_interpolation_without_try,none,
+      "interpolation can throw but is not marked with 'try'", ())
 ERROR(throwing_call_without_try,none,
       "call can throw but is not marked with 'try'", ())
 NOTE(note_forgot_try,none,

include/swift/AST/Expr.h

@@ -57,6 +57,7 @@ namespace swift {
   class PatternBindingDecl;
   class ParameterList;
   class EnumElementDecl;
+  class CallExpr;
 
 enum class ExprKind : uint8_t {
 #define EXPR(Id, Parent) Id,
@@ -171,6 +172,12 @@ class alignas(8) Expr {
     IsSingleExtendedGraphemeCluster : 1
   );
 
+  SWIFT_INLINE_BITFIELD_FULL(InterpolatedStringLiteralExpr, LiteralExpr, 32+20,
+    : NumPadBits,
+    InterpolationCount : 20,
+    LiteralCapacity : 32
+  );
+
   SWIFT_INLINE_BITFIELD(DeclRefExpr, Expr, 2+2,
     Semantics : 2, // an AccessSemantics
     FunctionRefKind : 2
@@ -916,6 +923,42 @@ class StringLiteralExpr : public LiteralExpr {
   }
 };
 
+/// \brief Runs a series of statements which use or modify \c SubExpr
+/// before it is given to the rest of the expression.
+///
+/// \c Body should begin with a \c VarDecl; this defines the variable
+/// \c TapExpr will initialize at the beginning and read a result
+/// from at the end. \c TapExpr creates a separate scope, then
+/// assigns the result of \c SubExpr to the variable and runs \c Body
+/// in it, returning the value of the variable after the \c Body runs.
+///
+/// (The design here could be a bit cleaner, particularly where the VarDecl
+/// is concerned.)
+class TapExpr : public Expr {
+  Expr *SubExpr;
+  BraceStmt *Body;
+
+public:
+  TapExpr(Expr *SubExpr, BraceStmt *Body);
+
+  Expr * getSubExpr() const { return SubExpr; }
+  void setSubExpr(Expr * se) { SubExpr = se; }
+
+  /// \brief The variable which will be accessed and possibly modified by
+  /// the \c Body. This is the first \c ASTNode in the \c Body.
+  VarDecl * getVar() const;
+
+  BraceStmt * getBody() const { return Body; }
+  void setBody(BraceStmt * b) { Body = b; }
+
+  SourceLoc getLoc() const { return SourceLoc(); }
+  SourceRange getSourceRange() const { return SourceRange(); }
+
+  static bool classof(const Expr *E) {
+    return E->getKind() == ExprKind::Tap;
+  }
+};
+
 /// InterpolatedStringLiteral - An interpolated string literal.
 ///
 /// An interpolated string literal mixes expressions (which are evaluated and
@@ -927,16 +970,37 @@ class StringLiteralExpr : public LiteralExpr {
 class InterpolatedStringLiteralExpr : public LiteralExpr {
   /// Points at the beginning quote.
   SourceLoc Loc;
-  MutableArrayRef<Expr *> Segments;
+  TapExpr *AppendingExpr;
   Expr *SemanticExpr;
 
 public:
-  InterpolatedStringLiteralExpr(SourceLoc Loc, MutableArrayRef<Expr *> Segments)
-    : LiteralExpr(ExprKind::InterpolatedStringLiteral, /*Implicit=*/false),
-      Loc(Loc), Segments(Segments), SemanticExpr() { }
-  
-  MutableArrayRef<Expr *> getSegments() { return Segments; }
-  ArrayRef<Expr *> getSegments() const { return Segments; }
+  InterpolatedStringLiteralExpr(SourceLoc Loc, unsigned LiteralCapacity, 
+                                unsigned InterpolationCount,
+                                TapExpr *AppendingExpr)
+      : LiteralExpr(ExprKind::InterpolatedStringLiteral, /*Implicit=*/false),
+        Loc(Loc), AppendingExpr(AppendingExpr), SemanticExpr() {
+    Bits.InterpolatedStringLiteralExpr.InterpolationCount = InterpolationCount;
+    Bits.InterpolatedStringLiteralExpr.LiteralCapacity = LiteralCapacity;
+  }
+
+  /// \brief Retrieve the value of the literalCapacity parameter to the
+  /// initializer.
+  unsigned getLiteralCapacity() const {
+    return Bits.InterpolatedStringLiteralExpr.LiteralCapacity;
+  }
+
+  /// \brief Retrieve the value of the interpolationCount parameter to the
+  /// initializer.
+  unsigned getInterpolationCount() const {
+    return Bits.InterpolatedStringLiteralExpr.InterpolationCount;
+  }
+
+  /// \brief A block containing expressions which call
+  /// \c StringInterpolationProtocol methods to append segments to the
+  /// string interpolation. The first node in \c Body should be an uninitialized
+  /// \c VarDecl; the other statements should append to it.
+  TapExpr * getAppendingExpr() const { return AppendingExpr; }
+  void setAppendingExpr(TapExpr * AE) { AppendingExpr = AE; }
 
   /// \brief Retrieve the expression that actually evaluates the resulting
   /// string, typically with a series of '+' operations.
@@ -951,6 +1015,10 @@ class InterpolatedStringLiteralExpr : public LiteralExpr {
     // token, so the range should be (Start == End).
     return Loc;
   }
+
+  /// \brief Call the \c callback with information about each segment in turn.
+  void forEachSegment(ASTContext &Ctx,
+                      llvm::function_ref<void(bool, CallExpr *)> callback);
 
   static bool classof(const Expr *E) {
     return E->getKind() == ExprKind::InterpolatedStringLiteral;

include/swift/AST/ExprNodes.def

@@ -188,7 +188,8 @@ EXPR(EditorPlaceholder, Expr)
 EXPR(ObjCSelector, Expr)
 EXPR(KeyPath, Expr)
 UNCHECKED_EXPR(KeyPathDot, Expr)
-LAST_EXPR(KeyPathDot)
+EXPR(Tap, Expr)
+LAST_EXPR(Tap)
 
 #undef EXPR_RANGE
 #undef LITERAL_EXPR

include/swift/AST/KnownIdentifiers.def

@@ -165,7 +165,12 @@ IDENTIFIER_(builtinUTF16StringLiteral)
 IDENTIFIER_(builtinStringLiteral)
 IDENTIFIER(StringLiteralType)
 IDENTIFIER(stringInterpolation)
-IDENTIFIER(stringInterpolationSegment)
+IDENTIFIER(StringInterpolation)
+IDENTIFIER(literalCapacity)
+IDENTIFIER(interpolationCount)
+IDENTIFIER(appendLiteral)
+IDENTIFIER(appendInterpolation)
+IDENTIFIER_WITH_NAME(dollarInterpolation, "$interpolation")
 IDENTIFIER(arrayLiteral)
 IDENTIFIER(dictionaryLiteral)
 IDENTIFIER_(getBuiltinLogicValue)

include/swift/AST/KnownProtocols.def

@@ -70,13 +70,15 @@ PROTOCOL(Decodable)
 PROTOCOL_(ObjectiveCBridgeable)
 PROTOCOL_(DestructorSafeContainer)
 
+PROTOCOL(StringInterpolationProtocol)
+
 EXPRESSIBLE_BY_LITERAL_PROTOCOL(ExpressibleByArrayLiteral)
 EXPRESSIBLE_BY_LITERAL_PROTOCOL(ExpressibleByBooleanLiteral)
 EXPRESSIBLE_BY_LITERAL_PROTOCOL(ExpressibleByDictionaryLiteral)
 EXPRESSIBLE_BY_LITERAL_PROTOCOL(ExpressibleByExtendedGraphemeClusterLiteral)
 EXPRESSIBLE_BY_LITERAL_PROTOCOL(ExpressibleByFloatLiteral)
 EXPRESSIBLE_BY_LITERAL_PROTOCOL(ExpressibleByIntegerLiteral)
-EXPRESSIBLE_BY_LITERAL_PROTOCOL_(ExpressibleByStringInterpolation)
+EXPRESSIBLE_BY_LITERAL_PROTOCOL(ExpressibleByStringInterpolation)
 EXPRESSIBLE_BY_LITERAL_PROTOCOL(ExpressibleByStringLiteral)
 EXPRESSIBLE_BY_LITERAL_PROTOCOL(ExpressibleByNilLiteral)
 EXPRESSIBLE_BY_LITERAL_PROTOCOL(ExpressibleByUnicodeScalarLiteral)

include/swift/Parse/Parser.h

@@ -1219,8 +1219,11 @@ class Parser {
   StringRef copyAndStripUnderscores(StringRef text);
 
   ParserStatus parseStringSegments(SmallVectorImpl<Lexer::StringSegment> &Segments,
-                                   SmallVectorImpl<Expr*> &Exprs,
-                                   Token EntireTok);
+                                   Token EntireTok,
+                                   VarDecl *InterpolationVar,
+                                   SmallVectorImpl<ASTNode> &Stmts,
+                                   unsigned &LiteralCapacity,
+                                   unsigned &InterpolationCount);
 
   /// Parse an argument label `identifier ':'`, if it exists.
   ///

lib/AST/ASTDumper.cpp

@@ -1839,10 +1839,10 @@ class PrintExpr : public ExprVisitor<PrintExpr> {
   }
   void visitInterpolatedStringLiteralExpr(InterpolatedStringLiteralExpr *E) {
     printCommon(E, "interpolated_string_literal_expr");
-    for (auto Segment : E->getSegments()) {
-      OS << '\n';
-      printRec(Segment);
-    }
+    PrintWithColorRAII(OS, LiteralValueColor) << " literal_capacity=" 
+      << E->getLiteralCapacity() << " interpolation_count="
+      << E->getInterpolationCount() << '\n';
+    printRec(E->getAppendingExpr());
     printSemanticExpr(E->getSemanticExpr());
     PrintWithColorRAII(OS, ParenthesisColor) << ')';
   }
@@ -2644,6 +2644,19 @@ class PrintExpr : public ExprVisitor<PrintExpr> {
     printCommon(E, "key_path_dot_expr");
     PrintWithColorRAII(OS, ParenthesisColor) << ')';
   }
+
+  void visitTapExpr(TapExpr *E) {
+    printCommon(E, "tap_expr");
+    PrintWithColorRAII(OS, DeclColor) << " var=";
+    printDeclRef(E->getVar());
+    OS << '\n';
+
+    printRec(E->getSubExpr());
+    OS << '\n';
+
+    printRec(E->getBody(), E->getVar()->getDeclContext()->getASTContext());
+    PrintWithColorRAII(OS, ParenthesisColor) << ')';
+  }
 };
 
 } // end anonymous namespace

lib/AST/ASTWalker.cpp

@@ -446,9 +446,9 @@ class Traversal : public ASTVisitor<Traversal, Expr*, Stmt*,
   Expr *visitInterpolatedStringLiteralExpr(InterpolatedStringLiteralExpr *E) {
     HANDLE_SEMANTIC_EXPR(E);
 
-    for (auto &Segment : E->getSegments()) {
-      if (Expr *Seg = doIt(Segment))
-        Segment = Seg;
+    if (auto oldAppendingExpr = E->getAppendingExpr()) {
+      if (auto appendingExpr = doIt(oldAppendingExpr))
+        E->setAppendingExpr(dyn_cast<TapExpr>(appendingExpr));
       else
         return nullptr;
     }
@@ -1050,6 +1050,28 @@ class Traversal : public ASTVisitor<Traversal, Expr*, Stmt*,
 
   Expr *visitKeyPathDotExpr(KeyPathDotExpr *E) { return E; }
 
+  Expr *visitTapExpr(TapExpr *E) {
+    if (auto oldSubExpr = E->getSubExpr()) {
+      if (auto subExpr = doIt(oldSubExpr)) {
+        E->setSubExpr(subExpr);
+      }
+      else {
+        return nullptr;
+      }
+    }
+
+    if (auto oldBody = E->getBody()) {
+      if (auto body = doIt(oldBody)) {
+        E->setBody(dyn_cast<BraceStmt>(body));
+      }
+      else {
+        return nullptr;
+      }
+    }
+
+    return E;
+  }
+
   //===--------------------------------------------------------------------===//
   //                           Everything Else
   //===--------------------------------------------------------------------===//

lib/AST/Decl.cpp

@@ -2995,6 +2995,8 @@ void NominalTypeDecl::addExtension(ExtensionDecl *extension) {
   // Add to the end of the list.
   LastExtension->NextExtension.setPointer(extension);
   LastExtension = extension;
+
+  addedExtension(extension);
 }
 
 auto NominalTypeDecl::getStoredProperties(bool skipInaccessible) const