[PATCH] D45392: [clang-tidy] add new check to find out objc ivars which do not have prefix '_'

Alexander Kornienko via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Apr 11 06:37:13 PDT 2018


alexfh requested changes to this revision.
alexfh added a comment.
This revision now requires changes to proceed.

Thank you, that's much better!

I'd also appreciate, if you could document the options this check supports in its .rst document (separately from this patch).

A few more comments inline.



================
Comment at: test/clang-tidy/readability-identifier-naming-objc.m:1-3
+// Remove UNSUPPORTED for powerpc64le when the problem introduced by
+// r288563 is resolved.
+// UNSUPPORTED: powerpc64le
----------------
I'd try committing the patch without disabling the test for powerpc64le and see whether the relevant buildbot complains. Looking at the description of r288563, I don't see anything that could be relevant to this test.


================
Comment at: test/clang-tidy/readability-identifier-naming-objc.m:7
+// RUN:     {key: readability-identifier-naming.ObjcIvarPrefix, value: '_'}, \
+// RUN:   ]}' -- -fno-delayed-template-parsing \
+// RUN:   -I%S/Inputs/readability-identifier-naming \
----------------
I suspect the -fno-delayed-template-parsing is not needed, since no templates are used in this test.


================
Comment at: test/clang-tidy/readability-identifier-naming-objc.m:8-9
+// RUN:   ]}' -- -fno-delayed-template-parsing \
+// RUN:   -I%S/Inputs/readability-identifier-naming \
+// RUN:   -isystem %S/Inputs/readability-identifier-naming/system
+
----------------
It looks like these flags are not needed, since there are no #include directives in this test.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D45392





More information about the cfe-commits mailing list