Introduce -Wunguarded-availability-new, which is like -Wunguarded-availability, except that it's enabled by default for new deployment targets

[arphaman]

Summary

This patch adds a new warning flag called -Wunguarded-availability-new. If -Wunguarded-availability is off, this warning only warns about uses of APIs that have been introduced in macOS >= 10.13, iOS >= 11, watchOS >= 4 and tvOS >= 11. This warning is on by default. We decided to use this kind of solution as we didn't want to turn on -Wunguarded-availability by default, as we didn't want our users to get warnings about uses of old APIs in their existing projects.

[erik.pilkington]

lib/Sema/SemaDeclAttr.cpp

7315 ↗

(On Diff #102766)

This assert seems a bit redundant, no?

7350 ↗

(On Diff #102766)

There is a version of this for decls that aren't referenced in a function, such as:

typedef int __attribute__((availability(macos, introducced=1000))) new_int;
new_int x; // warn

// Also:
struct S {
  new_int x; // warn
};

-Wunguarded-availability-new should also work with this, right? If we do want to support that, maybe we should add this check to ShouldDiagnoseAvailabilityOfDecl(), which is called by both paths.

[arphaman]

Remove the assert and support the other partial availability warnings.

[erik.pilkington]

lib/Sema/SemaDeclAttr.cpp

6944 ↗

(On Diff #102891)

Why did you remove the AR_Unavailable checking?

7021 ↗

(On Diff #102891)

I think checking Diags::isIgnored is fairly expensive, maybe we should swap the operands to the '&&' so that it is only done if necessary?

[arphaman]

lib/Sema/SemaDeclAttr.cpp

6944 ↗

(On Diff #102891)

Ooops, that was a mistake

[arphaman]

Swap the checking operators and bring back the accidentally deleted code.

[erik.pilkington]

lib/Sema/SemaDeclAttr.cpp

7031 ↗

(On Diff #102902)

Sorry to keep this going so long, but why are we even checking isIgnored? The only difference it could make in whether we emit a diagnostic is if both: -Wunguarded-availability and -Wno-unguarded-availability-new are passed in, which seems like it would never happen, right? Even if somebody did pass that in, it seems reasonable to warn on old stuff but not new stuff. Maybe I'm missing something here?

[arphaman]

lib/Sema/SemaDeclAttr.cpp

7031 ↗

(On Diff #102902)

Right, it's to handle the -Wunguarded-availability -Wno-unguarded-availability-new case. Your argument makes sense though, we could allow -Wunguarded-availability -Wno-unguarded-availability-new where we warn on old APIs. Although that still seems kinda weird to me. Maybe @dexonsmith has an opinion about this?

[dexonsmith]

lib/Sema/SemaDeclAttr.cpp

7031 ↗

(On Diff #102902)

I don't think the exact behaviour of -Wunguarded-availability -Wno-unguarded-availability-new is terribly important. I'm fine either way.

But, having a predictable command-line interface is nice, and I'd expect that combination to show diagnostics only for old APIs.

[arphaman]

Update the logic for -Wunguarded-availability -Wno-unguarded-availability-new so that it only warns for the old APIs.

[erik.pilkington]

LGTM, thanks for working on this!

cfe/trunk/include/clang/Basic/DiagnosticGroups.td

 def AbstractFinalClass : DiagGroup<"abstract-final-class">;
 
 def CXX11CompatDeprecatedWritableStr :
   DiagGroup<"c++11-compat-deprecated-writable-strings">;
 
 def DeprecatedAttributes : DiagGroup<"deprecated-attributes">;
 def DeprecatedDeclarations : DiagGroup<"deprecated-declarations">;
 def UnavailableDeclarations : DiagGroup<"unavailable-declarations">;
-def UnguardedAvailability : DiagGroup<"unguarded-availability">;
+def UnguardedAvailabilityNew : DiagGroup<"unguarded-availability-new">;
+def UnguardedAvailability : DiagGroup<"unguarded-availability",
+                                      [UnguardedAvailabilityNew]>;
 // partial-availability is an alias of unguarded-availability.
 def : DiagGroup<"partial-availability", [UnguardedAvailability]>;
 def DeprecatedDynamicExceptionSpec
     : DiagGroup<"deprecated-dynamic-exception-spec">;
 def DeprecatedImplementations :DiagGroup<"deprecated-implementations">;
 def DeprecatedIncrementBool : DiagGroup<"deprecated-increment-bool">;
 def DeprecatedRegister : DiagGroup<"deprecated-register">;
 def DeprecatedWritableStr : DiagGroup<"deprecated-writable-strings",

cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td

 def note_overridden_method : Note<
   "overridden method is here">;
 def note_protocol_method : Note<
   "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<
   "explicitly redeclare %0 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'">,

cfe/trunk/lib/Sema/SemaDeclAttr.cpp

else if (auto *CatD = dyn_cast<ObjCCategoryDecl>(Ctx))
       if (const ObjCInterfaceDecl *Interface = CatD->getClassInterface())
         if (CheckContext(Interface))
           return false;
   } while ((Ctx = cast_or_null<Decl>(Ctx->getDeclContext())));
 
   return true;
 }
 
+static bool
+shouldDiagnoseAvailabilityByDefault(const ASTContext &Context,
+                                    const VersionTuple &DeploymentVersion,
+                                    const VersionTuple &DeclVersion) {
+  const auto &Triple = Context.getTargetInfo().getTriple();
+  VersionTuple ForceAvailabilityFromVersion;
+  switch (Triple.getOS()) {
+  case llvm::Triple::IOS:
+  case llvm::Triple::TvOS:
+    ForceAvailabilityFromVersion = VersionTuple(/*Major=*/11);
+    break;
+  case llvm::Triple::WatchOS:
+    ForceAvailabilityFromVersion = VersionTuple(/*Major=*/4);
+    break;
+  case llvm::Triple::Darwin:
+  case llvm::Triple::MacOSX:
+    ForceAvailabilityFromVersion = VersionTuple(/*Major=*/10, /*Minor=*/13);
+    break;
+  default:
+    // New targets should always warn about availability.
+    return Triple.getVendor() == llvm::Triple::Apple;
+  }
+  return DeploymentVersion >= ForceAvailabilityFromVersion ||
+         DeclVersion >= ForceAvailabilityFromVersion;
+}
+
 static void DoEmitAvailabilityWarning(Sema &S, AvailabilityResult K,
                                       Decl *Ctx, const NamedDecl *D,
                                       StringRef Message, SourceLocation Loc,
                                       const ObjCInterfaceDecl *UnknownObjCClass,
                                       const ObjCPropertyDecl *ObjCProperty,
                                       bool ObjCPropertyAccess) {
   // Diagnostics for deprecated or unavailable.
   unsigned diag, diag_message, diag_fwdclass_message;
if (auto attr = D->getAttr<UnavailableAttr>()) {
           flagARCError();
           diag_available_here = diag::note_arc_field_with_ownership;
           break;
         }
       }
     }
     break;
 
-  case AR_NotYetIntroduced:
-    diag = diag::warn_partial_availability;
-    diag_message = diag::warn_partial_message;
-    diag_fwdclass_message = diag::warn_partial_fwdclass_message;
+  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(), D);
+    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 (AvailabilityResult Result =
     if (ContextVersion >= Introduced)
       return;
 
     // If the context of this function is less available than D, we should not
     // emit a diagnostic.
     if (!ShouldDiagnoseAvailabilityInContext(SemaRef, Result, Introduced, Ctx))
       return;
 
-    SemaRef.Diag(Range.getBegin(), diag::warn_unguarded_availability)
+    // 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.
+    unsigned DiagKind =
+        shouldDiagnoseAvailabilityByDefault(
+            SemaRef.Context,
+            SemaRef.Context.getTargetInfo().getPlatformMinVersion(), Introduced)
+            ? diag::warn_unguarded_availability_new
+            : diag::warn_unguarded_availability;
+
+    SemaRef.Diag(Range.getBegin(), DiagKind)
         << Range << D
         << AvailabilityAttr::getPrettyPlatformName(
                SemaRef.getASTContext().getTargetInfo().getPlatformName())
         << Introduced.getAsString();
 
     SemaRef.Diag(D->getLocation(), diag::note_availability_specified_here)
         << D << /* partial */ 3;
 

cfe/trunk/test/SemaObjC/unguarded-availability-new.m

+// RUN: %clang_cc1 -DMAC -triple x86_64-apple-macosx10.13 -fblocks -fsyntax-only -verify %s
+// RUN: %clang_cc1 -xobjective-c++ -DMAC -triple x86_64-apple-macosx10.13 -fblocks -fsyntax-only -verify %s
+
+// RUN: %clang_cc1 -DMAC -triple x86_64-apple-macosx10.13 -Wunguarded-availability-new -fblocks -fsyntax-only -verify %s
+// RUN: %clang_cc1 -DMAC -triple x86_64-apple-macosx10.13 -Wno-unguarded-availability-new -DNO_WARNING -fblocks -fsyntax-only -verify %s
+
+// unguarded-availability implies unguarded-availability-new:
+// RUN: %clang_cc1 -DMAC -triple x86_64-apple-macosx10.13 -Wunguarded-availability -fblocks -fsyntax-only -verify %s
+// RUN: %clang_cc1 -DMAC -triple x86_64-apple-macosx10.11 -Wunguarded-availability -Wno-unguarded-availability-new -DNO_WARNING -DWARN_PREV -fblocks -fsyntax-only -verify %s
+// RUN: %clang_cc1 -DMAC -triple x86_64-apple-macosx10.13 -Wno-unguarded-availability -DNO_WARNING  -fblocks -fsyntax-only -verify %s
+// RUN: %clang_cc1 -DMAC -triple x86_64-apple-macosx10.13 -Wno-unguarded-availability -Wunguarded-availability-new -fblocks -fsyntax-only -verify %s
+
+// RUN: %clang_cc1 -DMAC -triple x86_64-apple-macosx10.13 -D TEST_FUNC_CURRENT -fblocks -fsyntax-only -verify %s
+// RUN: %clang_cc1 -DMAC -triple x86_64-apple-macosx10.13 -D TEST_FUNC_NEXT -DNO_WARNING -fblocks -fsyntax-only -verify %s
+// RUN: %clang_cc1 -DMAC -triple x86_64-apple-ios11 -DNO_WARNING -fblocks -fsyntax-only -verify %s
+// RUN: %clang_cc1 -DMAC -triple x86_64-apple-macosx10.12 -DWARN_CURRENT -fblocks -fsyntax-only -verify %s
+
+// RUN: %clang_cc1 -DIOS -triple x86_64-apple-ios11 -fblocks -fsyntax-only -verify %s
+// RUN: %clang_cc1 -DIOS -triple x86_64-apple-ios11 -D TEST_FUNC_CURRENT -fblocks -fsyntax-only -verify %s
+// RUN: %clang_cc1 -DIOS -triple x86_64-apple-ios11 -D TEST_FUNC_NEXT -DNO_WARNING -fblocks -fsyntax-only -verify %s
+// RUN: %clang_cc1 -DIOS -triple x86_64-apple-ios10.3 -DWARN_CURRENT -fblocks -fsyntax-only -verify %s
+
+// RUN: %clang_cc1 -DTVOS -triple x86_64-apple-tvos11 -fblocks -fsyntax-only -verify %s
+// RUN: %clang_cc1 -DTVOS -triple x86_64-apple-tvos11 -D TEST_FUNC_CURRENT -fblocks -fsyntax-only -verify %s
+// RUN: %clang_cc1 -DTVOS -triple x86_64-apple-tvos11 -D TEST_FUNC_NEXT -DNO_WARNING -fblocks -fsyntax-only -verify %s
+// RUN: %clang_cc1 -DTVOS -triple x86_64-apple-tvos10 -DWARN_CURRENT -fblocks -fsyntax-only -verify %s
+
+// RUN: %clang_cc1 -DWATCHOS -triple i386-apple-watchos4 -fblocks -fsyntax-only -verify %s
+// RUN: %clang_cc1 -DWATCHOS -triple i386-apple-watchos4 -D TEST_FUNC_CURRENT -fblocks -fsyntax-only -verify %s
+// RUN: %clang_cc1 -DWATCHOS -triple i386-apple-watchos4 -D TEST_FUNC_NEXT -DNO_WARNING -fblocks -fsyntax-only -verify %s
+// RUN: %clang_cc1 -DWATCHOS -triple i386-apple-watchos3 -DWARN_CURRENT -fblocks -fsyntax-only -verify %s
+
+#ifdef MAC
+#define PLATFORM macos
+#define NEXT 10.14
+
+#define AVAILABLE_PREV __attribute__((availability(macos, introduced = 10.12)))
+#define AVAILABLE_CURRENT __attribute__((availability(macos, introduced = 10.13)))
+#define AVAILABLE_NEXT __attribute__((availability(macos, introduced = 10.14)))
+#endif
+
+#ifdef IOS
+#define PLATFORM ios
+#define NEXT 12
+
+#define AVAILABLE_PREV __attribute__((availability(ios, introduced = 10)))
+#define AVAILABLE_CURRENT __attribute__((availability(ios, introduced = 11)))
+#define AVAILABLE_NEXT __attribute__((availability(ios, introduced = 12)))
+#endif
+
+#ifdef TVOS
+#define PLATFORM tvos
+#define NEXT 13
+
+#define AVAILABLE_PREV __attribute__((availability(tvos, introduced = 10)))
+#define AVAILABLE_CURRENT __attribute__((availability(tvos, introduced = 11)))
+#define AVAILABLE_NEXT __attribute__((availability(tvos, introduced = 13)))
+#endif
+
+#ifdef WATCHOS
+#define PLATFORM watchos
+#define NEXT 5
+
+#define AVAILABLE_PREV __attribute__((availability(watchos, introduced = 3)))
+#define AVAILABLE_CURRENT __attribute__((availability(watchos, introduced = 4)))
+#define AVAILABLE_NEXT __attribute__((availability(watchos, introduced = 5)))
+#endif
+
+void previouslyAvailable() AVAILABLE_PREV;
+#ifdef WARN_PREV
+  // expected-note@-2 {{'previouslyAvailable' has been explicitly marked partial here}}
+#endif
+void currentlyAvailable() AVAILABLE_CURRENT;
+#ifdef WARN_CURRENT
+  // expected-note@-2 {{'currentlyAvailable' has been explicitly marked partial here}}
+#endif
+void willBeAvailabile() AVAILABLE_NEXT;
+#ifndef NO_WARNING
+  // expected-note@-2 {{'willBeAvailabile' has been explicitly marked partial here}}
+#endif
+
+#ifdef TEST_FUNC_CURRENT
+#define FUNC_AVAILABLE AVAILABLE_CURRENT
+#endif
+#ifdef TEST_FUNC_NEXT
+#define FUNC_AVAILABLE AVAILABLE_NEXT
+#endif
+#ifndef FUNC_AVAILABLE
+#define FUNC_AVAILABLE
+#endif
+
+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 {{explicitly redeclare 'new_int' to silence this warning}}
+#endif
+#ifdef IOS
+  // expected-warning@-6 {{'new_int' is partial: introduced in iOS 12}} expected-note@-6 {{explicitly redeclare 'new_int' to silence this warning}}
+#endif
+#ifdef TVOS
+  // expected-warning@-9 {{'new_int' is partial: introduced in tvOS 13}} expected-note@-9 {{explicitly redeclare 'new_int' to silence this warning}}
+#endif
+#ifdef WATCHOS
+  // expected-warning@-12 {{'new_int' is partial: introduced in watchOS 5}} expected-note@-12 {{explicitly redeclare 'new_int' 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}}
+#endif
+  // expected-note@-5 {{enclose 'previouslyAvailable' in an @available check to silence this warning}}
+#endif
+  currentlyAvailable();
+#ifdef WARN_CURRENT
+#ifdef MAC
+  // expected-warning@-3 {{'currentlyAvailable' is only available on macOS 10.13 or newer}}
+#endif
+#ifdef IOS
+  // expected-warning@-6 {{'currentlyAvailable' is only available on iOS 11 or newer}}
+#endif
+#ifdef TVOS
+  // expected-warning@-9 {{'currentlyAvailable' is only available on tvOS 11 or newer}}
+#endif
+#ifdef WATCHOS
+  // expected-warning@-12 {{'currentlyAvailable' is only available on watchOS 4 or newer}}
+#endif
+  // expected-note@-14 {{enclose 'currentlyAvailable' in an @available check to silence this warning}}
+#endif
+  willBeAvailabile();
+#ifndef NO_WARNING
+#ifdef MAC
+  // expected-warning@-3 {{'willBeAvailabile' is only available on macOS 10.14 or newer}}
+#endif
+#ifdef IOS
+  // expected-warning@-6 {{'willBeAvailabile' is only available on iOS 12 or newer}}
+#endif
+#ifdef TVOS
+  // expected-warning@-9 {{'willBeAvailabile' is only available on tvOS 13 or newer}}
+#endif
+#ifdef WATCHOS
+  // expected-warning@-12 {{'willBeAvailabile' is only available on watchOS 5 or newer}}
+#endif
+  // expected-note@-14 {{enclose 'willBeAvailabile' in an @available check to silence this warning}}
+#endif
+  if (@available(PLATFORM NEXT, *))
+    willBeAvailabile(); // OK
+}
+
+#ifdef NO_WARNING
+#ifndef WARN_PREV
+// expected-no-diagnostics
+#endif
+#endif