[PATCH] D16953: Enhance clang-tidy modernize-redundant-void-arg check to apply fixes to header files

Richard via cfe-commits cfe-commits at lists.llvm.org
Tue Feb 9 07:28:04 PST 2016


LegalizeAdulthood marked an inline comment as done.

================
Comment at: test/clang-tidy/check_clang_tidy.py:152
@@ -68,4 +151,3 @@
 
-  has_check_fixes = input_text.find('CHECK-FIXES') >= 0
-  has_check_messages = input_text.find('CHECK-MESSAGES') >= 0
+  has_check_fixes, has_check_messages = has_checks(input_text)
 
----------------
LegalizeAdulthood wrote:
> alexfh wrote:
> > This function doesn't look like a particularly useful one: 5 lines of code instead of 2 lines and an unclear interface (in which order do the two bools come?). Please revert this part.
> I initially had it doing the same checks on the header file as well and then it made more sense.  I dug myself into a dead-end on that series of changes, reverted and started over.
> 
> What I settled on here was the assumption that you won't have `CHECK-MESSAGES` or `CHECK-FIXES` in your header file, unless you also had them in the associated source file.  If that assumption is invalid, then I should call this function again for the header and change the tests to be asserting fixes in the source or header, messages in the source or header in various places.
IMO, "5 lines instead of 2 lines" isn't the kind of reasoning that is useful.  Otherwise we would never do [[ https://www.industriallogic.com/xp/refactoring/composeMethod.html | Compose Method ]] on a long method since the act of breaking a long method down into a bunch of smaller methods necessarily introduces more lines of code.

This script originally was one giant function that did everything.  A bunch of things need to be done twice now: one for the source file and once for the header file.  Applying Compose Method Extracting results in extracting a group of smaller functions/methods.

Now the functions have a bunch of state that needs to be passed around between them and the functions might be more clearly expressed as methods on a class, but I didn't go that far with it.


http://reviews.llvm.org/D16953





More information about the cfe-commits mailing list