[move-only] Emit a clearer message around deinits.

Specifically:

1. Fix the error message so that when we say you can't have a deinit that a
deinit can be on a noncopyable type along side a class or an actor.

2. Even though we already error on @objc enums and say they cannot be
noncopyable, we did not emit an error on the deinit saying that @objc enums
cannot have a deinit. I put in a nice to have error just to make it even
clearer.

rdar://105855978
rdar://106566054

Author: gottesmm

include/swift/AST/DiagnosticsParse.def

@@ -422,8 +422,10 @@ ERROR(initializer_result_type,PointsToFirstBadToken,
       "initializers cannot have a result type", ())
 
 // Destructor
-ERROR(destructor_decl_outside_class,none,
-      "deinitializers may only be declared within a class or actor", ())
+ERROR(destructor_decl_outside_class_or_noncopyable,none,
+      "deinitializers may only be declared within a class, actor, or noncopyable type", ())
+ERROR(destructor_decl_on_objc_enum,none,
+      "deinitializers cannot be declared on an @objc enum type", ())
 ERROR(expected_lbrace_destructor,PointsToFirstBadToken,
       "expected '{' for deinitializer", ())
 ERROR(destructor_has_name,PointsToFirstBadToken,

lib/Parse/ParseDecl.cpp

@@ -9062,14 +9062,23 @@ parseDeclDeinit(ParseDeclOptions Flags, DeclAttributes &Attributes) {
 
   DD->getAttrs() = Attributes;
 
-  // Reject 'destructor' functions outside of structs, enums, and classes.
+  // Reject 'destructor' functions outside of structs, enums, classes, or
+  // extensions that provide objc implementations.
   //
   // Later in the type checker, we validate that structs/enums only do this if
-  // they are move only.
-  auto *nom = dyn_cast<NominalTypeDecl>(CurDeclContext);
-  if (!nom ||
-      (!isa<StructDecl>(nom) && !isa<EnumDecl>(nom) && !isa<ClassDecl>(nom))) {
-    diagnose(DestructorLoc, diag::destructor_decl_outside_class);
+  // they are move only and that @objcImplementations are main-body.
+  auto rejectDestructor = [](DeclContext *dc) {
+    if (isa<StructDecl>(dc) || isa<EnumDecl>(dc) ||
+        isa<ClassDecl>(dc))
+      return false;
+
+    if (auto *ED = dyn_cast<ExtensionDecl>(dc))
+      return !ED->isObjCImplementation();
+
+    return true;
+  };
+  if (rejectDestructor(CurDeclContext)) {
+    diagnose(DestructorLoc, diag::destructor_decl_outside_class_or_noncopyable);
 
     // Tell the type checker not to touch this destructor.
     DD->setInvalid();

lib/Sema/TypeCheckDeclPrimary.cpp

@@ -3625,12 +3625,23 @@ class DeclChecker : public DeclVisitor<DeclChecker> {
   }
 
   void visitDestructorDecl(DestructorDecl *DD) {
-    // Only check again for destructor decl outside of a class if our dstructor
+    // Only check again for destructor decl outside of a class if our destructor
     // is not marked as invalid.
     if (!DD->isInvalid()) {
       auto *nom = dyn_cast<NominalTypeDecl>(DD->getDeclContext());
       if (!nom || (!isa<ClassDecl>(nom) && !nom->isMoveOnly())) {
-        DD->diagnose(diag::destructor_decl_outside_class);
+        DD->diagnose(diag::destructor_decl_outside_class_or_noncopyable);
+      }
+
+      // If we have a noncopyable type, check if we have an @objc enum with a
+      // deinit and emit a specialized error. We will have technically already
+      // emitted an error since @objc enum cannot be marked noncopyable, but
+      // this at least makes it a bit clearer to the user that the deinit is
+      // also incorrect.
+      if (auto *e = dyn_cast_or_null<EnumDecl>(nom)) {
+        if (e->isObjC()) {
+          DD->diagnose(diag::destructor_decl_on_objc_enum);
+        }
       }
     }
 

test/Parse/init_deinit.swift

@@ -34,7 +34,7 @@ struct FooStructDeinitializerB {
 }
 
 struct FooStructDeinitializerC {
-  deinit {} // expected-error {{deinitializers may only be declared within a class or actor}}
+  deinit {} // expected-error {{deinitializers may only be declared within a class, actor, or noncopyable type}}
 }
 
 class FooClassDeinitializerA {
@@ -53,30 +53,30 @@ init {} // expected-error {{initializers may only be declared within a type}} ex
 init() // expected-error {{initializers may only be declared within a type}}
 init() {} // expected-error {{initializers may only be declared within a type}}
 
-deinit {} // expected-error {{deinitializers may only be declared within a class or actor}}
+deinit {} // expected-error {{deinitializers may only be declared within a class, actor, or noncopyable type}}
 deinit // expected-error {{expected '{' for deinitializer}}
-deinit {} // expected-error {{deinitializers may only be declared within a class or actor}}
+deinit {} // expected-error {{deinitializers may only be declared within a class, actor, or noncopyable type}}
 
 struct BarStruct {
   init() {}
-  deinit {} // expected-error {{deinitializers may only be declared within a class or actor}}
+  deinit {} // expected-error {{deinitializers may only be declared within a class, actor, or noncopyable type}}
 }
 
 extension BarStruct {
   init(x : Int) {}
 
   // When/if we allow 'var' in extensions, then we should also allow dtors
-  deinit {} // expected-error {{deinitializers may only be declared within a class or actor}}
+  deinit {} // expected-error {{deinitializers may only be declared within a class, actor, or noncopyable type}}
 }
 
 enum BarUnion {
   init() {}
-  deinit {} // expected-error {{deinitializers may only be declared within a class or actor}}
+  deinit {} // expected-error {{deinitializers may only be declared within a class, actor, or noncopyable type}}
 }
 
 extension BarUnion {
   init(x : Int) {}
-  deinit {} // expected-error {{deinitializers may only be declared within a class or actor}}
+  deinit {} // expected-error {{deinitializers may only be declared within a class, actor, or noncopyable type}}
 }
 
 class BarClass {
@@ -86,22 +86,22 @@ class BarClass {
 
 extension BarClass {
   convenience init(x : Int) { self.init() }
-  deinit {} // expected-error {{deinitializers may only be declared within a class or actor}}
+  deinit {} // expected-error {{deinitializers may only be declared within a class, actor, or noncopyable type}}
 }
 
 protocol BarProtocol {
   init() {} // expected-error {{protocol initializers must not have bodies}}
-  deinit {} // expected-error {{deinitializers may only be declared within a class or actor}}
+  deinit {} // expected-error {{deinitializers may only be declared within a class, actor, or noncopyable type}}
 }
 
 extension BarProtocol {
   init(x : Int) {}
-  deinit {} // expected-error {{deinitializers may only be declared within a class or actor}}
+  deinit {} // expected-error {{deinitializers may only be declared within a class, actor, or noncopyable type}}
 }
 
 func fooFunc() {
   init() {} // expected-error {{initializers may only be declared within a type}}
-  deinit {} // expected-error {{deinitializers may only be declared within a class or actor}}
+  deinit {} // expected-error {{deinitializers may only be declared within a class, actor, or noncopyable type}}
 }
 
 func barFunc() {
@@ -113,7 +113,7 @@ func barFunc() {
 
   var y : () = { () -> () in
     // expected-warning@-1 {{variable 'y' was never used; consider replacing with '_' or removing it}}
-    deinit {} // expected-error {{deinitializers may only be declared within a class or actor}}
+    deinit {} // expected-error {{deinitializers may only be declared within a class, actor, or noncopyable type}}
     return
   } ()
 }

test/SIL/parse_swift_decls_in_sil_mode.sil

@@ -2,7 +2,7 @@
 
 typealias Int = ()
 
-deinit // expected-error {{deinitializers may only be declared within a class or actor}}
+deinit // expected-error {{deinitializers may only be declared within a class, actor, or noncopyable type}}
 
 subscript (x : Int, y : Int) -> Int { get }// expected-error{{'subscript' functions may only be declared within a type}}
 

test/Sema/moveonly_objc_enum.swift

@@ -7,7 +7,7 @@
 @_moveOnly
 @objc enum Foo : Int { // expected-error {{@objc enums cannot be marked as move-only}}
   case X, Y, Z
-  deinit {} // objc enums cannot have deinits
+  deinit {} // expected-error {{deinitializers cannot be declared on an @objc enum type}}
 }
 
 @_moveOnly

test/Serialization/AllowErrors/invalid-deinit.swift

@@ -12,7 +12,7 @@
 struct Foo {}
 
 @discardableResult // expected-error{{'@discardableResult' attribute cannot be applied to this declaration}}
-deinit {} // expected-error{{deinitializers may only be declared within a class or actor}}
+deinit {} // expected-error{{deinitializers may only be declared within a class, actor, or noncopyable type}}
 
 func foo() -> Foo { return Foo() }
 

test/SourceKit/DocumentStructure/structure.swift.empty.response

@@ -1721,8 +1721,8 @@
       key.line: 147,
       key.column: 1,
       key.severity: source.diagnostic.severity.error,
-      key.id: "destructor_decl_outside_class",
-      key.description: "deinitializers may only be declared within a class or actor",
+      key.id: "destructor_decl_outside_class_or_noncopyable",
+      key.description: "deinitializers may only be declared within a class, actor, or noncopyable type",
       key.diagnostic_stage: source.diagnostic.stage.swift.parse
     }
   ]

test/SourceKit/DocumentStructure/structure.swift.foobar.response

@@ -1726,8 +1726,8 @@
       key.column: 1,
       key.filepath: "-foobar",
       key.severity: source.diagnostic.severity.error,
-      key.id: "destructor_decl_outside_class",
-      key.description: "deinitializers may only be declared within a class or actor",
+      key.id: "destructor_decl_outside_class_or_noncopyable",
+      key.description: "deinitializers may only be declared within a class, actor, or noncopyable type",
       key.diagnostic_stage: source.diagnostic.stage.swift.parse
     }
   ]

test/SourceKit/DocumentStructure/structure.swift.response

@@ -1726,8 +1726,8 @@
       key.column: 1,
       key.filepath: main.swift,
       key.severity: source.diagnostic.severity.error,
-      key.id: "destructor_decl_outside_class",
-      key.description: "deinitializers may only be declared within a class or actor",
+      key.id: "destructor_decl_outside_class_or_noncopyable",
+      key.description: "deinitializers may only be declared within a class, actor, or noncopyable type",
       key.diagnostic_stage: source.diagnostic.stage.swift.parse
     }
   ]