[PATCH] D55640: [clang-tidy] Implement a check for large Objective-C type encodings 🔍

Haojian Wu via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Dec 17 02:19:35 PST 2018


hokein added inline comments.


================
Comment at: clang-tidy/objc/TypeEncodingSizeCheck.cpp:36
+      objcPropertyDecl(unless(isExpansionInSystemHeader()),
+                       anyOf(hasAncestor(objcInterfaceDecl().bind("interface")),
+                             hasAncestor(objcCategoryDecl().bind("category"))))
----------------
`hasAncestor` is an expensive matcher, does `hasDeclContext` meet your use cases?


================
Comment at: clang-tidy/objc/TypeEncodingSizeCheck.cpp:60
+
+  const auto *EncodedDecl = Result.Nodes.getNodeAs<NamedDecl>("decl");
+
----------------
nit: add an assertion `assert(EncodedDecl)`.


================
Comment at: clang-tidy/objc/TypeEncodingSizeCheck.cpp:63
+  std::string TypeEncoding;
+  if (const auto *IvarDecl = dyn_cast<ObjCIvarDecl>(EncodedDecl)) {
+    IvarDecl->getASTContext().getObjCEncodingForType(IvarDecl->getType(),
----------------
Do you forget to register the matcher for `ObjCIvarDecl`? In the matcher you register it for `ObjCPropertyDecl`, and `ObjCInterfaceDecl`, so this branch will never be executed.


================
Comment at: docs/clang-tidy/checks/objc-type-encoding-size.rst:13
+
+   Flag Objective-C type encodings that exceed this number of bytes.
----------------
nit: mention the default value.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D55640





More information about the cfe-commits mailing list