Warn about uses of `@available` that can't suppress the -Wunguarded-availability warnings

[arphaman]

Summary

We should warn about uses of @available in all places except for if (@available(...)) because we only support the guarded availability checking using if (@available(...)).

[erik.pilkington]

include/clang/Basic/DiagnosticSemaKinds.td

2889 ↗

(On Diff #99948)

Might be a bit more clear if you mention in words that @available can only be the sole condition to the if? This is probably fine though.

lib/Sema/SemaExpr.cpp

15747 ↗

(On Diff #99948)

I believe this will not be set if we're rebuilding a templated function, we should also flag this from TransformObjCAvailabilityCheckExpr in TreeTransform.h.

[erik.pilkington]

LGTM, thanks!

lib/Sema/SemaExpr.cpp

15747 ↗

(On Diff #99948)

On second thought, there is no point in doing that because we'd have already emitted a diagnostic from the pattern. This is correct as is.

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

 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_fwdclass_message : Warning<
     "%0 may be partial because the receiver type is unknown">,
     InGroup<UnguardedAvailability>, DefaultIgnore;
+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'">,
   InGroup<ThreadSafetyAttributes>, DefaultIgnore;
 def warn_thread_attribute_ignored : Warning<
   "ignoring %0 attribute because its argument is invalid">,
   InGroup<ThreadSafetyAttributes>, DefaultIgnore;

cfe/trunk/lib/Sema/SemaDeclAttr.cpp

public:
   }
 
   bool VisitMemberExpr(MemberExpr *ME) {
     DiagnoseDeclAvailability(ME->getMemberDecl(),
                              SourceRange(ME->getLocStart(), ME->getLocEnd()));
     return true;
   }
 
+  bool VisitObjCAvailabilityCheckExpr(ObjCAvailabilityCheckExpr *E) {
+    SemaRef.Diag(E->getLocStart(), diag::warn_at_available_unchecked_use)
+        << (!SemaRef.getLangOpts().ObjC1);
+    return true;
+  }
+
   bool VisitTypeLoc(TypeLoc Ty);
 };
 
 void DiagnoseUnguardedAvailability::DiagnoseDeclAvailability(
     NamedDecl *D, SourceRange Range) {
 
   VersionTuple ContextVersion = AvailabilityStack.back();
   if (AvailabilityResult Result =

cfe/trunk/lib/Sema/SemaExpr.cpp

auto Spec = std::find_if(AvailSpecs.begin(), AvailSpecs.end(),
                            [&](const AvailabilitySpec &Spec) {
                              return Spec.getPlatform() == Platform;
                            });
 
   VersionTuple Version;
   if (Spec != AvailSpecs.end())
     Version = Spec->getVersion();
 
+  // The use of `@available` in the enclosing function should be analyzed to
+  // warn when it's used inappropriately (i.e. not if(@available)).
+  if (getCurFunctionOrMethodDecl())
+    getEnclosingFunction()->HasPotentialAvailabilityViolations = true;
+  else if (getCurBlock() || getCurLambda())
+    getCurFunction()->HasPotentialAvailabilityViolations = true;
+
   return new (Context)
       ObjCAvailabilityCheckExpr(Version, AtLoc, RParen, Context.BoolTy);
 }

cfe/trunk/test/Parser/objc-available.m

-// RUN: %clang_cc1 -fsyntax-only -Wunguarded-availability -triple x86_64-apple-macosx10.10.0 -verify %s
+// RUN: %clang_cc1 -fsyntax-only -Wunguarded-availability -Wno-unsupported-availability-guard -triple x86_64-apple-macosx10.10.0 -verify %s
 
 void f() {
 
   if (@available(macos 10.12, *)) {}
   else if (@available(macos 10.11, *)) {}
   else {}
 
   (void)__builtin_available(ios 8, macos 10.10, *);

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

 // RUN: %clang_cc1 -triple x86_64-apple-macosx-10.9 -Wunguarded-availability -fblocks -fsyntax-only -verify %s
 // RUN: %clang_cc1 -xobjective-c++ -std=c++11 -DOBJCPP -triple x86_64-apple-macosx-10.9 -Wunguarded-availability -fblocks -fsyntax-only -verify %s
 
 #define AVAILABLE_10_0  __attribute__((availability(macos, introduced = 10.0)))
 #define AVAILABLE_10_11 __attribute__((availability(macos, introduced = 10.11)))
 #define AVAILABLE_10_12 __attribute__((availability(macos, introduced = 10.12)))
 
 int func_10_11() AVAILABLE_10_11; // expected-note 4 {{'func_10_11' has been explicitly marked partial here}}
 
 #ifdef OBJCPP
 // expected-note@+2 6 {{marked partial here}}
 #endif
-int func_10_12() AVAILABLE_10_12; // expected-note 6 {{'func_10_12' has been explicitly marked partial here}}
+int func_10_12() AVAILABLE_10_12; // expected-note 7 {{'func_10_12' has been explicitly marked partial here}}
 
 int func_10_0() AVAILABLE_10_0;
 
 void use_func() {
   func_10_11(); // expected-warning{{'func_10_11' is only available on macOS 10.11 or newer}} expected-note{{enclose 'func_10_11' in an @available check to silence this warning}}
 
   if (@available(macos 10.11, *))
     func_10_11();

 @interface Subscriptable
 - (id)objectAtIndexedSubscript:(int)sub AVAILABLE_10_12; // expected-note{{marked partial here}}
 @end
 
 void test_at(Subscriptable *x) {
   id y = x[42]; // expected-warning{{'objectAtIndexedSubscript:' is only available on macOS 10.12 or newer}} expected-note{{@available}}
 }
 
+void uncheckAtAvailable() {
+  if (@available(macOS 10.12, *) || 0) // expected-warning {{@available does not guard availability here; use if (@available) instead}}
+    func_10_12(); // expected-warning {{'func_10_12' is only available on macOS 10.12 or newer}}
+  // expected-note@-1 {{enclose 'func_10_12' in an @available check to silence this warning}}
+}
+
+void justAtAvailable() {
+  int availability = @available(macOS 10.12, *); // expected-warning {{@available does not guard availability here; use if (@available) instead}}
+}
+
 #ifdef OBJCPP
 
 int f(char) AVAILABLE_10_12;
 int f(int);
 
 template <class T> int use_f() {
   // FIXME: We should warn here!
   return f(T());