Emit fix-its for Return from initializer missing variables

lib/SILOptimizer/Mandatory/DefiniteInitialization.cpp

@@ -17,11 +17,11 @@
 #include "swift/AST/DiagnosticsSIL.h"
 #include "swift/AST/DistributedDecl.h"
 #include "swift/AST/Expr.h"
+#include "swift/AST/ParameterList.h"
+#include "swift/AST/SemanticAttrs.h"
 #include "swift/AST/Stmt.h"
-#include "swift/Basic/Assertions.h"
 #include "swift/ClangImporter/ClangModule.h"
 #include "swift/SIL/BasicBlockBits.h"
-#include "swift/AST/SemanticAttrs.h"
 #include "swift/SIL/BasicBlockData.h"
 #include "swift/SIL/InstructionUtils.h"
 #include "swift/SIL/MemAccessUtils.h"
@@ -30,7 +30,6 @@
 #include "swift/SIL/SILValue.h"
 #include "swift/SILOptimizer/PassManager/Passes.h"
 #include "swift/SILOptimizer/PassManager/Transforms.h"
-#include "swift/SILOptimizer/Utils/CFGOptUtils.h"
 #include "swift/SILOptimizer/Utils/DistributedActor.h"
 #include "swift/SILOptimizer/Utils/InstOptUtils.h"
 #include "llvm/ADT/STLExtras.h"
@@ -40,6 +39,7 @@
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/Compiler.h"
 #include "llvm/Support/Debug.h"
+#include <unordered_map>
 
 using namespace swift;
 using namespace ownership;
@@ -535,6 +535,8 @@ namespace {
     bool shouldEmitError(const SILInstruction *Inst);
     std::string getUninitElementName(const DIMemoryUse &Use);
     void noteUninitializedMembers(const DIMemoryUse &Use);
+    void emitUninitializedMembersFixit(const SILInstruction *Inst,
+                                       SILLocation loc, const DIMemoryUse &Use);
     void diagnoseInitError(const DIMemoryUse &Use,
                            Diag<StringRef, bool> DiagMessage);
     void diagnoseRefElementAddr(RefElementAddrInst *REI);
@@ -747,8 +749,7 @@ void LifetimeChecker::noteUninitializedMembers(const DIMemoryUse &Use) {
     }
 
     if (propertyInitIsolation.isGlobalActor()) {
-      auto *init =
-          dyn_cast<ConstructorDecl>(F.getDeclContext()->getAsDecl());
+      auto *init = dyn_cast<ConstructorDecl>(F.getDeclContext()->getAsDecl());
       diagnose(Module, Loc, diag::isolated_property_initializer,
                StringRef(Name), propertyInitIsolation,
                getActorIsolation(init));
@@ -2298,14 +2299,112 @@ bool LifetimeChecker::diagnoseReturnWithoutInitializingStoredProperties(
              theStruct->getParentModule()->getName(),
              theStruct->hasClangNode());
   } else {
-    diagnose(Module, loc,
-             diag::return_from_init_without_initing_stored_properties);
+    emitUninitializedMembersFixit(Inst, loc, Use);
     noteUninitializedMembers(Use);
   }
 
   return true;
 }
 
