Additional responses to feedback

* Make Decodable synthesis lazy again
* Remove side effects from assertions
* Assert on superclass failable initializers (and verify via a unit test)
* Fix space typo
* Remove more needless instances of single-element arrays
* Unambiguously refer to init() in unit tests
* Add failable superclass init tests and a test to ensure Decodable fails to be inherited when superclass synthesis fails
* Add tests for structs to ensure they receive no-argument initializers where appropriate

Author: Itai Ferber

lib/Sema/DerivedConformanceCodable.cpp

@@ -804,12 +804,8 @@ static FuncDecl *deriveEncodable_encode(TypeChecker &tc, Decl *parentDecl,
 
 /// Synthesizes the body for `init(from decoder: Decoder) throws`.
 ///
-/// \param tc The \c TypeChecker to use in looking up potential superclass
-///           Decodable conformance.
-///
 /// \param initDecl The function decl whose body to synthesize.
-static BraceStmt *deriveBodyDecodable_init(TypeChecker &tc,
-                                           AbstractFunctionDecl *initDecl) {
+static void deriveBodyDecodable_init(AbstractFunctionDecl *initDecl) {
   // struct Foo : Codable {
   //   var x: Int
   //   var y: String
@@ -971,10 +967,7 @@ static BraceStmt *deriveBodyDecodable_init(TypeChecker &tc,
   // superclass is Decodable, or super.init() if it is not.
   if (auto *classDecl = dyn_cast<ClassDecl>(targetDecl)) {
     if (auto *superclassDecl = classDecl->getSuperclassDecl()) {
-      auto superType = superclassDecl->getDeclaredInterfaceType();
-      if (tc.conformsToProtocol(superType,
-                                C.getProtocol(KnownProtocolKind::Decodable),
-                                superclassDecl, ConformanceCheckOptions())) {
+      if (superclassIsDecodable(classDecl)) {
         // Need to generate `try super.init(from: container.superDecoder())`
 
         // container.superDecoder
@@ -1014,12 +1007,15 @@ static BraceStmt *deriveBodyDecodable_init(TypeChecker &tc,
         DeclName initName(C, C.Id_init, ArrayRef<Identifier>());
 
         // We need to look this up in the superclass to see if it throws.
-        ConstructorDecl *superInitDecl = nullptr;
         auto result = superclassDecl->lookupDirect(initName);
 
         // We should have bailed one level up if this were not available.
-        assert(!result.empty() &&
-               (superInitDecl = dyn_cast<ConstructorDecl>(result.front())));
+        assert(!result.empty());
+
+        // If the init is failable, we should have already bailed one level
+        // above.
+        ConstructorDecl *superInitDecl = cast<ConstructorDecl>(result.front());
+        assert(superInitDecl->getFailability() == OTK_None);
 
         // super
         auto *superRef = new (C) SuperRefExpr(initDecl->getImplicitSelfDecl(),
@@ -1034,10 +1030,6 @@ static BraceStmt *deriveBodyDecodable_init(TypeChecker &tc,
                                                   ArrayRef<Expr *>(),
                                                   ArrayRef<Identifier>());
 
-        // If super.init is failable, super.init()!
-        if (superInitDecl->getFailability() != OTK_None)
-          callExpr = new (C) ForceValueExpr(callExpr, SourceLoc());
-
         // If super.init throws, try super.init()
         if (superInitDecl->hasThrows())
           callExpr = new (C) TryExpr(SourceLoc(), callExpr, Type(),
@@ -1048,8 +1040,9 @@ static BraceStmt *deriveBodyDecodable_init(TypeChecker &tc,
     }
   }
 
-  return BraceStmt::create(C, SourceLoc(), statements, SourceLoc(),
-                           /*implicit=*/true);
+  auto *body = BraceStmt::create(C, SourceLoc(), statements, SourceLoc(),
+                                 /*implicit=*/true);
+  initDecl->setBody(body);
 }
 
 /// Synthesizes a function declaration for `init(from: Decoder) throws` with a
@@ -1114,7 +1107,7 @@ static ValueDecl *deriveDecodable_init(TypeChecker &tc, Decl *parentDecl,
                                            SourceLoc(), selfDecl, paramList,
                                            /*GenericParams=*/nullptr, target);
   initDecl->setImplicit();
-  initDecl->setBody(deriveBodyDecodable_init(tc, initDecl));
+  initDecl->setBodySynthesizer(deriveBodyDecodable_init);
 
   // This constructor should be marked as `required` for non-final classes.
   if (isa<ClassDecl>(target) && !target->getAttrs().hasAttribute<FinalAttr>()) {
@@ -1229,6 +1222,7 @@ static bool canSynthesize(TypeChecker &tc, NominalTypeDecl *target,
           // isn't failable.
           tc.diagnose(initializer, diag::decodable_super_init_is_failable_here,
                       requirement->getFullName(), memberName);
+          return false;
         }
       }
     }

lib/Sema/TypeCheckDecl.cpp

@@ -7963,17 +7963,17 @@ static void diagnoseClassWithoutInitializers(TypeChecker &tc,
     ASTContext &C = tc.Context;
     auto *decodableProto = C.getProtocol(KnownProtocolKind::Decodable);
     auto superclassType = superclassDecl->getDeclaredInterfaceType();
-    if (auto ref = tc. conformsToProtocol(superclassType, decodableProto,
-                                          superclassDecl,
-                                          ConformanceCheckOptions(),
-                                          SourceLoc())) {
+    if (auto ref = tc.conformsToProtocol(superclassType, decodableProto,
+                                         superclassDecl,
+                                         ConformanceCheckOptions(),
+                                         SourceLoc())) {
       // super conforms to Decodable, so we've failed to inherit init(from:).
       // Let's suggest overriding it here.
       //
       // We're going to diagnose on the concrete init(from:) decl if it exists
       // and isn't implicit; otherwise, on the subclass itself.
       ValueDecl *diagDest = classDecl;
-      auto initFrom = DeclName(C, C.Id_init, (Identifier[1]){ C.Id_from });
+      auto initFrom = DeclName(C, C.Id_init, C.Id_from);
       auto result = tc.lookupMember(superclassDecl, superclassType, initFrom,
                                     NameLookupFlags::ProtocolMembers |
                                     NameLookupFlags::IgnoreAccessibility);
@@ -8204,10 +8204,11 @@ void TypeChecker::addImplicitConstructors(NominalTypeDecl *decl) {
   // NOTE: Lookups of synthesized initializers MUST come after
   //       decl->setAddedImplicitInitializers() in case synthesis requires
   //       protocol conformance checking, which might be recursive here.
+  // FIXME: Disable this code and prevent _any_ implicit constructors from doing
+  //        this. Investigate why this hasn't worked otherwise.
   DeclName synthesizedInitializers[1] = {
     // init(from:) is synthesized by derived conformance to Decodable.
-    DeclName(Context, DeclBaseName(Context.Id_init),
-             (Identifier[1]) { Context.Id_from })
+    DeclName(Context, DeclBaseName(Context.Id_init), Context.Id_from)
   };
 
   auto initializerIsSynthesized = [=](ConstructorDecl *initializer) {

test/decl/protocol/special/coding/class_codable_default_initializer.swift

@@ -8,7 +8,7 @@ class NoInitializers { // expected-error {{class 'NoInitializers' has no initial
 
   func foo() {
     // The class should not receive a default constructor.
-    let _ = NoInitializers.init // expected-error {{type 'NoInitializers' has no member 'init'}}
+    let _ = NoInitializers.init() // expected-error {{type 'NoInitializers' has no member 'init'}}
   }
 }
 
@@ -23,7 +23,7 @@ class CodableNoExplicitInitializers : Codable {
     let _ = CodableNoExplicitInitializers.encode(to:)
 
     // It should not, however, receive a default constructor.
-    let _ = CodableNoExplicitInitializers.init
+    let _ = CodableNoExplicitInitializers.init() // expected-error {{missing argument for parameter 'from' in call}}
   }
 }
 
@@ -32,7 +32,7 @@ class DefaultConstructed {
   var x: Double = .pi
 
   func foo() {
-    let _ = DefaultConstructed.init
+    let _ = DefaultConstructed.init()
   }
 }
 

test/decl/protocol/special/coding/class_codable_inheritance_diagnostics.swift

@@ -31,6 +31,30 @@ class InaccessibleNonDecodableSuper {
 class InaccessibleNonDecodableSub : InaccessibleNonDecodableSuper, Decodable { // expected-error {{type 'InaccessibleNonDecodableSub' does not conform to protocol 'Decodable'}}
 }
 
+// Non-Decodable superclasses of synthesized Decodable classes must have a
+// non-failable init().
+class FailableNonDecodableSuper {
+  init?() {} // expected-note {{cannot automatically synthesize 'init(from:)' because implementation would need to call 'init()', which is failable}}
+}
+
+class FailableNonDecodableSub : FailableNonDecodableSuper, Decodable { // expected-error {{type 'FailableNonDecodableSub' does not conform to protocol 'Decodable'}}
+}
+
+// Subclasses of classes whose Decodable synthesis fails should not inherit
+// conformance.
+class FailedSynthesisDecodableSuper : Decodable { // expected-error {{type 'FailedSynthesisDecodableSuper' does not conform to protocol 'Decodable'}}
+  enum CodingKeys : String, CodingKey {
+    case nonexistent // expected-note {{CodingKey case 'nonexistent' does not match any stored properties}}
+  }
+}
+
+class FailedSynthesisDecodableSub : FailedSynthesisDecodableSuper { // expected-note {{did you mean 'init'?}}
+  func foo() {
+    // Decodable should fail to synthesis or be inherited.
+    let _ = FailedSynthesisDecodableSub.init(from:) // expected-error {{type 'FailedSynthesisDecodableSub' has no member 'init(from:)'}}
+  }
+}
+
 // Subclasses of Decodable classes which can't inherit their initializers should
 // produce diagnostics.
 class DecodableSuper : Decodable {

test/decl/protocol/special/coding/struct_codable_memberwise_initializer.swift

@@ -31,7 +31,10 @@ struct InitialValueNoInitializers {
 
   func foo() {
     // The struct should receive a memberwise initializer.
-    let _ = NoInitializers.init(x:)
+    let _ = InitialValueNoInitializers.init(x:)
+
+    // The struct should receive a no-argument initializer.
+    let _ = InitialValueNoInitializers.init()
   }
 }
 
@@ -40,10 +43,13 @@ struct InitialValueCodableNoExplicitInitializers : Codable {
 
   func foo() {
     // The struct should receive a synthesized init(from:) and encode(to:).
-    let _ = CodableNoExplicitInitializers.init(from:)
-    let _ = CodableNoExplicitInitializers.encode(to:)
+    let _ = InitialValueCodableNoExplicitInitializers.init(from:)
+    let _ = InitialValueCodableNoExplicitInitializers.encode(to:)
 
     // It should still receive a memberwise initializer.
-    let _ = CodableNoExplicitInitializers.init(x:)
+    let _ = InitialValueCodableNoExplicitInitializers.init(x:)
+
+    // It should still receive a no-argument initializer.
+    let _ = InitialValueCodableNoExplicitInitializers.init()
   }
 }