Merge pull request #66513 from xedin/init-accessor-diagnostics

xedin · web-flow · commit 2b8a39724c24 · 2023-06-14T09:57:08.000-07:00
[Sema/SIL] Improve diagnostics related to init accessors
diff --git a/include/swift/AST/DeclContext.h b/include/swift/AST/DeclContext.h
@@ -82,6 +82,7 @@ namespace swift {
   class SerializedDefaultArgumentInitializer;
   class SerializedTopLevelCodeDecl;
   class StructDecl;
+  class AccessorDecl;
 
 namespace serialization {
 using DeclID = llvm::PointerEmbeddedInt<unsigned, 31>;
@@ -457,6 +458,18 @@ class alignas(1 << DeclContextAlignInBits) DeclContext
     return const_cast<DeclContext*>(this)->getInnermostMethodContext();
   }
 
+  /// Returns the innermost accessor context that belongs to a property.
+  ///
+  /// This routine looks through closure, initializer, and local function
+  /// contexts to find the innermost accessor declaration.
+  ///
+  /// \returns the innermost accessor, or null if there is no such context.
+  LLVM_READONLY
+  AccessorDecl *getInnermostPropertyAccessorContext();
+  const AccessorDecl *getInnermostPropertyAccessorContext() const {
+    return const_cast<DeclContext*>(this)->getInnermostPropertyAccessorContext();
+  }
+
   /// Returns the innermost type context.
   ///
   /// This routine looks through closure, initializer, and local function
diff --git a/include/swift/AST/DiagnosticsSema.def b/include/swift/AST/DiagnosticsSema.def
@@ -7364,7 +7364,22 @@ ERROR(init_accessor_accesses_attribute_on_other_declaration,none,
 ERROR(init_accessor_property_both_init_and_accessed,none,
       "property %0 cannot be both initialized and accessed",
       (DeclName))
-
+ERROR(invalid_use_of_self_in_init_accessor,none,
+      "'self' within init accessors can only be used to reference "
+      "properties listed in 'initializes' and 'accesses'; "
+      "init accessors are run before 'self' is fully available", ())
+ERROR(init_accessor_invalid_member_ref,none,
+      "cannot reference instance member %0; init accessors can only "
+      "refer to instance properties listed in 'initializes' and "
+      "'accesses' attributes",
+      (DeclNameRef))
+ERROR(cannot_synthesize_memberwise_due_to_property_init_order,none,
+      "cannot synthesize memberwise initializer",
+      ())
+NOTE(out_of_order_access_in_init_accessor,none,
+     "init accessor for %0 cannot access stored property %1 because it "
+     "is called before %1 is initialized",
+     (Identifier, Identifier))
 
 #define UNDEFINE_DIAGNOSTIC_MACROS
 #include "DefineDiagnosticMacros.h"
diff --git a/include/swift/Sema/CSFix.h b/include/swift/Sema/CSFix.h
@@ -447,6 +447,10 @@ enum class FixKind : uint8_t {
 
   /// Ignore missing 'each' keyword before value pack reference.
   IgnoreMissingEachKeyword,
+
+  /// Ignore the fact that member couldn't be referenced within init accessor
+  /// because its name doesn't appear in 'initializes' or 'accesses' attributes.
+  AllowInvalidMemberReferenceInInitAccessor,
 };
 
 class ConstraintFix {
@@ -3536,6 +3540,39 @@ class IgnoreMissingEachKeyword final : public ConstraintFix {
   }
 };
 
+class AllowInvalidMemberReferenceInInitAccessor final : public ConstraintFix {
+  DeclNameRef MemberName;
+
+  AllowInvalidMemberReferenceInInitAccessor(ConstraintSystem &cs,
+                                            DeclNameRef memberName,
+                                            ConstraintLocator *locator)
+      : ConstraintFix(cs, FixKind::AllowInvalidMemberReferenceInInitAccessor,
+                      locator),
+        MemberName(memberName) {}
+
+public:
+  std::string getName() const override {
+    llvm::SmallVector<char, 16> scratch;
+    auto memberName = MemberName.getString(scratch);
+    return "allow reference to member '" + memberName.str() +
+           "' in init accessor";
+  }
+
+  bool diagnose(const Solution &solution, bool asNote = false) const override;
+
+  bool diagnoseForAmbiguity(CommonFixesArray commonFixes) const override {
+    return diagnose(*commonFixes.front().first);
+  }
+
+  static AllowInvalidMemberReferenceInInitAccessor *
+  create(ConstraintSystem &cs, DeclNameRef memberName,
+         ConstraintLocator *locator);
+
+  static bool classof(const ConstraintFix *fix) {
+    return fix->getKind() == FixKind::AllowInvalidMemberReferenceInInitAccessor;
+  }
+};
+
 } // end namespace constraints
 } // end namespace swift
 
