stephanemoore added a comment.
In D55640#1329390 <https://reviews.llvm.org/D55640#1329390>, @theraven wrote:
> I wonder if we want to have an option to elide ObjC type info for all non-POD
> C++ types. Nothing that you do with the type encoding is likely to be
> correct (for example, you can see the pointer field in a `std::shared_ptr`,
> but you can't see that changes to it need to update reference counts) so it
> probably does more harm than good.
I think it's worth investigating some kind of option for eliding Objective-C
type information though I think that investigation is likely beyond the scope
of this proposal. Eliding Objective-C type information could change runtime
behavior of a variety of systems that rely on this information. This could
include systems embedded in Apple's libraries as well as systems implemented in
third party or open source libraries. I imagine that an investigation into such
a feature would probably need to be quite thorough.
================
Comment at: clang-tidy/objc/TypeEncodingSizeCheck.cpp:36
+ objcPropertyDecl(unless(isExpansionInSystemHeader()),
+
anyOf(hasAncestor(objcInterfaceDecl().bind("interface")),
+ hasAncestor(objcCategoryDecl().bind("category"))))
----------------
hokein wrote:
> `hasAncestor` is an expensive matcher, does `hasDeclContext` meet your use
> cases?
I started looking into using `hasDeclContext` but encountered some unexpected
behavior when adding relevant test cases—which should have been added from the
beginning—to verify. I am going to continue investigating whether
`hasDeclContext` is satisfactory here and try to resolve the unexpected
behavior that I encountered.
================
Comment at: clang-tidy/objc/TypeEncodingSizeCheck.cpp:63
+ std::string TypeEncoding;
+ if (const auto *IvarDecl = dyn_cast<ObjCIvarDecl>(EncodedDecl)) {
+ IvarDecl->getASTContext().getObjCEncodingForType(IvarDecl->getType(),
----------------
hokein wrote:
> 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.
On line 32 in this diff is where I register the matcher for `ObjCIvarDecl`. The
matching is functioning as I expect it should. Let me know if you want me to
reorganize the matching logic.
================
Comment at: docs/clang-tidy/checks/objc-type-encoding-size.rst:6
+
+Finds Objective-C type encodings that exceed a configured threshold.
+
----------------
Eugene.Zelenko wrote:
> Please synchronize with Release Notes.
I changed the check description to match the release notes. Let me know if I
misunderstood the requested change.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D55640/new/
https://reviews.llvm.org/D55640
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
