[Sema] Improve some -Wunguarded-availability diagnostic messages

[erik.pilkington]

Summary

This patch improves the decl-level unguarded availability warnings to use the same messages as the function-level ones. This makes the diagnostic have different parameters than deprecated/unavailable, so I moved some things around in DoEmitAvailabilityWarning so that partial diagnostics are handled separately.

rdar://33543523

Thanks,
Erik

[arphaman]

This needs a test for the fixits as well, see test/FixIt/fixit-availability*

[erik.pilkington]

In D36200#829104, @arphaman wrote:

This needs a test for the fixits as well, see test/FixIt/fixit-availability*

Why? This patch doesn't change the behavior of the fixits, so there isn't any new behavior to test.

[arphaman]

Right, sorry, I didn't realize that code was moved.

[erik.pilkington]

Ping!

[arphaman]

Sorry for the delay, LGTM

include/clang/Basic/DiagnosticSemaKinds.td

   "protocol method is here">;
 
 def warn_unguarded_availability :
   Warning<"%0 is only available on %1 %2 or newer">,
   InGroup<UnguardedAvailability>, DefaultIgnore;
 def warn_unguarded_availability_new :
   Warning<warn_unguarded_availability.Text>,
   InGroup<UnguardedAvailabilityNew>;
-def warn_partial_availability : Warning<"%0 is only available conditionally">,
-    InGroup<UnguardedAvailability>, DefaultIgnore;
-def warn_partial_availability_new : Warning<warn_partial_availability.Text>,
-  InGroup<UnguardedAvailabilityNew>;
-def note_partial_availability_silence : Note<
-  "annotate %select{%1|anonymous %1}0 with an availability attribute to silence">;
+def note_decl_unguarded_availability_silence : Note<
+  "annotate %select{%1|anonymous %1}0 with an availability attribute to silence this warning">;
 def note_unguarded_available_silence : Note<
   "enclose %0 in %select{an @available|a __builtin_available}1 check to silence"
   " this warning">;
-def warn_partial_message : Warning<"%0 is partial: %1">,
-    InGroup<UnguardedAvailability>, DefaultIgnore;
-def warn_partial_message_new : Warning<warn_partial_message.Text>,
-  InGroup<UnguardedAvailabilityNew>;
-def warn_partial_fwdclass_message : Warning<
-    "%0 may be partial because the receiver type is unknown">,
-    InGroup<UnguardedAvailability>, DefaultIgnore;
-def warn_partial_fwdclass_message_new :
-  Warning<warn_partial_fwdclass_message.Text>,
-  InGroup<UnguardedAvailabilityNew>;
 def warn_at_available_unchecked_use : Warning<
   "%select{@available|__builtin_available}0 does not guard availability here; "
   "use if (%select{@available|__builtin_available}0) instead">,
   InGroup<DiagGroup<"unsupported-availability-guard">>;
 
 // Thread Safety Attributes
 def warn_invalid_capability_name : Warning<
   "invalid capability name '%0'; capability name must be 'mutex' or 'role'">,

lib/Sema/SemaDeclAttr.cpp

 
   VersionTuple DeclVersion;
   if (const AvailabilityAttr *AA = getAttrForPlatform(S.Context, OffendingDecl))
     DeclVersion = AA->getIntroduced();
 
   if (!ShouldDiagnoseAvailabilityInContext(S, K, DeclVersion, Ctx))
     return;
 
