[Sema] Revise #15280 in response to feedback

Instead of passing the base expression of a member access to `getAccessSemanticsFromContext`, we now just pass a bool flag for whether this is a member access on the implicit 'self' declaration.

Author: hamishknight

include/swift/AST/Decl.h

@@ -2354,10 +2354,10 @@ class ValueDecl : public Decl {
   ///
   /// \param DC The declaration context.
   ///
-  /// \param base If querying for a MemberRefExpr, this is its base expression.
-  ///             Otherwise, \c nullptr.
+  /// \param isAccessOnSelf Whether this is a member access on the implicit
+  ///        'self' declaration of the declaration context.
   AccessSemantics getAccessSemanticsFromContext(const DeclContext *DC,
-                                                Expr *base) const;
+                                                bool isAccessOnSelf) const;
 
   /// Print a reference to the given declaration.
   std::string printRef() const;

lib/AST/Decl.cpp

@@ -1297,33 +1297,30 @@ static bool isPolymorphic(const AbstractStorageDecl *storage) {
 /// MemberRefExpr use of this value in the specified context.
 AccessSemantics
 ValueDecl::getAccessSemanticsFromContext(const DeclContext *UseDC,
-                                         Expr *base) const {
+                                         bool isAccessOnSelf) const {
   // If we're inside a @_transparent function, use the most conservative
   // access pattern, since we may be inlined from a different resilience
   // domain.
   ResilienceExpansion expansion = UseDC->getResilienceExpansion();
 
   if (auto *var = dyn_cast<AbstractStorageDecl>(this)) {
-    // Prevent variable mutations from within their own didSet/willSet
-    // specifiers from becoming infinite loops by accessing directly.
-    if (auto *UseFD = dyn_cast<AccessorDecl>(UseDC)) {
-      if (var->hasStorage() && var->hasAccessorFunctions() &&
-          UseFD->getStorage() == var) {
-        // A plain variable access from within its own observer is direct.
-        if (!base)
+    auto isMember = var->getDeclContext()->isTypeContext();
+    if (isAccessOnSelf)
+      assert(isMember && "Access on self, but var isn't a member");
+
+    // Within a variable's own didSet/willSet specifier, access its storage
+    // directly if it's either:
+    // 1) A 'plain variable' (i.e a variable that's not a member).
+    // 2) An access to the member on the implicit 'self' declaration. If it's a
+    //    member access on some other base, we want to call the setter as we
+    //    might be accessing the member on a *different* instance.
+    // This prevents assignments from becoming infinite loops in most cases.
+    if (!isMember || isAccessOnSelf)
+      if (auto *UseFD = dyn_cast<AccessorDecl>(UseDC))
+        if (var->hasStorage() && var->hasAccessorFunctions() &&
+            UseFD->getStorage() == var)
           return AccessSemantics::DirectToStorage;
 
-        // A member access on the implicit 'self' declaration within its own
-        // observer is direct. If the base is some other expression, we want
-        // to call the setter, as we might be accessing the member on a
-        // *different* instance.
-        base = base->getValueProvidingExpr();
-        if (auto baseDRE = dyn_cast<DeclRefExpr>(base))
-          if (baseDRE->getDecl() == UseFD->getImplicitSelfDecl())
-            return AccessSemantics::DirectToStorage;
-      }
-    }
-
     // "StoredWithTrivialAccessors" are generally always accessed indirectly,
     // but if we know that the trivial accessor will always produce the same
     // thing as the getter/setter (i.e., it can't be overridden), then just do a

lib/Sema/CSApply.cpp

@@ -247,8 +247,16 @@ getImplicitMemberReferenceAccessSemantics(Expr *base, VarDecl *member,
     }
   }
 
+  // Check whether this is a member access on 'self'.
+  bool isAccessOnSelf = false;
+  if (auto *baseDRE = dyn_cast<DeclRefExpr>(base->getValueProvidingExpr()))
+    if (auto *AFD = dyn_cast<AbstractFunctionDecl>(DC))
+      if (auto *selfDecl = AFD->getImplicitSelfDecl())
+        if (baseDRE->getDecl() == selfDecl)
+          isAccessOnSelf = true;
+
   // If the value is always directly accessed from this context, do it.
-  return member->getAccessSemanticsFromContext(DC, base);
+  return member->getAccessSemanticsFromContext(DC, isAccessOnSelf);
 }
 
 void ConstraintSystem::propagateLValueAccessKind(Expr *E, AccessKind accessKind,

lib/Sema/TypeCheckExpr.cpp

@@ -516,7 +516,8 @@ bool TypeChecker::requireArrayLiteralIntrinsics(SourceLoc loc) {
 Expr *TypeChecker::buildCheckedRefExpr(VarDecl *value, DeclContext *UseDC,
                                        DeclNameLoc loc, bool Implicit) {
   auto type = getUnopenedTypeOfReference(value, Type(), UseDC);
-  auto semantics = value->getAccessSemanticsFromContext(UseDC, /*base*/nullptr);
+  auto semantics = value->getAccessSemanticsFromContext(UseDC,
+                                                       /*isAccessOnSelf*/false);
   return new (Context) DeclRefExpr(value, loc, Implicit, semantics, type);
 }
 
@@ -527,7 +528,7 @@ Expr *TypeChecker::buildRefExpr(ArrayRef<ValueDecl *> Decls,
 
   if (Decls.size() == 1 && !isa<ProtocolDecl>(Decls[0]->getDeclContext())) {
     auto semantics = Decls[0]->getAccessSemanticsFromContext(UseDC,
-                                                             /*base*/nullptr);
+                                                       /*isAccessOnSelf*/false);
     return new (Context) DeclRefExpr(Decls[0], NameLoc, Implicit, semantics);
   }