[Sema] Support negation/parens with __builtin_available

[gburgessiv]

At present, __builtin_available is really restrictive with its use. Overall, this seems like a good thing, since the analyses behind it are not very expensive.

That said, it's very straightforward to support these two cases:

if ((__builtin_available(foo, *))) {
  // ...
}

and

if (!__builtin_available(foo, *)) {
  // ...
} else {
  // ...
}

Seems nice to do so.

[llvmbot]

@llvm/pr-subscribers-clang

Author: George Burgess IV (gburgessiv)

Changes

At present, __builtin_available is really restrictive with its use. Overall, this seems like a good thing, since the analyses behind it are not very expensive.

That said, it's very straightforward to support these two cases:

if ((__builtin_available(foo, *))) {
  // ...
}

and

if (!__builtin_available(foo, *)) {
  // ...
} else {
  // ...
}

Seems nice to do so.

---

Full diff: https://github.com/llvm/llvm-project/pull/111439.diff

2 Files Affected:

• (modified) clang/lib/Sema/SemaAvailability.cpp (+40-11)
• (modified) clang/test/Sema/attr-availability.c (+24-1)

diff --git a/clang/lib/Sema/SemaAvailability.cpp b/clang/lib/Sema/SemaAvailability.cpp
index e04cbeec165552..798cabaa31a476 100644
--- a/clang/lib/Sema/SemaAvailability.cpp
+++ b/clang/lib/Sema/SemaAvailability.cpp
@@ -1005,25 +1005,54 @@ bool DiagnoseUnguardedAvailability::VisitTypeLoc(TypeLoc Ty) {
   return true;
 }
 
+struct ExtractedAvailabilityExpr {
+  const ObjCAvailabilityCheckExpr *E = nullptr;
+  bool isNegated = false;
+};
+
+ExtractedAvailabilityExpr extractAvailabilityExpr(const Expr *IfCond) {
+  const auto *E = IfCond;
+  bool IsNegated = false;
+  while (true) {
+    E = E->IgnoreParens();
+    if (const auto *AE = dyn_cast<ObjCAvailabilityCheckExpr>(E)) {
+      return ExtractedAvailabilityExpr{AE, IsNegated};
+    }
+
+    const auto *UO = dyn_cast<UnaryOperator>(E);
+    if (!UO || UO->getOpcode() != UO_LNot) {
+      return ExtractedAvailabilityExpr{};
+    }
+    E = UO->getSubExpr();
+    IsNegated = !IsNegated;
+  }
+}
+
 bool DiagnoseUnguardedAvailability::TraverseIfStmt(IfStmt *If) {
-  VersionTuple CondVersion;
-  if (auto *E = dyn_cast<ObjCAvailabilityCheckExpr>(If->getCond())) {
-    CondVersion = E->getVersion();
-
-    // If we're using the '*' case here or if this check is redundant, then we
-    // use the enclosing version to check both branches.
-    if (CondVersion.empty() || CondVersion <= AvailabilityStack.back())
-      return TraverseStmt(If->getThen()) && TraverseStmt(If->getElse());
-  } else {
+  ExtractedAvailabilityExpr IfCond = extractAvailabilityExpr(If->getCond());
+  if (!IfCond.E) {
     // This isn't an availability checking 'if', we can just continue.
     return Base::TraverseIfStmt(If);
   }
 
+  VersionTuple CondVersion = IfCond.E->getVersion();
+  // If we're using the '*' case here or if this check is redundant, then we
+  // use the enclosing version to check both branches.
+  if (CondVersion.empty() || CondVersion <= AvailabilityStack.back()) {
+    return TraverseStmt(If->getThen()) && TraverseStmt(If->getElse());
+  }
+
+  auto *Guarded = If->getThen();
+  auto *Unguarded = If->getElse();
+  if (IfCond.isNegated) {
+    std::swap(Guarded, Unguarded);
+  }
+
   AvailabilityStack.push_back(CondVersion);
-  bool ShouldContinue = TraverseStmt(If->getThen());
+  bool ShouldContinue = TraverseStmt(Guarded);
   AvailabilityStack.pop_back();
 
-  return ShouldContinue && TraverseStmt(If->getElse());
+  return ShouldContinue && TraverseStmt(Unguarded);
 }
 
 } // end anonymous namespace
diff --git a/clang/test/Sema/attr-availability.c b/clang/test/Sema/attr-availability.c
index a5cc602a8fa9d4..a496c5271f2a3d 100644
--- a/clang/test/Sema/attr-availability.c
+++ b/clang/test/Sema/attr-availability.c
@@ -40,7 +40,7 @@ void test_10095131(void) {
 #ifdef WARN_PARTIAL
 // FIXME: This note should point to the declaration with the availability
 // attribute.
-// expected-note@+2 {{'PartiallyAvailable' has been marked as being introduced in macOS 10.8 here, but the deployment target is macOS 10.5}}
+// expected-note@+2 5 {{'PartiallyAvailable' has been marked as being introduced in macOS 10.8 here, but the deployment target is macOS 10.5}}
 #endif
 extern void PartiallyAvailable(void) ;
 void with_redeclaration(void) {
@@ -53,6 +53,29 @@ void with_redeclaration(void) {
   enum PartialEnum p = kPartialEnumConstant;
 }
 
+#ifdef WARN_PARTIAL
+void conditional_warnings() {
+  if (__builtin_available(macos 10.8, *)) {
+    PartiallyAvailable();
+  } else {
+    PartiallyAvailable(); // expected-warning {{only available on macOS 10.8 or newer}} expected-note {{enclose 'PartiallyAvailable'}}
+  }
+  if (!__builtin_available(macos 10.8, *)) {
+    PartiallyAvailable(); // expected-warning {{only available on macOS 10.8 or newer}} expected-note {{enclose 'PartiallyAvailable'}}
+  } else {
+    PartiallyAvailable();
+  }
+  if (!!!(!__builtin_available(macos 10.8, *))) {
+    PartiallyAvailable();
+  } else {
+    PartiallyAvailable(); // expected-warning {{only available on macOS 10.8 or newer}} expected-note {{enclose 'PartiallyAvailable'}}
+  }
+  if (~__builtin_available(macos 10.8, *)) { // expected-warning {{does not guard availability here}}
+    PartiallyAvailable(); // expected-warning {{only available on macOS 10.8 or newer}} expected-note {{enclose 'PartiallyAvailable'}}
+  }
+}
+#endif
+
 __attribute__((availability(macos, unavailable))) // expected-warning {{attribute 'availability' is ignored}}
 enum {
     NSDataWritingFileProtectionWriteOnly = 0x30000000,

[gburgessiv]

Hey Damyan, GH suggested you as a reviewer for this. Would you be able to TAL? Happy to find someone else if not :)

[damyanp]

Hey Damyan, GH suggested you as a reviewer for this. Would you be able to TAL? Happy to find someone else if not :)

I'm not sure why GH would have done that :). I'm afraid I wouldn't be able to give an informed review here. (I might have a quick look anyway).

[gburgessiv]

OK, thanks! I'll try to find someone with more context later today :)

[hekota]

LGTM!

[gburgessiv]

(...Or a reviewer will find me - thank you!)