Track and verify how an l-value expression is used in the AST.

I'll use this information in a follow-up commit.

Swift SVN r32292

include/swift/AST/Expr.h

@@ -34,6 +34,7 @@ namespace llvm {
 }
 
 namespace swift {
+  enum class AccessKind : unsigned char;
   class ArchetypeType;
   class ASTContext;
   class AvailabilitySpec;
@@ -118,11 +119,13 @@ class alignas(8) Expr {
     friend class Expr;
     /// The subclass of Expr that this is.
     unsigned Kind : 8;
+    /// How this l-value is used, if it's an l-value.
+    unsigned LValueAccessKind : 2;
     /// Whether the Expr represents something directly written in source or
     /// it was implicitly generated by the type-checker.
     unsigned Implicit : 1;
   };
-  enum { NumExprBits = 10 };
+  enum { NumExprBits = 11 };
   static_assert(NumExprBits <= 32, "fits in an unsigned");
 
   class LiteralExprBitfields {
@@ -327,11 +330,16 @@ protected:
 private:
   /// Ty - This is the type of the expression.
   Type Ty;
+
+  void setLValueAccessKind(AccessKind accessKind) {
+    ExprBits.LValueAccessKind = unsigned(accessKind) + 1;
+  }
  
 protected:
   Expr(ExprKind Kind, bool Implicit, Type Ty = Type()) : Ty(Ty) {
     ExprBits.Kind = unsigned(Kind);
     ExprBits.Implicit = Implicit;
+    ExprBits.LValueAccessKind = 0;
   }
 
 public:
@@ -434,6 +442,25 @@ public:
   void setImplicit(bool Implicit = true) {
     ExprBits.Implicit = Implicit;
   }
+
+  /// getLValueAccessKind - Determines how this l-value expression is used.
+  AccessKind getLValueAccessKind() const {
+    assert(hasLValueAccessKind());
+    return AccessKind(ExprBits.LValueAccessKind - 1);
+  }
+  bool hasLValueAccessKind() const {
+    return ExprBits.LValueAccessKind != 0;
+  }
+
+  /// Set that this l-value expression is used in the given way.
+  ///
+  /// This information is also correctly propagated to any l-value
+  /// sub-expressions from which this l-value is derived.
+  ///
+  /// \param allowOverwrite - true if it's okay if an expression already
+  ///   has an access kind
+  void propagateLValueAccessKind(AccessKind accessKind,
+                                 bool allowOverwrite = false);
   
   /// Determine whether this expression is 'super', possibly converted to
   /// a base class.

include/swift/AST/Types.h

@@ -453,6 +453,11 @@ public:
     return getRecursiveProperties().hasUnboundGeneric();
   }
 
+  /// \brief Check if this type is a valid type for the LHS of an assignment.
+  /// This mainly means isLValueType(), but empty tuples and tuples of empty
+  /// tuples also qualify.
+  bool isAssignableType();
+
   /// isExistentialType - Determines whether this type is an existential type,
   /// whose real (runtime) type is unknown but which is known to conform to
   /// some set of protocols. Protocol and protocol-conformance types are

lib/AST/ASTDumper.cpp

@@ -1359,12 +1359,24 @@ public:
   }
   void printRec(TypeRepr *T);
 