diff --git a/include/swift/Sema/ConstraintSystem.h b/include/swift/Sema/ConstraintSystem.h
@@ -1879,6 +1879,11 @@ struct MemberLookupResult {
     /// This is a static member being access through a protocol metatype
     /// but its result type doesn't conform to this protocol.
     UR_InvalidStaticMemberOnProtocolMetatype,
+
+    /// This is a member that doesn't appear in 'initializes' and/or
+    /// 'accesses' attributes of the init accessor and therefore canno
+    /// t be referenced in its body.
+    UR_UnavailableWithinInitAccessor,
   };
 
   /// This is a list of considered (but rejected) candidates, along with a
diff --git a/lib/AST/Decl.cpp b/lib/AST/Decl.cpp
@@ -7168,6 +7168,12 @@ bool VarDecl::isMemberwiseInitialized(bool preferDeclaredProperties) const {
       isBackingStorageForDeclaredProperty(this))
     return false;
 
+  // If this is a computed property with `init` accessor, it's
+  // memberwise initializable when it could be used to initialize
+  // other stored properties.
+  if (auto *init = getAccessor(AccessorKind::Init))
+    return init->getAttrs().hasAttribute<InitializesAttr>();
+
   // If this is a computed property, it's not memberwise initialized unless
   // the caller has asked for the declared properties and it is either a
   // `lazy` property or a property with an attached wrapper.
diff --git a/lib/AST/DeclContext.cpp b/lib/AST/DeclContext.cpp
@@ -211,6 +211,24 @@ AbstractFunctionDecl *DeclContext::getInnermostMethodContext() {
   return nullptr;
 }
 
+AccessorDecl *DeclContext::getInnermostPropertyAccessorContext() {
+  auto dc = this;
+  do {
+    if (auto decl = dc->getAsDecl()) {
+      auto accessor = dyn_cast<AccessorDecl>(decl);
+      // If we found a non-accessor decl, we're done.
+      if (accessor == nullptr)
+        return nullptr;
+
+      auto *storage = accessor->getStorage();
+      if (isa<VarDecl>(storage) && storage->getDeclContext()->isTypeContext())
+        return accessor;
+    }
+  } while ((dc = dc->getParent()));
+
+  return nullptr;
+}
+
 bool DeclContext::isTypeContext() const {
   if (auto decl = getAsDecl())
     return isa<NominalTypeDecl>(decl) || isa<ExtensionDecl>(decl);
diff --git a/lib/SILGen/SILGenConstructor.cpp b/lib/SILGen/SILGenConstructor.cpp
@@ -272,12 +272,13 @@ static RValue maybeEmitPropertyWrapperInitFromValue(
                                                           subs, std::move(arg));
 }
 
