[Diag] QoI: Add ReferenceOwnership to DiagnosticArgumentKind

By formalizing ReferenceOwnership as a diagnostic argument kind, we get
less boilerplate, better type safety, better output consistency, and
last but not least: future proofing.

Author: davezarzycki

include/swift/AST/DiagnosticEngine.h

@@ -30,6 +30,7 @@ namespace swift {
   class ValueDecl;
 
   enum class PatternKind : uint8_t;
+  enum class ReferenceOwnership : uint8_t;
   enum class StaticSpellingKind : uint8_t;
   enum class DescriptiveDeclKind : uint8_t;
   enum DeclAttrKind : unsigned;
@@ -76,6 +77,7 @@ namespace swift {
     Type,
     TypeRepr,
     PatternKind,
+    ReferenceOwnership,
     StaticSpellingKind,
     DescriptiveDeclKind,
     DeclAttribute,
@@ -103,6 +105,7 @@ namespace swift {
       Type TypeVal;
       TypeRepr *TyR;
       PatternKind PatternKindVal;
+      ReferenceOwnership ReferenceOwnershipVal;
       StaticSpellingKind StaticSpellingKindVal;
       DescriptiveDeclKind DescriptiveDeclKindVal;
       const DeclAttribute *DeclAttributeVal;
@@ -162,6 +165,10 @@ namespace swift {
     DiagnosticArgument(PatternKind K)
         : Kind(DiagnosticArgumentKind::PatternKind), PatternKindVal(K) {}
 
+    DiagnosticArgument(ReferenceOwnership RO)
+        : Kind(DiagnosticArgumentKind::ReferenceOwnership),
+          ReferenceOwnershipVal(RO) {}
+
     DiagnosticArgument(StaticSpellingKind SSK)
         : Kind(DiagnosticArgumentKind::StaticSpellingKind),
           StaticSpellingKindVal(SSK) {}
@@ -237,6 +244,11 @@ namespace swift {
       return PatternKindVal;
     }
 
+    ReferenceOwnership getAsReferenceOwnership() const {
+      assert(Kind == DiagnosticArgumentKind::ReferenceOwnership);
+      return ReferenceOwnershipVal;
+    }
+
     StaticSpellingKind getAsStaticSpellingKind() const {
       assert(Kind == DiagnosticArgumentKind::StaticSpellingKind);
       return StaticSpellingKindVal;

include/swift/AST/DiagnosticsSema.def

@@ -1960,9 +1960,8 @@ ERROR(override_argument_name_mismatch,none,
       "of overridden %select{method|initializer}0 %2",
       (bool, DeclName, DeclName))
 ERROR(override_ownership_mismatch,none,
-      "cannot override %select{strong|weak|unowned|unowned(unsafe)}0 property "
-      "with %select{strong|weak|unowned|unowned(unsafe)}1 property",
-      (/*ReferenceOwnership*/unsigned, /*ReferenceOwnership*/unsigned))
+      "cannot override %0 property with %1 property",
+      (ReferenceOwnership, ReferenceOwnership))
 ERROR(override_dynamic_self_mismatch,none,
       "cannot override a Self return type with a non-Self return type",
       ())
@@ -3069,10 +3068,9 @@ NOTE(add_explicit_type_annotation_to_silence,none,
      "add an explicit type annotation to silence this warning", ())
 
 WARNING(unowned_assignment_immediate_deallocation,none,
-        "instance will be immediately deallocated as %0 is "
-        "%select{a 'strong'|a 'weak'|an 'unowned'|an 'unowned'}1 "
-        "%select{variable|property}2",
-        (Identifier, /*Ownership*/unsigned, /*Is Property*/unsigned))
+        "instance will be immediately deallocated because "
+        "%select{variable|property}2 %0 is %1",
+        (Identifier, ReferenceOwnership, /*Is Property*/unsigned))
 NOTE(unowned_assignment_requires_strong,none,
      "a strong reference is required to prevent the instance from being "
      "deallocated", ())
@@ -3230,24 +3228,23 @@ ERROR(implicitly_unwrapped_optional_in_illegal_position,none,
 
 // Ownership
 ERROR(invalid_ownership_type,none,
-      "'%select{strong|weak|unowned|unowned}0' may only be applied to "
-      "class and class-bound protocol types, not %1",
-      (/*ReferenceOwnership*/unsigned, Type))
+      "%0 may only be applied to class and class-bound protocol types, not %1",
+      (ReferenceOwnership, Type))
 ERROR(invalid_ownership_protocol_type,none,
-      "'%select{strong|weak|unowned|unowned}0' must not be applied to "
-      "non-class-bound %1; consider adding a protocol conformance that has a class bound",
-      (/*ReferenceOwnership*/unsigned, Type))
+      "%0 must not be applied to non-class-bound %1; "
+      "consider adding a protocol conformance that has a class bound",
+      (ReferenceOwnership, Type))
 ERROR(invalid_weak_ownership_not_optional,none,
       "'weak' variable should have optional type %0", (Type))
 ERROR(invalid_weak_let,none,
       "'weak' must be a mutable variable, because it may change at runtime", ())
 ERROR(ownership_invalid_in_protocols,none,
-      "'%select{strong|weak|unowned|unowned}0' cannot be applied to a property declaration in a protocol",
-      (/*ReferenceOwnership*/unsigned))
+      "%0 cannot be applied to a property declaration in a protocol",
+      (ReferenceOwnership))
 WARNING(ownership_invalid_in_protocols_compat_warning,none,
-      "'%select{strong|weak|unowned|unowned}0' should not be applied to a property declaration "
+      "%0 should not be applied to a property declaration "
       "in a protocol and will be disallowed in future versions",
-      (/*ReferenceOwnership*/unsigned))
+      (ReferenceOwnership))
 
 // required
 ERROR(required_initializer_nonclass,none,

include/swift/AST/Ownership.h

@@ -20,6 +20,7 @@
 #define SWIFT_OWNERSHIP_H
 
 #include "swift/Basic/InlineBitfield.h"
+#include "llvm/Support/raw_ostream.h"
 #include <stdint.h>
 
 namespace swift {
@@ -46,6 +47,9 @@ enum class ReferenceOwnership : uint8_t {
 enum : unsigned { NumReferenceOwnershipBits =
   countBitsUsed(static_cast<unsigned>(ReferenceOwnership::Last_Kind)) };
 
+/// Diagnostic printing of \c StaticSpellingKind.
+llvm::raw_ostream &operator<<(llvm::raw_ostream &OS, ReferenceOwnership RO);
+
 /// Different kinds of value ownership supported by Swift.
 enum class ValueOwnership : uint8_t {
   /// \brief the context-dependent default ownership (sometimes shared,

lib/AST/Decl.cpp

@@ -296,6 +296,17 @@ llvm::raw_ostream &swift::operator<<(llvm::raw_ostream &OS,
   llvm_unreachable("bad StaticSpellingKind");
 }
 
+llvm::raw_ostream &swift::operator<<(llvm::raw_ostream &OS,
+                                     ReferenceOwnership RO) {
+  switch (RO) {
+  case ReferenceOwnership::Strong: return OS << "'strong'";
+  case ReferenceOwnership::Weak: return OS << "'weak'";
+  case ReferenceOwnership::Unowned: return OS << "'unowned'";
+  case ReferenceOwnership::Unmanaged: return OS << "'unowned(unsafe)'";
+  }
+  llvm_unreachable("bad ReferenceOwnership kind");
+}
+
 DeclContext *Decl::getInnermostDeclContext() const {
   if (auto func = dyn_cast<AbstractFunctionDecl>(this))
     return const_cast<AbstractFunctionDecl*>(func);

lib/AST/DiagnosticEngine.cpp

@@ -472,6 +472,19 @@ static void formatDiagnosticArgument(StringRef Modifier,
     assert(Modifier.empty() && "Improper modifier for PatternKind argument");
     Out << Arg.getAsPatternKind();
     break;
+
+  case DiagnosticArgumentKind::ReferenceOwnership:
+    if (Modifier == "select") {
+      formatSelectionArgument(ModifierArguments, Args,
+                              unsigned(Arg.getAsReferenceOwnership()),
+                              FormatOpts, Out);
+    } else {
+      assert(Modifier.empty() &&
+             "Improper modifier for ReferenceOwnership argument");
+      Out << Arg.getAsReferenceOwnership();
+    }
+    break;
+
   case DiagnosticArgumentKind::StaticSpellingKind:
     if (Modifier == "select") {
       formatSelectionArgument(ModifierArguments, Args,

lib/Sema/MiscDiagnostics.cpp

@@ -2128,8 +2128,7 @@ static void diagnoseUnownedImmediateDeallocationImpl(TypeChecker &TC,
     storageKind = SK_Property;
 
   TC.diagnose(diagLoc, diag::unowned_assignment_immediate_deallocation,
-              varDecl->getName(),
-              (unsigned) ownershipAttr->get(), (unsigned) storageKind)
+              varDecl->getName(), ownershipAttr->get(), unsigned(storageKind))
     .highlight(diagRange);
 
   TC.diagnose(diagLoc, diag::unowned_assignment_requires_strong)

lib/Sema/TypeCheckAttr.cpp

@@ -2178,21 +2178,21 @@ void TypeChecker::checkReferenceOwnershipAttr(VarDecl *var,
       D = diag::invalid_ownership_protocol_type;
     }
 
-    diagnose(var->getStartLoc(), D, (unsigned) ownershipKind, underlyingType);
+    diagnose(var->getStartLoc(), D, ownershipKind, underlyingType);
     attr->setInvalid();
   } else if (isa<ProtocolDecl>(var->getDeclContext()) &&
              !cast<ProtocolDecl>(var->getDeclContext())->isObjC()) {
     // Ownership does not make sense in protocols, except for "weak" on
     // properties of Objective-C protocols.
     if (Context.isSwiftVersionAtLeast(5))
       diagnose(attr->getLocation(),
-        diag::ownership_invalid_in_protocols,
-        (unsigned) ownershipKind)
+               diag::ownership_invalid_in_protocols,
+               ownershipKind)
         .fixItRemove(attr->getRange());
     else
       diagnose(attr->getLocation(),
-        diag::ownership_invalid_in_protocols_compat_warning,
-        (unsigned) ownershipKind)
+               diag::ownership_invalid_in_protocols_compat_warning,
+               ownershipKind)
         .fixItRemove(attr->getRange());
 
     attr->setInvalid();

lib/Sema/TypeCheckDecl.cpp

@@ -5484,8 +5484,7 @@ class DeclChecker : public DeclVisitor<DeclChecker> {
         parentOwnership = ReferenceOwnership::Strong;
       if (parentOwnership != ownershipAttr->get()) {
         TC.diagnose(decl, diag::override_ownership_mismatch,
-                    (unsigned)parentOwnership,
-                    (unsigned)ownershipAttr->get());
+                    parentOwnership, ownershipAttr->get());
         TC.diagnose(matchDecl, diag::overridden_here);
       }
     }

test/SILOptimizer/definite_init_diagnostics.swift

@@ -111,7 +111,7 @@ func test2() {
   weak var w1 : SomeClass?
   _ = w1                // ok: default-initialized
 
-  // expected-warning@+3 {{instance will be immediately deallocated as 'w2' is a 'weak' variable}}
+  // expected-warning@+3 {{instance will be immediately deallocated because variable 'w2' is 'weak'}}
   // expected-note@+2 {{a strong reference is required to prevent the instance from being deallocated}}
   // expected-note@+1 {{'w2' declared here}}
   weak var w2 = SomeClass()
@@ -124,7 +124,7 @@ func test2() {
   unowned var u1 : SomeClass // expected-note {{variable defined here}}
   _ = u1                // expected-error {{variable 'u1' used before being initialized}}
 
-  // expected-warning@+3 {{instance will be immediately deallocated as 'u2' is an 'unowned' variable}}
+  // expected-warning@+3 {{instance will be immediately deallocated because variable 'u2' is 'unowned'}}
   // expected-note@+2 {{a strong reference is required to prevent the instance from being deallocated}}
   // expected-note@+1 {{'u2' declared here}}
   unowned let u2 = SomeClass()