+/// Emit fix-its for each uninitialized stored property in a designated
+/// initializer.
+void LifetimeChecker::emitUninitializedMembersFixit(const SILInstruction *Inst,
+                                                    SILLocation loc,
+                                                    const DIMemoryUse &Use) {
+  // to generate the missing variables
+  std::string missingVariablesFixIt;
+  std::string suggestedInitializerDeclsString;
+  std::unordered_map<std::string, bool> handledVariablesMap;
+  AvailabilitySet Liveness =
+      getLivenessAtInst(Use.Inst, Use.FirstElement, Use.NumElements);
+  Decl *initFunctionDecl = Inst->getFunction()->getDeclContext()->getAsDecl();
+  AbstractFunctionDecl *functionDecl =
+      dyn_cast<AbstractFunctionDecl>(initFunctionDecl);
+  auto *parameters = functionDecl->getParameters();
+  SourceLoc RParenLoc = parameters->getRParenLoc();
+  SILLocation initFunctionLocation = SILLocation(functionDecl);
+
+  for (auto param : parameters->getArray()) {
+    auto paramName = param->getName().str().str();
+    handledVariablesMap[paramName] = false;
+  }
+  for (unsigned i = Use.FirstElement, e = Use.FirstElement + Use.NumElements;
+       i != e; ++i) {
+    // if Already initialized, skip the Decl.
+    if (Liveness.get(i) == DIKind::Yes)
+      continue;
+
+    // Ignore a failed super.init requirement.
+    if (i == TheMemory.getNumElements() - 1 && TheMemory.isDerivedClassSelf())
+      continue;
+    std::string Name;
+    auto *Decl = TheMemory.getPathStringToElement(i, Name);
+
+    auto propertyInitIsolation = ActorIsolation::forUnspecified();
+
+    std::string inferredDeclType;
+    std::string declName;
+
+    if (Decl) {
+      if (auto *var = dyn_cast<VarDecl>(Decl)) {
+        inferredDeclType = var->getValueInterfaceType().getString();
+        declName = var->getName().str();
+        propertyInitIsolation = var->getInitializerIsolation();
+      }
+
+      auto variable = handledVariablesMap.find(declName);
+      auto variableWasSuggestedOrExistsInCurrentParams =
+          variable != handledVariablesMap.end();
+      if (variableWasSuggestedOrExistsInCurrentParams && variable->second) {
+        continue;
+      }
+    }
+
+    auto variable = handledVariablesMap.find(declName);
+    // if a variable exists, and it's false so it's alreeady passed in the
+    // init(..)
+    auto shouldAddDecl = false;
+    if (variable != handledVariablesMap.end()) {
+      shouldAddDecl = variable->first == declName;
+    }
+
+    if (!shouldAddDecl) {
+      suggestedInitializerDeclsString +=
+          declName + ": " + inferredDeclType + ", ";
+    }
+    missingVariablesFixIt += "self." + declName + " = " + declName + "\n";
+
+    // mark variable as handled, which means it's already passed in the init, or
+    // going to appear in the fixit, this's needed for tuples, to avoid
+    // generating a decl for each element type defined.
+    handledVariablesMap[declName] = true;
+  }
+
+  // Drop last `, ` from the parameters String
+  auto didGenerateInitializingStoredProperties =
+      suggestedInitializerDeclsString.size() > 0;
+  if (didGenerateInitializingStoredProperties) {
+    suggestedInitializerDeclsString.pop_back();
+    suggestedInitializerDeclsString.pop_back();
+  }
+
+  // if we have a message,and there is already one parameter at least in the
+  // init then we add a comma to the beginning of the message
+  if (parameters->size() >= 1 && didGenerateInitializingStoredProperties) {
+    suggestedInitializerDeclsString = ", " + suggestedInitializerDeclsString;
+  }
+
+  auto diag =
+      diagnose(Module, initFunctionLocation,
+               diag::return_from_init_without_initing_stored_properties);
+  diag.fixItInsert(initFunctionLocation.getEndSourceLoc(),
+                   missingVariablesFixIt);
+
+  if (suggestedInitializerDeclsString.size() > 0) {
+    diag.fixItInsert(RParenLoc, suggestedInitializerDeclsString);
+  }
+}
+
 /// Check and diagnose various failures when a load use is not fully
 /// initialized.
 ///
@@ -2390,8 +2489,7 @@ void LifetimeChecker::handleLoadUseFailure(const DIMemoryUse &Use,
                diag::superselfinit_not_called_before_return,
                (unsigned)TheMemory.isDelegatingInit());
     } else {
-      diagnose(Module, Inst->getLoc(),
-               diag::return_from_init_without_initing_stored_properties);
+      emitUninitializedMembersFixit(Inst, Inst->getLoc(), Use);
       noteUninitializedMembers(Use);
     }
     return;

test/SILOptimizer/definite_init.swift

@@ -64,17 +64,17 @@ class CheckCompilerInitAttr {
   }
 
   // NB: this case is testing whether we only get a note for `whatever` and not `poster`
