Sema: Assert that we don't try to do direct-to-storage access of properties in resilient types

Right now, the rule is that any access of a stored property from a
constructor or destructor through 'self' must go directly to storage,
skipping observers or getter/setter overrides.

This means that in particular, constructors of resilient types cannot
be @_transparent, or defined in an extension in another module.

Previously, this was only caught in IRGen when the @_transparent
function was inlined into a function from another module, which made
debugging difficult.

Now, we hit an assert in Sema in this case. Of course it should be a
diagnostic; we'll get there eventually.

We could partially lift the restriction, allowing convenience
initializers to be defined @_transparent or be added in extensions,
by having accesses of stored properties from convenience inits go
through accessors. This would be safe, because at that point, we must
already have performed a self.init() delegation, however it would
potentially break existing code in subtle ways.

Also, this patch marks the RangeIterator and Range types @_fixed_layout,
since they define @_transparent initializers and this tripped the
new assert. Furthermore, note that the @_transparent initializer
must be versioned because it is inlined into another versioned
@_transparent function.

Author: slavapestov

lib/Sema/CSApply.cpp

@@ -257,6 +257,44 @@ static VarDecl *findNamedPropertyWitness(TypeChecker &tc, DeclContext *dc,
   return findNamedWitnessImpl<VarDecl>(tc, dc, type, proto, name, diag);
 }
 
+static bool shouldAccessStorageDirectly(Expr *base, VarDecl *member,
+                                        DeclContext *DC) {
+  // This only matters for stored properties.
+  if (!member->hasStorage())
+    return false;
+
+  // ... referenced from constructors and destructors.
+  auto *AFD = dyn_cast<AbstractFunctionDecl>(DC);
+  if (AFD == nullptr)
+    return false;
+
+  if (!isa<ConstructorDecl>(AFD) && !isa<DestructorDecl>(AFD))
+    return false;
+
+  // ... via a "self.property" reference.
+  auto *DRE = dyn_cast<DeclRefExpr>(base);
+  if (DRE == nullptr)
+    return false;
+
+  if (AFD->getImplicitSelfDecl() != cast<DeclRefExpr>(base)->getDecl())
+    return false;
+
+  // Convenience initializers do not require special handling.
+  // FIXME: This is a language change -- for now, keep the old behavior
+#if 0
+  if (auto *CD = dyn_cast<ConstructorDecl>(AFD))
+    if (!CD->isDesignatedInit())
+      return false;
+#endif
+
+  // Ctor or dtor are for immediate class, not a derived class.
+  if (AFD->getParent()->getDeclaredTypeOfContext()->getCanonicalType() !=
+      member->getDeclContext()->getDeclaredTypeOfContext()->getCanonicalType())
+    return false;
+
+  return true;
+}
+
 /// Return the implicit access kind for a MemberRefExpr with the
 /// specified base and member in the specified DeclContext.
 static AccessSemantics
@@ -265,22 +303,17 @@ getImplicitMemberReferenceAccessSemantics(Expr *base, VarDecl *member,
   // Properties that have storage and accessors are frequently accessed through
   // accessors.  However, in the init and destructor methods for the type
   // immediately containing the property, accesses are done direct.
-  if (auto *AFD_DC = dyn_cast<AbstractFunctionDecl>(DC))
-    if (member->hasStorage() &&
-        // In a ctor or dtor.
-        (isa<ConstructorDecl>(AFD_DC) || isa<DestructorDecl>(AFD_DC)) &&
-
-        // Ctor or dtor are for immediate class, not a derived class.
-        AFD_DC->getParent()->getDeclaredTypeOfContext()->getCanonicalType() ==
-          member->getDeclContext()->getDeclaredTypeOfContext()->getCanonicalType() &&
-
-        // Is a "self.property" reference.
-        isa<DeclRefExpr>(base) &&
-        AFD_DC->getImplicitSelfDecl() == cast<DeclRefExpr>(base)->getDecl()) {
-      // Access this directly instead of going through (e.g.) observing or
-      // trivial accessors.
-      return AccessSemantics::DirectToStorage;
-    }
+  if (shouldAccessStorageDirectly(base, member, DC)) {
+    // The storage better not be resilient.
+    assert(member->hasFixedLayout(DC->getParentModule(),
+                                  DC->getResilienceExpansion()) &&
+           "Designated initializers and destructors of resilient types "
+           "cannot be @_transparent or defined in extensions");
+
+    // Access this directly instead of going through (e.g.) observing or
+    // trivial accessors.
+    return AccessSemantics::DirectToStorage;
+  }
 
   // If the value is always directly accessed from this context, do it.
   return member->getAccessSemanticsFromContext(DC);

stdlib/public/core/Range.swift

@@ -11,6 +11,7 @@
 //===----------------------------------------------------------------------===//
 
 /// An iterator over the elements of `Range<Element>`.
+@_fixed_layout
 public struct RangeIterator<
   Element : ForwardIndex
 > : IteratorProtocol, Sequence {
@@ -72,6 +73,7 @@ public struct RangeIterator<
 ///     }
 ///     print(brackets(Range<Int>(start: -99, end: 100), 0))
 ///     // Prints "0"
+@_fixed_layout
 public struct Range<
   Element : ForwardIndex
 > : Equatable, Collection,
@@ -87,6 +89,7 @@ public struct Range<
 
   /// Construct a range with `startIndex == start` and `endIndex ==
   /// end`.
+  @_versioned
   @_transparent
   internal init(_start: Element, end: Element) {
     self.startIndex = _start