+  static const char *getAccessKindString(AccessKind kind) {
+    switch (kind) {
+    case AccessKind::Read: return "read";
+    case AccessKind::Write: return "write";
+    case AccessKind::ReadWrite: return "readwrite";
+    }
+    llvm_unreachable("bad access kind");
+  }
+
   raw_ostream &printCommon(Expr *E, const char *C) {
     OS.indent(Indent) << '(' << C;
     if (E->isImplicit())
       OS << " implicit";
     OS << " type='" << E->getType() << '\'';
 
+    if (E->hasLValueAccessKind())
+      OS << " accessKind=" << getAccessKindString(E->getLValueAccessKind());
+
     // If we have a source range and an ASTContext, print the source range.
     if (auto Ty = E->getType()) {
       auto &Ctx = Ty->getASTContext();

lib/AST/Expr.cpp

@@ -16,6 +16,7 @@
 
 #include "swift/AST/Expr.h"
 #include "swift/Basic/Unicode.h"
+#include "swift/AST/ASTVisitor.h"
 #include "swift/AST/Decl.h" // FIXME: Bad dependency
 #include "swift/AST/Stmt.h"
 #include "swift/AST/AST.h"
@@ -182,6 +183,148 @@ Expr *Expr::getValueProvidingExpr() {
   return E;
 }
 
+/// Propagate l-value use information to children.
+void Expr::propagateLValueAccessKind(AccessKind accessKind,
+                                     bool allowOverwrite) {
+  /// A visitor class which walks an entire l-value expression.
+  class PropagateAccessKind
+       : public ExprVisitor<PropagateAccessKind, void, AccessKind> {
+#ifndef NDEBUG
+    bool AllowOverwrite;
+#endif
+  public:
+    PropagateAccessKind(bool allowOverwrite)
+#ifndef NDEBUG
+      : AllowOverwrite(allowOverwrite)
+#endif
+    {}
+
+    void visit(Expr *E, AccessKind kind) {
+      assert((AllowOverwrite || !E->hasLValueAccessKind()) &&
+             "l-value access kind has already been set");
+
+      assert(E->getType()->isAssignableType() &&
+             "setting access kind on non-l-value");
+      E->setLValueAccessKind(kind);
+
+      // Propagate this to sub-expressions.
+      ASTVisitor::visit(E, kind);
+    }
+
+#define NON_LVALUE_EXPR(KIND)                                           \
+    void visit##KIND##Expr(KIND##Expr *, AccessKind accessKind) {       \
+      llvm_unreachable("not an l-value");                               \
+    }
+#define LEAF_LVALUE_EXPR(KIND)                                          \
+    void visit##KIND##Expr(KIND##Expr *E, AccessKind accessKind) {}
+#define COMPLETE_PHYSICAL_LVALUE_EXPR(KIND, ACCESSOR)                   \
+    void visit##KIND##Expr(KIND##Expr *E, AccessKind accessKind) {      \
+      visit(E->ACCESSOR, accessKind);                                   \
+    }
+#define PARTIAL_PHYSICAL_LVALUE_EXPR(KIND, ACCESSOR)                    \
+    void visit##KIND##Expr(KIND##Expr *E, AccessKind accessKind) {      \
+      visit(E->ACCESSOR, getPartialAccessKind(accessKind));             \
+    }
+
+    void visitMemberRefExpr(MemberRefExpr *E, AccessKind accessKind) {
+      if (!E->getBase()->getType()->isLValueType()) return;
+      visit(E->getBase(), getBaseAccessKind(E->getMember(), accessKind));
+    }
+    void visitSubscriptExpr(SubscriptExpr *E, AccessKind accessKind) {
+      if (!E->getBase()->getType()->isLValueType()) return;
+      visit(E->getBase(), getBaseAccessKind(E->getDecl(), accessKind));
+    }
+
+    static AccessKind getPartialAccessKind(AccessKind accessKind) {
+      return (accessKind == AccessKind::Read
+                ? accessKind : AccessKind::ReadWrite);
+    }
+
+    static AccessKind getBaseAccessKind(ConcreteDeclRef member,
+                                        AccessKind accessKind) {
+      // We assume writes are partial writes, so the result is always
+      // either Read or ReadWrite.
+      auto memberDecl = cast<AbstractStorageDecl>(member.getDecl());
+
+      // If we're reading and the getter is mutating, or we're writing
+      // and the setter is mutating, this is readwrite.
+      if ((accessKind != AccessKind::Write &&
+           memberDecl->isGetterMutating()) ||
+          (accessKind != AccessKind::Read &&
+           !memberDecl->isSetterNonMutating())) {
+        return AccessKind::ReadWrite;
+      }
+
+      return AccessKind::Read;
+    }
+
+    void visitTupleExpr(TupleExpr *E, AccessKind accessKind) {
+      for (auto elt : E->getElements()) {
+        visit(elt, accessKind);
+      }
+    }
+
+    void visitOpenExistentialExpr(OpenExistentialExpr *E,
+                                  AccessKind accessKind) {
+      bool opaqueValueHadAK = E->getOpaqueValue()->hasLValueAccessKind();
+      AccessKind oldOpaqueValueAK =
+        (opaqueValueHadAK ? E->getOpaqueValue()->getLValueAccessKind()
+                          : AccessKind::Read);
+
+      visit(E->getSubExpr(), accessKind);
+
+      // Propagate the new access kind from the OVE to the original existential
+      // if we just set or changed it on the OVE.
+      if (E->getOpaqueValue()->hasLValueAccessKind()) {
+        auto newOpaqueValueAK = E->getOpaqueValue()->getLValueAccessKind();
+        if (!opaqueValueHadAK || newOpaqueValueAK != oldOpaqueValueAK)
+          visit(E->getExistentialValue(), newOpaqueValueAK);
+      }
+    }
+
+    LEAF_LVALUE_EXPR(DeclRef)
+    LEAF_LVALUE_EXPR(DiscardAssignment)
+    LEAF_LVALUE_EXPR(DynamicLookup)
+    LEAF_LVALUE_EXPR(OpaqueValue)
+
+    COMPLETE_PHYSICAL_LVALUE_EXPR(AnyTry, getSubExpr())
+    PARTIAL_PHYSICAL_LVALUE_EXPR(BindOptional, getSubExpr())
+    COMPLETE_PHYSICAL_LVALUE_EXPR(DotSyntaxBaseIgnored, getRHS());
+    PARTIAL_PHYSICAL_LVALUE_EXPR(ForceValue, getSubExpr())
+    COMPLETE_PHYSICAL_LVALUE_EXPR(Identity, getSubExpr())
+    PARTIAL_PHYSICAL_LVALUE_EXPR(TupleElement, getBase())
+
+    NON_LVALUE_EXPR(Error)
+    NON_LVALUE_EXPR(Literal)
+    NON_LVALUE_EXPR(SuperRef)
+    NON_LVALUE_EXPR(Type)
+    NON_LVALUE_EXPR(OtherConstructorDeclRef)
+    NON_LVALUE_EXPR(Collection)
+    NON_LVALUE_EXPR(CaptureList)
+    NON_LVALUE_EXPR(AbstractClosure)
+    NON_LVALUE_EXPR(InOut)
+    NON_LVALUE_EXPR(DynamicType)
+    NON_LVALUE_EXPR(RebindSelfInConstructor)
+    NON_LVALUE_EXPR(Apply)
+    NON_LVALUE_EXPR(ImplicitConversion)
+    NON_LVALUE_EXPR(ExplicitCast)
+    NON_LVALUE_EXPR(OptionalEvaluation)
+    NON_LVALUE_EXPR(If)
+    NON_LVALUE_EXPR(Assign)
+    NON_LVALUE_EXPR(DefaultValue)
+    NON_LVALUE_EXPR(CodeCompletion)
+
+#define UNCHECKED_EXPR(KIND, BASE) \
+    NON_LVALUE_EXPR(KIND)
+#include "swift/AST/ExprNodes.def"
+
+#undef PHYSICAL_LVALUE_EXPR
+#undef LEAF_LVALUE_EXPR
+#undef NON_LVALUE_EXPR
+  };
+
+  PropagateAccessKind(allowOverwrite).visit(this, accessKind);
+}
 
 /// Enumerate each immediate child expression of this node, invoking the
 /// specific functor on it.  This ignores statements and other non-expression