+  // The declaration can have multiple availability attributes, we are looking
+  // at one of them.
+  const AvailabilityAttr *A = getAttrForPlatform(S.Context, OffendingDecl);
+  if (A && A->isInherited()) {
+    for (const Decl *Redecl = OffendingDecl->getMostRecentDecl(); Redecl;
+         Redecl = Redecl->getPreviousDecl()) {
+      const AvailabilityAttr *AForRedecl =
+          getAttrForPlatform(S.Context, Redecl);
+      if (AForRedecl && !AForRedecl->isInherited()) {
+        // If D is a declaration with inherited attributes, the note should
+        // point to the declaration with actual attributes.
+        NoteLocation = Redecl->getLocation();
+        break;
+      }
+    }
+  }
+
   switch (K) {
+  case AR_NotYetIntroduced: {
+    // We would like to emit the diagnostic even if -Wunguarded-availability is
+    // not specified for deployment targets >= to iOS 11 or equivalent or
+    // for declarations that were introduced in iOS 11 (macOS 10.13, ...) or
+    // later.
+    const AvailabilityAttr *AA =
+        getAttrForPlatform(S.getASTContext(), OffendingDecl);
+    VersionTuple Introduced = AA->getIntroduced();
+
+    bool UseNewWarning = shouldDiagnoseAvailabilityByDefault(
+        S.Context, S.Context.getTargetInfo().getPlatformMinVersion(),
+        Introduced);
+    unsigned Warning = UseNewWarning ? diag::warn_unguarded_availability_new
+                                     : diag::warn_unguarded_availability;
+
+    S.Diag(Loc, Warning)
+        << OffendingDecl
+        << AvailabilityAttr::getPrettyPlatformName(
+               S.getASTContext().getTargetInfo().getPlatformName())
+        << Introduced.getAsString();
+
+    S.Diag(OffendingDecl->getLocation(), diag::note_availability_specified_here)
+        << OffendingDecl << /* partial */ 3;
+
+    if (const auto *Enclosing = findEnclosingDeclToAnnotate(Ctx)) {
+      if (auto *TD = dyn_cast<TagDecl>(Enclosing))
+        if (TD->getDeclName().isEmpty()) {
+          S.Diag(TD->getLocation(),
+                 diag::note_decl_unguarded_availability_silence)
+              << /*Anonymous*/ 1 << TD->getKindName();
+          return;
+        }
+      auto FixitNoteDiag =
+          S.Diag(Enclosing->getLocation(),
+                 diag::note_decl_unguarded_availability_silence)
+          << /*Named*/ 0 << Enclosing;
+      // Don't offer a fixit for declarations with availability attributes.
+      if (Enclosing->hasAttr<AvailabilityAttr>())
+        return;
+      if (!S.getPreprocessor().isMacroDefined("API_AVAILABLE"))
+        return;
+      Optional<AttributeInsertion> Insertion = createAttributeInsertion(
+          Enclosing, S.getSourceManager(), S.getLangOpts());
+      if (!Insertion)
+        return;
+      std::string PlatformName =
+          AvailabilityAttr::getPlatformNameSourceSpelling(
+              S.getASTContext().getTargetInfo().getPlatformName())
+              .lower();
+      std::string Introduced =
+          OffendingDecl->getVersionIntroduced().getAsString();
+      FixitNoteDiag << FixItHint::CreateInsertion(
+          Insertion->Loc,
+          (llvm::Twine(Insertion->Prefix) + "API_AVAILABLE(" + PlatformName +
+           "(" + Introduced + "))" + Insertion->Suffix)
+              .str());
+    }
+    return;
+  }
   case AR_Deprecated:
     diag = !ObjCPropertyAccess ? diag::warn_deprecated
                                : diag::warn_property_method_deprecated;
     diag_message = diag::warn_deprecated_message;
     diag_fwdclass_message = diag::warn_deprecated_fwdclass_message;
     property_note_select = /* deprecated */ 0;
     available_here_select_kind = /* deprecated */ 2;
     if (const auto *attr = OffendingDecl->getAttr<DeprecatedAttr>())

           flagARCError();
           diag_available_here = diag::note_arc_field_with_ownership;
           break;
         }
       }
     }
     break;
 
