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

Matthias Gehre via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Sun Jun 16 12:46:35 PDT 2019


mgehre marked an inline comment as done.
mgehre added inline comments.


================
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) {
----------------
lebedev.ri wrote:
> 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;
>   }
> ```
Good idea, will do!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D63088





More information about the cfe-commits mailing list