-  init(v2: Void) {} // expected-error {{return from initializer without initializing all stored properties}}
+  init(v2: Void) {} // expected-error {{return from initializer without initializing all stored properties}} {{29:3-3=self.ivar = ivar\n}}
 }
 
 class AgainCheckCompilerInitAttr {
   @_compilerInitialized let cleanup: Int // expected-note {{'self.cleanup' not initialized}}
   var whenever: Int
 
   // NB: this case ensures we still get a note for `cleanup` because no other properties are uninitialized
-  init() {
+  init() {// expected-error {{return from initializer without initializing all stored properties}}
     whenever = 0
-  } // expected-error {{return from initializer without initializing all stored properties}}
+  }
 }
 
 class Super {

test/SILOptimizer/definite_init_closures_fail.swift

@@ -7,36 +7,36 @@ struct Simple {
               // expected-note @-1 {{'self.x' not initialized}}
   let y: Bool
 
-  init() {
+  init() { // expected-error {{return from initializer without initializing all stored properties}} {{12:3-3=self.x = x\n}} {{8-8=x: Bool}}
     y = false || x // expected-error {{constant 'self.x' used before being initialized}}
-  } // expected-error {{return from initializer without initializing all stored properties}}
+  }
 
-  init(b: Bool) {
+  init(b: Bool) { // expected-error {{return from initializer without initializing all stored properties}} {{19:3-3=self.x = x\n}} {{15-15=, x: Bool}}
     if b {
       x = false
     }
     y = false || x // expected-error {{constant 'self.x' used before being initialized}}
-  } // expected-error {{return from initializer without initializing all stored properties}}
+  } 
 }
 
 struct NestedClosures {
   let x: Bool // expected-note {{'self.x' not initialized}}
   let y: Bool
   let z: Bool
 
-  init(_ a: Bool) {
+  init(_ a: Bool) {// expected-error {{return from initializer without initializing all stored properties}} {{30:3-3=self.x = x\n}} {{17-17=, x: Bool}}
     y = false
     z = false || (y || (x || a)) // expected-error {{constant 'self.x' used before being initialized}}
-  } // expected-error {{return from initializer without initializing all stored properties}}
+  }
 }
 
 class SimpleClass {
   let x: Bool // expected-note {{'self.x' not initialized}}
   let y: Bool
 
-  init() {
+  init() {// expected-error {{return from initializer without initializing all stored properties}} {{39:3-3=self.x = x\n}} {{8-8=x: Bool}}
     y = false || x // expected-error {{constant 'self.x' used before being initialized}}
-  } // expected-error {{return from initializer without initializing all stored properties}}
+  }
 }
 
 func forward(_ b: inout Bool) -> Bool {
@@ -60,8 +60,8 @@ struct Generic<T : P> {
   let x: T // expected-note {{'self.x' not initialized}}
   let y: Bool
 
-  init(_ t: T) {
+  init(_ t: T) {// expected-error {{return from initializer without initializing all stored properties}} {{65:3-3=self.x = x\n}} {{14-14=, x: T}}
     y = false || x.b // expected-error {{constant 'self.x' used before being initialized}}
-  } // expected-error {{return from initializer without initializing all stored properties}}
+  }
 }
 

test/SILOptimizer/definite_init_cross_module_swift4.swift

@@ -14,9 +14,9 @@ extension Point {
     self.y = yy
   }
 
-  init(xx: Double) {
+  init(xx: Double) {// expected-error {{return from initializer without initializing all stored properties}} {{19:3-3=self.y = y\n}} {{18-18=, y: Double}}
     self.x = xx // expected-warning {{initializer for struct 'Point' must use "self.init(...)" or "self = ..." because it is not in module 'OtherModule'}}
-  } // expected-error {{return from initializer without initializing all stored properties}}
+  }
 
   init(xxx: Double, yyy: Double) {
     // This is OK
@@ -241,15 +241,14 @@ extension PrivatePoint {
     // This is OK
     self = other
   }
-
-  init(other: PrivatePoint, cond: Bool) {
+  init(other: PrivatePoint, cond: Bool) { // expected-error {{return from initializer without initializing all stored properties}} {{246:3-3=self.x = x\nself.y = y\n}} {{39-39=, x: Double, y: Double}}
     if cond { self = other }
-  } // expected-error {{return from initializer without initializing all stored properties}}
+  }
 
   // Ideally we wouldn't mention the names of non-public stored properties
   // across module boundaries, but this will go away in Swift 5 mode anyway.
-  init() {
-  } // expected-error {{return from initializer without initializing all stored properties}}
+  init() {// expected-error {{return from initializer without initializing all stored properties}} {{251:3-3=self.x = x\nself.y = y\n}} {{8-8=x: Double, y: Double}}
+  }
 }
 
 extension Empty {

test/SILOptimizer/definite_init_diagnostics.swift

@@ -23,9 +23,9 @@ class SomeClass {
   var computedProperty : Int { return 42 }
 
   init() { x = 0 }
-  init(b : Bool) {
+  init(b : Bool) {// expected-error {{return from initializer without initializing all stored properties}} {{28:3-3=self.x = x\n}} {{16-16=, x: Int}}
     if (b) {}
-  } // expected-error {{return from initializer without initializing all stored properties}}
+  }
 
   func baseMethod() {}
 }
@@ -817,7 +817,7 @@ struct LetProperties {
   }
 
   // Multiple initializations are an error.
-  init(a : Int) {
+  init(a : Int) {// expected-error {{return from initializer without initializing all stored properties}} {{837:3-3=self.z = z\n}} {{15-15=, z: Int?}}
     y = a
     y = a   // expected-error {{immutable value 'self.y' may only be initialized once}}
 
@@ -834,7 +834,7 @@ struct LetProperties {
 
     arr = []
     arr = [] // expected-error {{immutable value 'self.arr' may only be initialized once}}
-  }  // expected-error {{return from initializer without initializing all stored properties}}
+  }
 
   // inout uses of let properties are an error.
   init() throws {
@@ -980,9 +980,9 @@ struct AddressOnlyStructWithInit<T, U> {
   let a : T?
   let b : U?   // expected-note {{'self.b' not initialized}}
 
-  init(a : T) {
+  init(a : T) {// expected-error {{return from initializer without initializing all stored properties}} {{985:3-3=self.b = b\n}} {{13-13=, b: U?}}
     self.a = a
-  }     // expected-error {{return from initializer without initializing all stored properties}}
+  }
 }
 
 enum AddressOnlyEnumWithInit<T> {
@@ -1159,10 +1159,10 @@ struct S1_44078 {
   let a: Int
   let b: Int // expected-note {{'self.b' not initialized}}
 
-  init?(x: Int, y: Int) {
+  init?(x: Int, y: Int) {// expected-error {{return from initializer without initializing all stored properties}} {{1169:3-3=self.b = b\n}} {{23-23=, b: Int}}
     self.a = x
     if y == 42 {
-      return // expected-error {{return from initializer without initializing all stored properties}}
+      return
     }
     // many lines later
     self.b = y
@@ -1173,23 +1173,23 @@ struct S2_44078 {
   let a: Int
   let b: Int // expected-note {{'self.b' not initialized}}
 
-  init?(x: Int, y: Int) {
+  init?(x: Int, y: Int) {// expected-error {{return from initializer without initializing all stored properties}} {{1179:3-3=self.b = b\n}} {{23-23=, b: Int}}
     self.a = x
-    return // expected-error {{return from initializer without initializing all stored properties}}
+    return
   }
 }
 
 struct S3_44078 {
   let a: Int
   let b: Int // expected-note {{'self.b' not initialized}}
 
-  init?(x: Int, y: Int) {
+  init?(x: Int, y: Int) {// expected-error {{return from initializer without initializing all stored properties}} {{1192:3-3=self.b = b\n}} {{23-23=, b: Int}}
     self.a = x
     if y == 42 {
       self.b = y
       return
     }
-  } // expected-error {{return from initializer without initializing all stored properties}}
+  }
 }
 
 enum E1_44078 {

test/SILOptimizer/definite_init_extension.swift

@@ -10,7 +10,7 @@ extension S where T == (Int, String) {
     t.1 = "hi"
   }
 
-  init(y: ()) {
+  init(y: ()) {// expected-error {{return from initializer without initializing all stored properties}} {{15:3-3=self.t = t\n}} {{13-13=, t: T}}
     t.0 = 1
-  } // expected-error {{return from initializer without initializing all stored properties}}
+  }
 }

test/SILOptimizer/definite_init_failable_initializers_diagnostics.swift

@@ -13,11 +13,11 @@ class FailableInitThatFailsReallyHard {
 // Failable initializers must produce correct diagnostics
 struct A {
   var x: Int  // expected-note {{'self.x' not initialized}}
-  init?(i: Int) {
+  init?(i: Int) {// expected-error {{return from initializer without initializing all stored properties}} {{20:3-3=self.x = x\n}} {{15-15=, x: Int}}
     if i > 0 {
       self.x = i
     }
-  } // expected-error {{return from initializer without initializing all stored properties}}
+  }
 }
 
 // Delegating, failable initializers that doesn't initialize along all paths must produce correct diagnostics.