[PATCH] D63088: [clang-tidy] misc-unused-parameters: don't comment out parameter name for C code

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri accepted this revision.
lebedev.ri added a comment.
This revision is now accepted and ready to land.

Seems obviously correct, with the nit.



================
Comment at: clang-tools-extra/clang-tidy/misc/UnusedParametersCheck.cpp:141-156
+  // Cannot remove parameter for non-local functions.
   if (Function->isExternallyVisible() ||
       !Result.SourceManager->isInMainFile(Function->getLocation()) ||
       !Indexer->getOtherRefs(Function).empty() || isOverrideMethod(Function)) {
-    SourceRange RemovalRange(Param->getLocation());
-    // Note: We always add a space before the '/*' to not accidentally create a
-    // '*/*' for pointer types, which doesn't start a comment. clang-format will
-    // clean this up afterwards.
-    MyDiag << FixItHint::CreateReplacement(
-        RemovalRange, (Twine(" /*") + Param->getName() + "*/").str());
+
+    // Comment out parameter name.
+    if (Result.Context->getLangOpts().CPlusPlus) {
----------------
I'd recommend to instead do less confusing
```
  // Cannot remove parameter for non-local functions.
  if (Function->isExternallyVisible() ||
      !Result.SourceManager->isInMainFile(Function->getLocation()) ||
      !Indexer->getOtherRefs(Function).empty() || isOverrideMethod(Function)) {
    // It is illegal to omit parameter name here in C code, so early-out.
    if (!Result.Context->getLangOpts().CPlusPlus)
      return;

    SourceRange RemovalRange(Param->getLocation());
    // Note: We always add a space before the '/*' to not accidentally create
    // a '*/*' for pointer types, which doesn't start a comment. clang-format
    // will clean this up afterwards.
    MyDiag << FixItHint::CreateReplacement(
        RemovalRange, (Twine(" /*") + Param->getName() + "*/").str());
    return;
  }
```


================
Comment at: clang-tools-extra/test/clang-tidy/misc-unused-parameters.c:1-7
 // RUN: %check_clang_tidy %s misc-unused-parameters %t -- -- -xc
 
 // Basic removal
 // =============
 void a(int i) {;}
 // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: parameter 'i' is unused [misc-unused-parameters]
-// CHECK-FIXES: {{^}}void a(int  /*i*/) {;}{{$}}
----------------
This screams a separate bug to me.
Does `%check_clang_tidy` not check that sources, after applying fix-it's, still parse?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D63088