-  case AR_NotYetIntroduced: {
-    // We would like to emit the diagnostic even if -Wunguarded-availability is
-    // not specified for deployment targets >= to iOS 11 or equivalent or
-    // for declarations that were introduced in iOS 11 (macOS 10.13, ...) or
-    // later.
-    const AvailabilityAttr *AA =
-        getAttrForPlatform(S.getASTContext(), OffendingDecl);
-    VersionTuple Introduced = AA->getIntroduced();
-    bool NewWarning = shouldDiagnoseAvailabilityByDefault(
-        S.Context, S.Context.getTargetInfo().getPlatformMinVersion(),
-        Introduced);
-    diag = NewWarning ? diag::warn_partial_availability_new
-                      : diag::warn_partial_availability;
-    diag_message = NewWarning ? diag::warn_partial_message_new
-                              : diag::warn_partial_message;
-    diag_fwdclass_message = NewWarning ? diag::warn_partial_fwdclass_message_new
-                                       : diag::warn_partial_fwdclass_message;
-    property_note_select = /* partial */ 2;
-    available_here_select_kind = /* partial */ 3;
-    break;
-  }
-
   case AR_Available:
     llvm_unreachable("Warning for availability of available declaration?");
   }
 
   CharSourceRange UseRange;
   StringRef Replacement;
   if (K == AR_Deprecated) {
     if (auto attr = OffendingDecl->getAttr<DeprecatedAttr>())

           << ObjCProperty->getDeclName() << property_note_select;
   } else {
     S.Diag(Loc, diag_fwdclass_message) << ReferringDecl
       << (UseRange.isValid() ?
           FixItHint::CreateReplacement(UseRange, Replacement) : FixItHint());
     S.Diag(UnknownObjCClass->getLocation(), diag::note_forward_class);
   }
 