lib/AST/Type.cpp

@@ -539,6 +539,18 @@ bool TypeBase::isVoid() {
   return isEqual(getASTContext().TheEmptyTupleType);
 }
 
+bool TypeBase::isAssignableType() {
+  if (isLValueType()) return true;
+  if (auto tuple = getAs<TupleType>()) {
+    for (auto eltType : tuple->getElementTypes()) {
+      if (!eltType->isAssignableType())
+        return false;
+    }
+    return true;
+  }
+  return false;
+}
+
 namespace {
 class GetRValueTypeVisitor : public TypeVisitor<GetRValueTypeVisitor, Type> {
 public:

lib/AST/Verifier.cpp

@@ -384,7 +384,30 @@ struct ASTNodeBase {};
     /// @{
     /// These verification functions are run on type checked ASTs if there were
     /// no errors.
-    void verifyChecked(Expr *E) { }
+    void verifyChecked(Expr *E) {
+      // Some imported expressions don't have types, even in checked mode.
+      // TODO: eliminate all these
+      if (!E->getType()) {
+        // The raw value of an imported EnumElementDecl doesn't seem to have
+        // a type for some reason.
+        if (!isa<IntegerLiteralExpr>(E)) {
+          Out << "expression has no type\n";
+          E->print(Out);
+          abort();
+        }
+        return;
+      }
+
+      // Require an access kind to be set on every l-value expression.
+      // Note that the empty tuple type is assignable but usually isn't
+      // an l-value, so we have to be conservative there.
+      if (E->getType()->isLValueType() != E->hasLValueAccessKind() &&
+          !(E->hasLValueAccessKind() && E->getType()->isAssignableType())) {
+        Out << "l-value expression does not have l-value access kind set\n";
+        E->print(Out);
+        abort();
+      }
+    }
     void verifyChecked(Stmt *S) {}
     void verifyChecked(Pattern *P) { }
     void verifyChecked(Decl *D) {}
@@ -1369,6 +1392,7 @@ struct ASTNodeBase {};
         Out << "Unexpected types in IdentityExpr\n";
         abort();
       }
+      checkSameLValueAccessKind(E, E->getSubExpr(), "IdentityExpr");
 
       verifyCheckedBase(E);
     }
