[PATCH] D71694: [objc_direct] Tigthen checks for direct methods

[Adrian Prantl via Phabricator via cfe-commits]

aprantl added a comment.

Added superficial LLVM coding style comments.



================
Comment at: clang/lib/Sema/SemaDeclObjC.cpp:4735
 
+    // Merge directness from the canonical declaration upfront
+    if (!ObjCMethod->isDirectMethod()) {
----------------
missing `.` at end of sentence.

"Upfront" as opposed to? It would be helpful to add a ", because ..." here


================
Comment at: clang/lib/Sema/SemaDeclObjC.cpp:4754
+
+          // if the match is from the same Class (not super),
+          // validate that there is no declaration/implementation
----------------
// If

Is there a better way to express `Class (not super)`?


================
Comment at: clang/lib/Sema/SemaDeclObjC.cpp:4777
+
+          if (directContainerMismatch) {
+            int decl = 0, impl = 0;
----------------
how about factoring this into a diagContainerMismatch lambda?


================
Comment at: clang/lib/Sema/SemaDeclObjC.cpp:4829
+      // There can be a single declaration in any @interface container
+      // for a given direct method, look for clashes as we add them
+      //
----------------
`.`


================
Comment at: clang/lib/Sema/SemaDeclObjC.cpp:4845
+            // diag::err_objc_direct_on_protocol is already emitted for these
+            // with a better diagnostic, so don't do it twice
+          } else if (ObjCMethod->isDirectMethod() || IMD->isDirectMethod()) {
----------------
full sentence please


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D71694/new/

https://reviews.llvm.org/D71694