-static void emitApplyOfInitAccessor(SILGenFunction &SGF, SILLocation loc,
-                                    AccessorDecl *accessor, SILValue selfValue,
-                                    SILType selfTy, RValue &&initialValue) {
+static void
+emitApplyOfInitAccessor(SILGenFunction &SGF, SILLocation loc,
+                        AccessorDecl *accessor, SILValue selfValue,
+                        SILType selfTy, RValue &&initialValue) {
   SmallVector<SILValue> arguments;
 
-  auto emitFieldReference = [&](VarDecl *field) {
+  auto emitFieldReference = [&](VarDecl *field, bool forInit = false) {
     auto fieldTy =
         selfTy.getFieldType(field, SGF.SGM.M, SGF.getTypeExpansionContext());
     return SGF.B.createStructElementAddr(loc, selfValue, field,
@@ -287,7 +288,7 @@ static void emitApplyOfInitAccessor(SILGenFunction &SGF, SILLocation loc,
   // First, let's emit all of the indirect results.
   if (auto *initAttr = accessor->getAttrs().getAttribute<InitializesAttr>()) {
     for (auto *property : initAttr->getPropertyDecls(accessor)) {
-      arguments.push_back(emitFieldReference(property));
+      arguments.push_back(emitFieldReference(property, /*forInit=*/true));
     }
   }
 
@@ -399,29 +400,39 @@ static void emitImplicitValueConstructor(SILGenFunction &SGF,
 
   // If we have an indirect return slot, initialize it in-place.
   if (resultSlot) {
-    // Tracks all the init accessors we have emitted
-    // because they can initialize more than one property.
-    llvm::SmallPtrSet<AccessorDecl *, 2> emittedInitAccessors;
-
     auto elti = elements.begin(), eltEnd = elements.end();
-    for (VarDecl *field : decl->getStoredProperties()) {
+
+    llvm::SmallPtrSet<VarDecl *, 4> storedProperties;
+    {
+      auto properties = decl->getStoredProperties();
+      storedProperties.insert(properties.begin(), properties.end());
+    }
+
+    for (auto *member : decl->getMembers()) {
+      auto *field = dyn_cast<VarDecl>(member);
+      if (!field)
+        continue;
+
+      if (initializedViaAccessor.count(field))
+        continue;
 
       // Handle situations where this stored propery is initialized
       // via a call to an init accessor on some other property.
-      if (initializedViaAccessor.count(field)) {
-        auto *initProperty = initializedViaAccessor.find(field)->second;
-        auto *initAccessor = initProperty->getAccessor(AccessorKind::Init);
-
-        if (emittedInitAccessors.count(initAccessor))
+      if (auto *initAccessor = field->getAccessor(AccessorKind::Init)) {
+        if (field->isMemberwiseInitialized(/*preferDeclaredProperties=*/true)) {
+          assert(elti != eltEnd &&
+                 "number of args does not match number of fields");
+
+          emitApplyOfInitAccessor(SGF, Loc, initAccessor, resultSlot, selfTy,
+                                  std::move(*elti));
+          ++elti;
           continue;
+        }
+      }
 
-        emitApplyOfInitAccessor(SGF, Loc, initAccessor, resultSlot, selfTy,
-                                std::move(*elti));
-
-        emittedInitAccessors.insert(initAccessor);
-        ++elti;
+      // If this is not one of the stored properties, let's move on.
+      if (!storedProperties.count(field))
         continue;
-      }
 
       auto fieldTy =
           selfTy.getFieldType(field, SGF.SGM.M, SGF.getTypeExpansionContext());
diff --git a/lib/SILOptimizer/Mandatory/DefiniteInitialization.cpp b/lib/SILOptimizer/Mandatory/DefiniteInitialization.cpp
@@ -1154,14 +1154,38 @@ void LifetimeChecker::doIt() {
 
   // All of the indirect results marked as "out" have to be fully initialized
   // before their lifetime ends.
-  if (TheMemory.isOut() && Uses.empty()) {
-    auto loc = TheMemory.getLoc();
-
-    std::string propertyName;
-    auto *property = TheMemory.getPathStringToElement(0, propertyName);
-    diagnose(Module, F.getLocation(),
-             diag::ivar_not_initialized_by_init_accessor, property->getName());
-    EmittedErrorLocs.push_back(loc);
+  if (TheMemory.isOut()) {
+    auto diagnoseMissingInit = [&]() {
+      std::string propertyName;
+      auto *property = TheMemory.getPathStringToElement(0, propertyName);
+      diagnose(Module, F.getLocation(),
+               diag::ivar_not_initialized_by_init_accessor,
+               property->getName());
+      EmittedErrorLocs.push_back(TheMemory.getLoc());
+    };
+
+    // No uses means that there was no initialization.
+    if (Uses.empty()) {
+      diagnoseMissingInit();
+      return;
+    }
+
+    // Go over every return block and check whether member is fully initialized
+    // because it's possible that there is branch that doesn't have any use of
+    // the memory and nothing else is going to diagnose that. This is different
+    // from `self`, for example, because it would always have either `copy_addr`
+    // or `load` before return.
+
+    auto returnBB = F.findReturnBB();
+
+    while (returnBB != F.end()) {
+      auto *terminator = returnBB->getTerminator();
+
+      if (!isInitializedAtUse(DIMemoryUse(terminator, DIUseKind::Load, 0, 1)))
+        diagnoseMissingInit();
+
+      ++returnBB;
+    }
   }
 
   // If the memory object has nontrivial type, then any destroy/release of the
diff --git a/lib/Sema/CSDiagnostics.cpp b/lib/Sema/CSDiagnostics.cpp
@@ -9059,3 +9059,8 @@ bool MissingEachForValuePackReference::diagnoseAsError() {
 
   return true;
 }
+
+bool InvalidMemberReferenceWithinInitAccessor::diagnoseAsError() {
+  emitDiagnostic(diag::init_accessor_invalid_member_ref, MemberName);
+  return true;
+}
diff --git a/lib/Sema/CSDiagnostics.h b/lib/Sema/CSDiagnostics.h
@@ -3015,6 +3015,19 @@ class MissingEachForValuePackReference final : public FailureDiagnostic {
   bool diagnoseAsError() override;
 };
 
+class InvalidMemberReferenceWithinInitAccessor final
+    : public FailureDiagnostic {
+  DeclNameRef MemberName;
+
+public:
+  InvalidMemberReferenceWithinInitAccessor(const Solution &solution,
+                                           DeclNameRef memberName,
+                                           ConstraintLocator *locator)
+      : FailureDiagnostic(solution, locator), MemberName(memberName) {}
+
+  bool diagnoseAsError() override;
+};
+
 } // end namespace constraints
 } // end namespace swift
 
diff --git a/lib/Sema/CSFix.cpp b/lib/Sema/CSFix.cpp
@@ -2796,3 +2796,18 @@ IgnoreMissingEachKeyword::create(ConstraintSystem &cs, Type valuePackTy,
   return new (cs.getAllocator())
       IgnoreMissingEachKeyword(cs, valuePackTy, locator);
 }
+
+bool AllowInvalidMemberReferenceInInitAccessor::diagnose(
+    const Solution &solution, bool asNote) const {
+  InvalidMemberReferenceWithinInitAccessor failure(solution, MemberName,
+                                                   getLocator());
+  return failure.diagnose(asNote);
+}
+
+AllowInvalidMemberReferenceInInitAccessor *
+AllowInvalidMemberReferenceInInitAccessor::create(ConstraintSystem &cs,
+                                                  DeclNameRef memberName,
+                                                  ConstraintLocator *locator) {
+  return new (cs.getAllocator())
+      AllowInvalidMemberReferenceInInitAccessor(cs, memberName, locator);
+}
diff --git a/lib/Sema/CSSimplify.cpp b/lib/Sema/CSSimplify.cpp
@@ -9635,6 +9635,44 @@ performMemberLookup(ConstraintKind constraintKind, DeclNameRef memberName,
         }
       }
 
+      if (auto *UDE =
+              getAsExpr<UnresolvedDotExpr>(memberLocator->getAnchor())) {
+        auto *base = UDE->getBase();
+        if (auto *accessor = DC->getInnermostPropertyAccessorContext()) {
+          if (accessor->isInitAccessor() && isa<DeclRefExpr>(base) &&
+              accessor->getImplicitSelfDecl() ==
+                  cast<DeclRefExpr>(base)->getDecl()) {
+            bool isValidReference = false;
+
+            // If name doesn't appear in either `initializes` or `accesses`
+            // then it's invalid instance member.
+
+            if (auto *initializesAttr =
+                    accessor->getAttrs().getAttribute<InitializesAttr>()) {
+              isValidReference |= llvm::any_of(
+                  initializesAttr->getProperties(), [&](Identifier name) {
+                    return DeclNameRef(name) == memberName;
+                  });
+            }
+
+            if (auto *accessesAttr =
+                    accessor->getAttrs().getAttribute<AccessesAttr>()) {
+              isValidReference |= llvm::any_of(
+                  accessesAttr->getProperties(), [&](Identifier name) {
+                    return DeclNameRef(name) == memberName;
+                  });
+            }
+
+            if (!isValidReference) {
+              result.addUnviable(
+                  candidate,
+                  MemberLookupResult::UR_UnavailableWithinInitAccessor);
+              return;
+            }
+          }
+        }
+      }
+
     // If the underlying type of a typealias is fully concrete, it is legal
     // to access the type with a protocol metatype base.
     } else if (instanceTy->isExistentialType() &&
@@ -10215,6 +10253,10 @@ fixMemberRef(ConstraintSystem &cs, Type baseTy,
 
     case MemberLookupResult::UR_InvalidStaticMemberOnProtocolMetatype:
       return AllowInvalidStaticMemberRefOnProtocolMetatype::create(cs, locator);
+
+    case MemberLookupResult::UR_UnavailableWithinInitAccessor:
+      return AllowInvalidMemberReferenceInInitAccessor::create(cs, memberName,
+                                                               locator);
     }
   }
 
@@ -14640,6 +14682,10 @@ ConstraintSystem::SolutionKind ConstraintSystem::simplifyFixConstraint(
     return recordFix(fix, 100) ? SolutionKind::Error : SolutionKind::Solved;
   }
 
+  case FixKind::AllowInvalidMemberReferenceInInitAccessor: {
+    return recordFix(fix, 5) ? SolutionKind::Error : SolutionKind::Solved;
+  }
+
   case FixKind::ExplicitlyConstructRawRepresentable: {
     // Let's increase impact of this fix for binary operators because
     // it's possible to get both `.rawValue` and construction fixes for
diff --git a/lib/Sema/CodeSynthesis.cpp b/lib/Sema/CodeSynthesis.cpp
diff --git a/lib/Sema/PreCheckExpr.cpp b/lib/Sema/PreCheckExpr.cpp
diff --git a/test/Interpreter/init_accessors.swift b/test/Interpreter/init_accessors.swift
diff --git a/test/SILOptimizer/init_accessor_definite_init_diagnostics.swift b/test/SILOptimizer/init_accessor_definite_init_diagnostics.swift
diff --git a/test/decl/var/init_accessors.swift b/test/decl/var/init_accessors.swift

Original file line number	Diff line number	Diff line change
`@@ -9059,3 +9059,8 @@ bool MissingEachForValuePackReference::diagnoseAsError() {`
`9059`	`9059`
`9060`	`9060`	`return true;`
`9061`	`9061`	`}`
	`9062`	`+`
	`9063`	`+bool InvalidMemberReferenceWithinInitAccessor::diagnoseAsError() {`
	`9064`	`+ emitDiagnostic(diag::init_accessor_invalid_member_ref, MemberName);`
	`9065`	`+ return true;`
	`9066`	`+}`