[clang] 1553cb5 - [Sema] Support negation/parens with __builtin_available (#111439)

[via cfe-commits]

Author: George Burgess IV
Date: 2024-10-09T10:40:53-06:00
New Revision: 1553cb5d3b14a0516c2796c295a3b32d147d13d0

URL: https://github.com/llvm/llvm-project/commit/1553cb5d3b14a0516c2796c295a3b32d147d13d0
DIFF: https://github.com/llvm/llvm-project/commit/1553cb5d3b14a0516c2796c295a3b32d147d13d0.diff

LOG: [Sema] Support negation/parens with __builtin_available (#111439)

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.

Added: 
    

Modified: 
    clang/lib/Sema/SemaAvailability.cpp
    clang/test/Sema/attr-availability.c

Removed: 
    


################################################################################
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 at +2 {{'PartiallyAvailable' has been marked as being introduced in macOS 10.8 here, but the deployment target is macOS 10.5}}
+// expected-note at +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,