@@ -2383,6 +2407,15 @@ struct ASTNodeBase {};
       abort();
     }
 
+    void checkSameLValueAccessKind(Expr *LHS, Expr *RHS, const char *what) {
+      if (LHS->hasLValueAccessKind() != RHS->hasLValueAccessKind() ||
+          (LHS->hasLValueAccessKind() &&
+           LHS->getLValueAccessKind() != RHS->getLValueAccessKind())) {
+        Out << what << " has a mismatched l-value access kind\n";
+        abort();
+      }
+    }
+
     // Verification utilities.
     Type checkMetatypeType(Type type, const char *what) {
       auto metatype = type->getAs<AnyMetatypeType>();

lib/Sema/CSApply.cpp

@@ -643,6 +643,15 @@ namespace {
         }
       }
 
+      // If the opaque value has an l-value access kind, then
+      // the OpenExistentialExpr isn't making a derived l-value, which
+      // means this is our only chance to propagate the l-value access kind
+      // down to the original existential value.  Otherwise, propagateLVAK
+      // will handle this.
+      if (record.OpaqueValue->hasLValueAccessKind())
+        record.ExistentialValue->propagateLValueAccessKind(
+                                    record.OpaqueValue->getLValueAccessKind());
+
       // Form the open-existential expression.
       result = new (tc.Context) OpenExistentialExpr(
                                   record.ExistentialValue,
@@ -2639,6 +2648,11 @@ namespace {
     }
 
     Expr *visitInOutExpr(InOutExpr *expr) {
+      // The default assumption is that inouts are read-write.  It's easier
+      // to do this unconditionally here and then overwrite in the exception
+      // case (when we turn the inout into an UnsafePointer) than to try to
+      // discover that we're in that case right now.
+      expr->getSubExpr()->propagateLValueAccessKind(AccessKind::ReadWrite);
       auto lvTy = expr->getSubExpr()->getType()->castTo<LValueType>();
 
       // The type is simply inout.
@@ -3149,6 +3163,7 @@ namespace {
       auto destTy = cs.computeAssignDestType(expr->getDest(), expr->getLoc());
       if (!destTy)
         return nullptr;
+      expr->getDest()->propagateLValueAccessKind(AccessKind::Write);
 
       // Convert the source to the simplified destination type.
       auto locator =
@@ -4499,6 +4514,7 @@ Expr *ExprRewriter::coerceToType(Expr *expr, Type toType,
 
     case ConversionRestrictionKind::LValueToRValue: {
       // Load from the lvalue.
+      expr->propagateLValueAccessKind(AccessKind::Read);
       expr = new (tc.Context) LoadExpr(expr, fromType->getRValueType());
 
       // Coerce the result.
@@ -4594,6 +4610,16 @@ Expr *ExprRewriter::coerceToType(Expr *expr, Type toType,
     }
 
     case ConversionRestrictionKind::InoutToPointer: {
+      // Overwrite the l-value access kind to be read-only if we're
+      // converting to a non-mutable pointer type.
+      PointerTypeKind pointerKind;
+      auto toEltType = toType->getAnyPointerElementType(pointerKind);
+      assert(toEltType && "not a pointer type?"); (void) toEltType;
+      if (pointerKind == PTK_UnsafePointer) {
+        cast<InOutExpr>(expr->getValueProvidingExpr())->getSubExpr()
+          ->propagateLValueAccessKind(AccessKind::Read, /*overwrite*/ true);
+      }
+
       tc.requirePointerArgumentIntrinsics(expr->getLoc());
       return new (tc.Context) InOutToPointerExpr(expr, toType);
     }
@@ -4694,6 +4720,7 @@ Expr *ExprRewriter::coerceToType(Expr *expr, Type toType,
       // In an 'inout' operator like "++i", the operand is converted from
       // an implicit lvalue to an inout argument.
       assert(toIO->getObjectType()->isEqual(fromLValue->getObjectType()));
+      expr->propagateLValueAccessKind(AccessKind::ReadWrite);
       return new (tc.Context) InOutExpr(expr->getStartLoc(), expr,
                                             toType, /*isImplicit*/true);
     }
@@ -4710,6 +4737,7 @@ Expr *ExprRewriter::coerceToType(Expr *expr, Type toType,
 
     if (performLoad) {
       // Load from the lvalue.
+      expr->propagateLValueAccessKind(AccessKind::Read);
       expr = new (tc.Context) LoadExpr(expr, fromLValue->getObjectType());
 
       // Coerce the result.
@@ -4933,6 +4961,7 @@ ExprRewriter::coerceObjectArgumentToType(Expr *expr,
 
   // Use InOutExpr to convert it to an explicit inout argument for the
   // receiver.
+  expr->propagateLValueAccessKind(AccessKind::ReadWrite);
   return new (ctx) InOutExpr(expr->getStartLoc(), expr,
                                  toType, /*isImplicit*/true);
 }

lib/Sema/TypeCheckConstraints.cpp

@@ -1903,8 +1903,10 @@ Expr *TypeChecker::coerceToRValue(Expr *expr) {
   }
 
   // If we already have an rvalue, we're done, otherwise emit a load.
-  if (auto lvalueTy = expr->getType()->getAs<LValueType>())
+  if (auto lvalueTy = expr->getType()->getAs<LValueType>()) {
+    expr->propagateLValueAccessKind(AccessKind::Read);
     return new (Context) LoadExpr(expr, lvalueTy->getObjectType());
+  }
 
   return expr;
 }
@@ -1915,8 +1917,10 @@ Expr *TypeChecker::coerceToMaterializable(Expr *expr) {
     return expr;
   
   // Load lvalues.
-  if (auto lvalue = expr->getType()->getAs<LValueType>())
+  if (auto lvalue = expr->getType()->getAs<LValueType>()) {
+    expr->propagateLValueAccessKind(AccessKind::Read);
     return new (Context) LoadExpr(expr, lvalue->getObjectType());
+  }
 
   // Walk into parenthesized expressions to update the subexpression.
   if (auto paren = dyn_cast<IdentityExpr>(expr)) {