-  // The declaration can have multiple availability attributes, we are looking
-  // at one of them.
-  const AvailabilityAttr *A = getAttrForPlatform(S.Context, OffendingDecl);
-  if (A && A->isInherited()) {
-    for (const Decl *Redecl = OffendingDecl->getMostRecentDecl(); Redecl;
-         Redecl = Redecl->getPreviousDecl()) {
-      const AvailabilityAttr *AForRedecl = getAttrForPlatform(S.Context,
-                                                              Redecl);
-      if (AForRedecl && !AForRedecl->isInherited()) {
-        // If D is a declaration with inherited attributes, the note should
-        // point to the declaration with actual attributes.
-        S.Diag(Redecl->getLocation(), diag_available_here) << OffendingDecl
-            << available_here_select_kind;
-        break;
-      }
-    }
-  }
-  else
-    S.Diag(NoteLocation, diag_available_here)
-        << OffendingDecl << available_here_select_kind;
-
-  if (K == AR_NotYetIntroduced)
-    if (const auto *Enclosing = findEnclosingDeclToAnnotate(Ctx)) {
-      if (auto *TD = dyn_cast<TagDecl>(Enclosing))
-        if (TD->getDeclName().isEmpty()) {
-          S.Diag(TD->getLocation(), diag::note_partial_availability_silence)
-              << /*Anonymous*/1 << TD->getKindName();
-          return;
-        }
-      auto FixitNoteDiag = S.Diag(Enclosing->getLocation(),
-                                  diag::note_partial_availability_silence)
-                           << /*Named*/ 0 << Enclosing;
-      // Don't offer a fixit for declarations with availability attributes.
-      if (Enclosing->hasAttr<AvailabilityAttr>())
-        return;
-      if (!S.getPreprocessor().isMacroDefined("API_AVAILABLE"))
-        return;
-      Optional<AttributeInsertion> Insertion = createAttributeInsertion(
-          Enclosing, S.getSourceManager(), S.getLangOpts());
-      if (!Insertion)
-        return;
-      std::string PlatformName =
-          AvailabilityAttr::getPlatformNameSourceSpelling(
-              S.getASTContext().getTargetInfo().getPlatformName())
-              .lower();
-      std::string Introduced =
-          OffendingDecl->getVersionIntroduced().getAsString();
-      FixitNoteDiag << FixItHint::CreateInsertion(
-          Insertion->Loc,
-          (llvm::Twine(Insertion->Prefix) + "API_AVAILABLE(" + PlatformName +
-           "(" + Introduced + "))" + Insertion->Suffix)
-              .str());
-    }
+  S.Diag(NoteLocation, diag_available_here)
+    << OffendingDecl << available_here_select_kind;
 }
 
 static void handleDelayedAvailabilityCheck(Sema &S, DelayedDiagnostic &DD,
                                            Decl *Ctx) {
   assert(DD.Kind == DelayedDiagnostic::Availability &&
          "Expected an availability diagnostic here");
 
   DD.Triggered = true;

test/SemaObjC/attr-availability.m

 
 #if defined(WARN_PARTIAL)
   // expected-note@+2 2 {{'PartialI2' has been explicitly marked partial here}}
 #endif
 __attribute__((availability(macosx, introduced = 10.8))) @interface PartialI2
 @end
 
 #if defined(WARN_PARTIAL)
-// expected-warning@+2 {{'PartialI2' is partial: introduced in macOS 10.8}} expected-note@+2 {{annotate 'partialinter1' with an availability attribute to silence}}
+// expected-warning@+2 {{'PartialI2' is only available on macOS 10.8 or newer}} expected-note@+2 {{annotate 'partialinter1' with an availability attribute to silence}}
 #endif
 void partialinter1(PartialI2* p) {
 }
 
 @class PartialI2;
 
 #ifdef WARN_PARTIAL
-// expected-warning@+2 {{'PartialI2' is partial: introduced in macOS 10.8}} expected-note@+2 {{annotate 'partialinter2' with an availability attribute to silence}}
+// expected-warning@+2 {{'PartialI2' is only available on macOS 10.8 or newer}} expected-note@+2 {{annotate 'partialinter2' with an availability attribute to silence}}
 #endif
 void partialinter2(PartialI2* p) {
 }
 
 
 // Test that both the use of the 'typedef' and the enum constant
 // produces an error. rdar://problem/20903588
 #define UNAVAILABLE __attribute__((unavailable("not available")))

test/SemaObjC/unguarded-availability-new.m

 
 typedef int AVAILABLE_NEXT new_int;
 #ifndef NO_WARNING
   // expected-note@-2 {{'new_int' has been explicitly marked partial here}}
 #endif
 FUNC_AVAILABLE new_int x;
 #ifndef NO_WARNING
 #ifdef MAC
-  // expected-warning@-3 {{'new_int' is partial: introduced in macOS 10.14}} expected-note@-3 {{annotate 'x' with an availability attribute to silence}}
+  // expected-warning@-3 {{'new_int' is only available on macOS 10.14 or newer}} expected-note@-3 {{annotate 'x' with an availability attribute to silence this warning}}
 #endif
 #ifdef IOS
-  // expected-warning@-6 {{'new_int' is partial: introduced in iOS 12}} expected-note@-6 {{annotate 'x' with an availability attribute to silence}}
+  // expected-warning@-6 {{'new_int' is only available on iOS 12 or newer}} expected-note@-6 {{annotate 'x' with an availability attribute to silence this warning}}
 #endif
 #ifdef TVOS
-  // expected-warning@-9 {{'new_int' is partial: introduced in tvOS 13}} expected-note@-9 {{annotate 'x' with an availability attribute to silence}}
+  // expected-warning@-9 {{'new_int' is only available on tvOS 13 or newer}} expected-note@-9 {{annotate 'x' with an availability attribute to silence this warning}}
 #endif
 #ifdef WATCHOS
-  // expected-warning@-12 {{'new_int' is partial: introduced in watchOS 5}} expected-note@-12 {{annotate 'x' with an availability attribute to silence}}
+  // expected-warning@-12 {{'new_int' is only available on watchOS 5}} expected-note@-12 {{annotate 'x' with an availability attribute to silence this warning}}
 #endif
 #endif
 
 void test() FUNC_AVAILABLE {
   previouslyAvailable();
 #ifdef WARN_PREV
 #ifdef MAC
   // expected-warning@-3 {{'previouslyAvailable' is only available on macOS 10.12 or newer}}

test/SemaObjC/unguarded-availability.m

 
 void use_typedef() {
   int_10_11 x; // expected-warning{{'int_10_11' is only available on macOS 10.11 or newer}} expected-note{{enclose 'int_10_11' in an @available check to silence this warning}}
 }
 
 __attribute__((objc_root_class))
 AVAILABLE_10_11 @interface Class_10_11 { // expected-note{{annotate 'Class_10_11' with an availability attribute to silence}}
   int_10_11 foo;
-  int_10_12 bar; // expected-warning {{'int_10_12' is partial: introduced in macOS 10.12}}
+  int_10_12 bar; // expected-warning {{'int_10_12' is only available on macOS 10.12 or newer}}
 }
 - (void)method1;
 - (void)method2;
 @end
 
 @implementation Class_10_11
 - (void) method1 {
   func_10_11();

 }
 
 void test_blocks() {
   (void) ^{
     func_10_12(); // expected-warning{{'func_10_12' is only available on macOS 10.12 or newer}} expected-note{{@available}}
   };
 }
 
-void test_params(int_10_12 x); // expected-warning {{'int_10_12' is partial: introduced in macOS 10.12}} expected-note{{annotate 'test_params' with an availability attribute to silence}}
+void test_params(int_10_12 x); // expected-warning {{'int_10_12' is only available on macOS 10.12 or newer}} expected-note{{annotate 'test_params' with an availability attribute to silence this warning}}
 
 void test_params2(int_10_12 x) AVAILABLE_10_12; // no warn
 
 void (^topLevelBlockDecl)() = ^ {
   func_10_12(); // expected-warning{{'func_10_12' is only available on macOS 10.12 or newer}} expected-note{{@available}}
   if (@available(macos 10.12, *))
     func_10_12();
 };
void functionInFunction() {
   (void)(^ {
     func_10_12(); // expected-warning{{'func_10_12' is only available on macOS 10.12 or newer}} expected-note{{@available}}
   });
 }
 
 #endif
 
 struct InStruct { // expected-note{{annotate 'InStruct' with an availability attribute to silence}}
-  new_int mem; // expected-warning{{'new_int' is partial}}
+  new_int mem; // expected-warning{{'new_int' is only available on macOS 10.12 or newer}}
 
-  struct { new_int mem; } anon; // expected-warning{{'new_int' is partial}} expected-note{{annotate anonymous struct with an availability attribute}}
+  struct { new_int mem; } anon; // expected-warning{{'new_int' is only available on macOS 10.12 or newer}} expected-note{{annotate anonymous struct with an availability attribute to silence}}
 };
 
 #ifdef OBJCPP
 static constexpr int AVAILABLE_10_12 SomeConstexprValue = 2; // expected-note{{marked partial here}}
 typedef enum { // expected-note{{annotate anonymous enum with an availability attribute}}
-  SomeValue = SomeConstexprValue // expected-warning{{'SomeConstexprValue' is partial}} 
+  SomeValue = SomeConstexprValue // expected-warning{{'SomeConstexprValue' is only available on macOS 10.12 or newer}} 
 } SomeEnum;
 #endif
 
 @interface InInterface
--(new_int)meth; // expected-warning{{'new_int' is partial}} expected-note{{annotate 'meth' with an availability attribute}}
+-(new_int)meth; // expected-warning{{'new_int' is only available on macOS 10.12 or newer}} expected-note{{annotate 'meth' with an availability attribute}}
 @end
 
 @interface Proper // expected-note{{annotate 'Proper' with an availability attribute}}
-@property (class) new_int x; // expected-warning{{'new_int' is partial}}
+@property (class) new_int x; // expected-warning{{'new_int' is only available}}
 @end
 
 void with_local_struct() {
   struct local { // expected-note{{annotate 'local' with an availability attribute}}
-    new_int x; // expected-warning{{'new_int' is partial}}
+    new_int x; // expected-warning{{'new_int' is only available}}
   };
 }
 
 // rdar://33156429:
 // Avoid the warning on protocol requirements.
 
 AVAILABLE_10_12
 @protocol NewProtocol // expected-note {{'NewProtocol' has been explicitly marked partial here}}
 @end
 
 @protocol ProtocolWithNewProtocolRequirement <NewProtocol> // expected-note {{annotate 'ProtocolWithNewProtocolRequirement' with an availability attribute to silence}}
 
-@property(copy) id<NewProtocol> prop; // expected-warning {{'NewProtocol' is partial: introduced in macOS 10.12}}
+@property(copy) id<NewProtocol> prop; // expected-warning {{'NewProtocol' is only available on macOS 10.12 or newer}}
 
 @end
 
 @interface BaseClass
 @end
 
 @interface ClassWithNewProtocolRequirement : BaseClass <NewProtocol>
 
 @end
 
 @interface BaseClass (CategoryWithNewProtocolRequirement) <NewProtocol>
 
 @end