Clean up l-value emission in SILGen.

There were several bits of code which were unnecessarily
repeating the core logic of breaking down an access strategy
and either setting up an LValue or directly emitting it.
These places have now been unified to just create and then
load or othrwise use an LValue.

Introduce a visitor which handles the common parts of breaking
down an access strategy and computing information like the
LValueTypeData.  In addition to its direct benefits (which are
somewhat lost in the boilerplate of capturing local state into
the visitor subclass), this eliminates some of the ad-hocness
of how the various emission paths use AccessStrategy.

Finally, implement the MaterializeToTemporary strategy in its
full generality by using the actual read and write sub-strategies
instead of always falling back on calling the getter and setter.
This part is not NFC because it causes us to perform the read
part of a read/write to a stored-with-observers property by
directly accessing the storage instead of calling the getter.

Author: rjmccall

lib/SILGen/LValue.h

@@ -105,6 +105,7 @@ class PathComponent {
 
     // Logical LValue kinds
     GetterSetterKind,           // property or subscript getter/setter
+    MaterializeToTemporaryKind,
     OwnershipKind,              // weak pointer remapping
     AutoreleasingWritebackKind, // autorelease pointer on set
     WritebackPseudoKind,        // a fake component to customize writeback
@@ -261,13 +262,15 @@ class LogicalPathComponent : public PathComponent {
   virtual RValue get(SILGenFunction &SGF, SILLocation loc,
                      ManagedValue base, SGFContext c) && = 0;
 
-  /// Compare 'this' lvalue and the 'rhs' lvalue (which is guaranteed to have
-  /// the same dynamic PathComponent type as the receiver) to see if they are
-  /// identical.  If so, there is a conflicting writeback happening, so emit a
-  /// diagnostic.
-  virtual void diagnoseWritebackConflict(LogicalPathComponent *rhs,
-                                         SILLocation loc1, SILLocation loc2,
-                                         SILGenFunction &SGF) = 0;
+  struct AccessedStorage {
+    AbstractStorageDecl *Storage;
+    bool IsSuper;
+    const RValue *Indices;
+    Expr *IndexExprForDiagnostics;
+  };
+
+  /// Get the storage accessed by this component.
+  virtual Optional<AccessedStorage> getAccessedStorage() const = 0;
 
 
   /// Materialize the storage into memory.  If the access is for
@@ -313,10 +316,8 @@ class TranslationPathComponent : public LogicalPathComponent {
     return kind;
   }
 
-  void diagnoseWritebackConflict(LogicalPathComponent *RHS,
-                                 SILLocation loc1, SILLocation loc2,
-                                 SILGenFunction &SGF) override {
-    // no useful writeback diagnostics at this point
+  Optional<AccessedStorage> getAccessedStorage() const override {
+    return None;
   }
 
   RValue get(SILGenFunction &SGF, SILLocation loc,
@@ -428,25 +429,28 @@ class LValue {
     Path.emplace_back(new T(std::forward<As>(args)...));
   }
 
+  void addNonMemberVarComponent(SILGenFunction &SGF, SILLocation loc,
+                                VarDecl *var, Optional<SubstitutionMap> subs,
+                                LValueOptions options,
+                                AccessStrategy strategy,
+                                CanType formalRValueType);
+
   /// Add a member component to the access path of this lvalue.
   void addMemberComponent(SILGenFunction &SGF, SILLocation loc,
                           AbstractStorageDecl *storage,
                           SubstitutionMap subs,
                           LValueOptions options,
                           bool isSuper,
-                          AccessKind accessKind,
-                          AccessSemantics accessSemantics,
                           AccessStrategy accessStrategy,
                           CanType formalRValueType,
-                          RValue &&indices);
+                          RValue &&indices,
+                          Expr *indexExprForDiagnostics);
 
   void addMemberVarComponent(SILGenFunction &SGF, SILLocation loc,
                              VarDecl *var,
                              SubstitutionMap subs,
                              LValueOptions options,
                              bool isSuper,
-                             AccessKind accessKind,
-                             AccessSemantics accessSemantics,
                              AccessStrategy accessStrategy,
                              CanType formalRValueType);
 
@@ -455,8 +459,6 @@ class LValue {
                                    SubstitutionMap subs,
                                    LValueOptions options,
                                    bool isSuper,
-                                   AccessKind accessKind,
-                                   AccessSemantics accessSemantics,
                                    AccessStrategy accessStrategy,
                                    CanType formalRValueType,
                                    RValue &&indices,

lib/SILGen/SILGen.h

@@ -334,8 +334,8 @@ class LLVM_LIBRARY_VISIBILITY SILGenModule : public ASTVisitor<SILGenModule> {
 
   SILDeclRef getGetterDeclRef(AbstractStorageDecl *decl);
   SILDeclRef getSetterDeclRef(AbstractStorageDecl *decl);
-  SILDeclRef getAddressorDeclRef(AbstractStorageDecl *decl,
-                                 AccessKind accessKind);
+  SILDeclRef getAddressorDeclRef(AbstractStorageDecl *decl);
+  SILDeclRef getMutableAddressorDeclRef(AbstractStorageDecl *decl);
   SILDeclRef getMaterializeForSetDeclRef(AbstractStorageDecl *decl);
 
   KeyPathPatternComponent

lib/SILGen/SILGenApply.cpp

@@ -5364,9 +5364,14 @@ emitMaterializeForSetAccessor(SILLocation loc, SILDeclRef materializeForSet,
                             optionalCallback, callbackStorage);
 }
 
-SILDeclRef SILGenModule::getAddressorDeclRef(AbstractStorageDecl *storage,
-                                             AccessKind accessKind) {
-  FuncDecl *addressorFunc = storage->getAddressorForAccess(accessKind);
+SILDeclRef SILGenModule::getAddressorDeclRef(AbstractStorageDecl *storage) {
+  FuncDecl *addressorFunc = storage->getAddressor();
+  return SILDeclRef(addressorFunc, SILDeclRef::Kind::Func);
+}
+
+SILDeclRef SILGenModule::getMutableAddressorDeclRef(
+                                                 AbstractStorageDecl *storage) {
+  FuncDecl *addressorFunc = storage->getMutableAddressor();
   return SILDeclRef(addressorFunc, SILDeclRef::Kind::Func);
 }
 

lib/SILGen/SILGenExpr.cpp

@@ -955,7 +955,7 @@ static SILDeclRef getRValueAccessorDeclRef(SILGenFunction &SGF,
     if (accessor == AccessorKind::Get) {
       return SGF.SGM.getGetterDeclRef(storage);
     } else {
-      return SGF.SGM.getAddressorDeclRef(storage, AccessKind::Read);  
+      return SGF.SGM.getAddressorDeclRef(storage);
     }
   }
 
@@ -3188,17 +3188,9 @@ static SILFunction *getOrCreateKeyPathSetter(SILGenModule &SGM,
   auto strategy = property->getAccessStrategy(semantics, AccessKind::Write);
 
   LValueOptions lvOptions;
-  if (auto var = dyn_cast<VarDecl>(property)) {
-    lv.addMemberVarComponent(subSGF, loc, var, subs, lvOptions,
-                             /*super*/ false, AccessKind::Write,
-                             semantics, strategy, propertyType);
-  } else {
-    auto sub = cast<SubscriptDecl>(property);
-    lv.addMemberSubscriptComponent(subSGF, loc, sub, subs, lvOptions,
-                                   /*super*/ false, AccessKind::Write,
-                                   semantics, strategy, propertyType,
-                                   std::move(indexValue));
-  }
+  lv.addMemberComponent(subSGF, loc, property, subs, lvOptions,
+                        /*super*/ false, strategy, propertyType,
+                        std::move(indexValue), /*index for diags*/ nullptr);
 
   subSGF.emitAssignToLValue(loc,
     RValue(subSGF, loc, propertyType, valueSubst),
@@ -5349,14 +5341,8 @@ class AutoreleasingWritebackComponent : public LogicalPathComponent {
     return RValue(SGF, ManagedValue::forUnmanaged(unowned), refType);
   }
 
-  /// Compare 'this' lvalue and the 'rhs' lvalue (which is guaranteed to have
-  /// the same dynamic PathComponent type as the receiver) to see if they are
-  /// identical.  If so, there is a conflicting writeback happening, so emit a
-  /// diagnostic.
-  void diagnoseWritebackConflict(LogicalPathComponent *RHS,
-                                 SILLocation loc1, SILLocation loc2,
-                                 SILGenFunction &SGF) override {
-    //      auto &rhs = (GetterSetterComponent&)*RHS;
+  Optional<AccessedStorage> getAccessedStorage() const override {
+    return None;
   }
 
   void dump(raw_ostream &OS, unsigned indent) const override {

lib/SILGen/SILGenFunction.h

@@ -1065,11 +1065,7 @@ class LLVM_LIBRARY_VISIBILITY SILGenFunction
                                          AccessKind accessKind);
 
   // FIXME: demote this to private state.
-  ManagedValue maybeEmitAddressOfNonMemberVarDecl(SILLocation loc,
-                                                  VarDecl *var,
-                                                  CanType formalRValueType,
-                                                  AccessKind accessKind,
-                                                  AccessSemantics semantics);
+  ManagedValue maybeEmitValueOfLocalVarDecl(VarDecl *var);
 
   /// Produce an RValue for a reference to the specified declaration,
   /// with the given type and in response to the specified expression.  Try to