[PATCH] D98251: [-Wcompletion-handler] Extend list of detected conventions

[Valeriy Savchenko via Phabricator via cfe-commits]

vsavchenko created this revision.
vsavchenko added a reviewer: NoQ.
Herald added a subscriber: Charusso.
vsavchenko requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Update convention detection to accomodate changes from:
https://github.com/DougGregor/swift-evolution/blob/concurrency-objc/proposals/NNNN-concurrency-objc.md#asynchronous-completion-handler-methods


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D98251

Files:
  clang/lib/Analysis/CalledOnceCheck.cpp
  clang/test/SemaObjC/warn-called-once.m


Index: clang/test/SemaObjC/warn-called-once.m
===================================================================
--- clang/test/SemaObjC/warn-called-once.m
+++ clang/test/SemaObjC/warn-called-once.m
@@ -1036,6 +1036,20 @@
   }
 }
 
+- (void)test:(int)cond fooWithReplyTo:(void (^)(void))handler {
+  if (cond) {
+    // expected-warning@-1{{completion handler is never called when taking 
false branch}}
+    handler();
+  }
+}
+
+- (void)test:(int)cond with:(void (^)(void))fooWithCompletionBlock {
+  if (cond) {
+    // expected-warning@-1{{completion handler is never called when taking 
false branch}}
+    fooWithCompletionBlock();
+  }
+}
+
 - (void)completion_handler_wrong_type:(int (^)(void))completionHandler {
   // We don't want to consider completion handlers with non-void return types.
   if ([self condition]) {
Index: clang/lib/Analysis/CalledOnceCheck.cpp
===================================================================
--- clang/lib/Analysis/CalledOnceCheck.cpp
+++ clang/lib/Analysis/CalledOnceCheck.cpp
@@ -48,9 +48,12 @@
 template <class T>
 using CFGSizedVector = llvm::SmallVector<T, EXPECTED_NUMBER_OF_BASIC_BLOCKS>;
 constexpr llvm::StringLiteral CONVENTIONAL_NAMES[] = {
-    "completionHandler", "completion", "withCompletionHandler"};
+    "completionHandler", "completion",      "withCompletionHandler",
+    "withCompletion",    "completionBlock", "withCompletionBlock",
+    "replyTo",           "reply",           "withReplyTo"};
 constexpr llvm::StringLiteral CONVENTIONAL_SUFFIXES[] = {
-    "WithCompletionHandler", "WithCompletion"};
+    "WithCompletionHandler", "WithCompletion", "WithCompletionBlock",
+    "WithReplyTo", "WithReply"};
 constexpr llvm::StringLiteral CONVENTIONAL_CONDITIONS[] = {
     "error", "cancel", "shouldCall", "done", "OK", "success"};
 
@@ -994,13 +997,15 @@
       return hasConventionalSuffix(MethodSelector.getNameForSlot(0));
     }
 
-    return isConventional(MethodSelector.getNameForSlot(PieceIndex));
+    llvm::StringRef PieceName = MethodSelector.getNameForSlot(PieceIndex);
+    return isConventional(PieceName) || hasConventionalSuffix(PieceName);
   }
 
   bool shouldBeCalledOnce(const ParmVarDecl *Parameter) const {
     return isExplicitlyMarked(Parameter) ||
            (CheckConventionalParameters &&
-            isConventional(Parameter->getName()) &&
+            (isConventional(Parameter->getName()) ||
+             hasConventionalSuffix(Parameter->getName())) &&
             isConventional(Parameter->getType()));
   }
 



Index: clang/test/SemaObjC/warn-called-once.m
===================================================================
--- clang/test/SemaObjC/warn-called-once.m
+++ clang/test/SemaObjC/warn-called-once.m
@@ -1036,6 +1036,20 @@
   }
 }
 
+- (void)test:(int)cond fooWithReplyTo:(void (^)(void))handler {
+  if (cond) {
+    // expected-warning@-1{{completion handler is never called when taking false branch}}
+    handler();
+  }
+}
+
+- (void)test:(int)cond with:(void (^)(void))fooWithCompletionBlock {
+  if (cond) {
+    // expected-warning@-1{{completion handler is never called when taking false branch}}
+    fooWithCompletionBlock();
+  }
+}
+
 - (void)completion_handler_wrong_type:(int (^)(void))completionHandler {
   // We don't want to consider completion handlers with non-void return types.
   if ([self condition]) {
Index: clang/lib/Analysis/CalledOnceCheck.cpp
===================================================================
--- clang/lib/Analysis/CalledOnceCheck.cpp
+++ clang/lib/Analysis/CalledOnceCheck.cpp
@@ -48,9 +48,12 @@
 template <class T>
 using CFGSizedVector = llvm::SmallVector<T, EXPECTED_NUMBER_OF_BASIC_BLOCKS>;
 constexpr llvm::StringLiteral CONVENTIONAL_NAMES[] = {
-    "completionHandler", "completion", "withCompletionHandler"};
+    "completionHandler", "completion",      "withCompletionHandler",
+    "withCompletion",    "completionBlock", "withCompletionBlock",
+    "replyTo",           "reply",           "withReplyTo"};
 constexpr llvm::StringLiteral CONVENTIONAL_SUFFIXES[] = {
-    "WithCompletionHandler", "WithCompletion"};
+    "WithCompletionHandler", "WithCompletion", "WithCompletionBlock",
+    "WithReplyTo", "WithReply"};
 constexpr llvm::StringLiteral CONVENTIONAL_CONDITIONS[] = {
     "error", "cancel", "shouldCall", "done", "OK", "success"};
 
@@ -994,13 +997,15 @@
       return hasConventionalSuffix(MethodSelector.getNameForSlot(0));
     }
 
-    return isConventional(MethodSelector.getNameForSlot(PieceIndex));
+    llvm::StringRef PieceName = MethodSelector.getNameForSlot(PieceIndex);
+    return isConventional(PieceName) || hasConventionalSuffix(PieceName);
   }
 
   bool shouldBeCalledOnce(const ParmVarDecl *Parameter) const {
     return isExplicitlyMarked(Parameter) ||
            (CheckConventionalParameters &&
-            isConventional(Parameter->getName()) &&
+            (isConventional(Parameter->getName()) ||
+             hasConventionalSuffix(Parameter->getName())) &&
             isConventional(Parameter->getType()));
   }
 

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits