[PATCH] D116778: [clang-tidy][clang] Don't trigger unused-parameter warnings on naked functions

[Tommaso Bonvicini via Phabricator via cfe-commits]

MuAlphaOmegaEpsilon added a comment.

In D116778#3255414 , @aaron.ballman 
wrote:

> LGTM! The CI failure is finally down to just an unrelated one (yay?). Do you 
> need someone to commit on your behalf? If so, what name and email address 
> would you like used for patch attribution?

Great! 🎉
I've never committed on the LLVM repository, so I don't know the requirements. 
If it doesn't bother you feel free to commit on my behalf using the following: 
MuAlphaOmegaEpsilon 

Thank you very much Aaron! 😀


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116778

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D116778: [clang-tidy][clang] Don't trigger unused-parameter warnings on naked functions

[Tommaso Bonvicini via Phabricator via cfe-commits]

MuAlphaOmegaEpsilon added a comment.

In D116778#3276421 , @aaron.ballman 
wrote:

> In D116778#3259189 , 
> @MuAlphaOmegaEpsilon wrote:
>
>> In D116778#3255414 , 
>> @aaron.ballman wrote:
>>
>>> LGTM! The CI failure is finally down to just an unrelated one (yay?). Do 
>>> you need someone to commit on your behalf? If so, what name and email 
>>> address would you like used for patch attribution?
>>
>> Great! 🎉
>> I've never committed on the LLVM repository, so I don't know the 
>> requirements. If it doesn't bother you, feel free to commit on my behalf 
>> using the following: 
>> MuAlphaOmegaEpsilon 
>>
>> Thank you very much Aaron! 😀
>
> Sorry about the delay in getting this landed (it slipped beneath my radar). 
> I've commit on your behalf in ccce1a03c9ce9c3917b310097c89e39bb68527e2 
> . Thank 
> you for the fix!

No problem, I wasn't in a hurry! :)

Thank you very much again Aaron! 😀


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116778

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D116778: [clang-tidy][clang] Don't trigger unused-parameter warnings on naked functions

[Tommaso Bonvicini via Phabricator via cfe-commits]

MuAlphaOmegaEpsilon created this revision.
MuAlphaOmegaEpsilon added a reviewer: aaron.ballman.
Herald added subscribers: carlosgalvezp, xazax.hun.
MuAlphaOmegaEpsilon requested review of this revision.
Herald added projects: clang, clang-tools-extra.
Herald added a subscriber: cfe-commits.

This commit checks if a function is marked with the naked attribute and,
if it is, will silence the emission of any unused-parameter warning.

Inside a naked function only the usage of basic ASM instructions is
expected. In this context the parameters can actually be used by
fetching them according to the underlying ABI. Since parameters might be
used through ASM instructions, the linter and the compiler will have a
hard time understanding if one of those is unused or not. Usage of the
naked attribute implies the user taking full responsibility in the
handling of prologue and epilogue of the function, therefore no
unused-parameter warning should ever be triggered whenever a function is
marked naked.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D116778

Files:
  clang-tools-extra/clang-tidy/misc/UnusedParametersCheck.cpp
  clang/lib/Sema/SemaDecl.cpp


Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -14632,8 +14632,16 @@
 Diag(FD->getLocation(), diag::ext_pure_function_definition);
 
   if (!FD->isInvalidDecl()) {
+bool FDHasNakedAttr{false};
+if (FD->hasAttrs())
+  for (const clang::Attr *A : FD->getAttrs())
+if (A->getParsedKind() == Attr::AT_Naked) {
+  FDHasNakedAttr = true;
+  break;
+}
 // Don't diagnose unused parameters of defaulted or deleted functions.
-if (!FD->isDeleted() && !FD->isDefaulted() && !FD->hasSkippedBody())
+if (!FD->isDeleted() && !FD->isDefaulted() && !FD->hasSkippedBody() &&
+!FDHasNakedAttr)
   DiagnoseUnusedParameters(FD->parameters());
 DiagnoseSizeOfParametersAndReturnValue(FD->parameters(),
FD->getReturnType(), FD);
Index: clang-tools-extra/clang-tidy/misc/UnusedParametersCheck.cpp
===
--- clang-tools-extra/clang-tidy/misc/UnusedParametersCheck.cpp
+++ clang-tools-extra/clang-tidy/misc/UnusedParametersCheck.cpp
@@ -174,6 +174,10 @@
   const auto *Function = Result.Nodes.getNodeAs("function");
   if (!Function->hasWrittenPrototype() || Function->isTemplateInstantiation())
 return;
+  if (Function->hasAttrs())
+for (const clang::Attr *A : Function->getAttrs())
+  if (A->getParsedKind() == Attr::AT_Naked)
+return;
   if (const auto *Method = dyn_cast(Function))
 if (Method->isLambdaStaticInvoker())
   return;


Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -14632,8 +14632,16 @@
 Diag(FD->getLocation(), diag::ext_pure_function_definition);
 
   if (!FD->isInvalidDecl()) {
+bool FDHasNakedAttr{false};
+if (FD->hasAttrs())
+  for (const clang::Attr *A : FD->getAttrs())
+if (A->getParsedKind() == Attr::AT_Naked) {
+  FDHasNakedAttr = true;
+  break;
+}
 // Don't diagnose unused parameters of defaulted or deleted functions.
-if (!FD->isDeleted() && !FD->isDefaulted() && !FD->hasSkippedBody())
+if (!FD->isDeleted() && !FD->isDefaulted() && !FD->hasSkippedBody() &&
+!FDHasNakedAttr)
   DiagnoseUnusedParameters(FD->parameters());
 DiagnoseSizeOfParametersAndReturnValue(FD->parameters(),
FD->getReturnType(), FD);
Index: clang-tools-extra/clang-tidy/misc/UnusedParametersCheck.cpp
===
--- clang-tools-extra/clang-tidy/misc/UnusedParametersCheck.cpp
+++ clang-tools-extra/clang-tidy/misc/UnusedParametersCheck.cpp
@@ -174,6 +174,10 @@
   const auto *Function = Result.Nodes.getNodeAs("function");
   if (!Function->hasWrittenPrototype() || Function->isTemplateInstantiation())
 return;
+  if (Function->hasAttrs())
+for (const clang::Attr *A : Function->getAttrs())
+  if (A->getParsedKind() == Attr::AT_Naked)
+return;
   if (const auto *Method = dyn_cast(Function))
 if (Method->isLambdaStaticInvoker())
   return;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D116778: [clang-tidy][clang] Don't trigger unused-parameter warnings on naked functions

[Tommaso Bonvicini via Phabricator via cfe-commits]

MuAlphaOmegaEpsilon updated this revision to Diff 398182.
MuAlphaOmegaEpsilon added a comment.

Rebased to more recent main branch, updated comment


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116778

Files:
  clang-tools-extra/clang-tidy/misc/UnusedParametersCheck.cpp
  clang/lib/Sema/SemaDecl.cpp


Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -14632,8 +14632,17 @@
 Diag(FD->getLocation(), diag::ext_pure_function_definition);
 
   if (!FD->isInvalidDecl()) {
-// Don't diagnose unused parameters of defaulted or deleted functions.
-if (!FD->isDeleted() && !FD->isDefaulted() && !FD->hasSkippedBody())
+bool FDHasNakedAttr{false};
+if (FD->hasAttrs())
+  for (const clang::Attr *A : FD->getAttrs())
+if (A->getParsedKind() == Attr::AT_Naked) {
+  FDHasNakedAttr = true;
+  break;
+}
+// Don't diagnose unused parameters of defaulted, deleted or naked
+// functions.
+if (!FD->isDeleted() && !FD->isDefaulted() && !FD->hasSkippedBody() &&
+!FDHasNakedAttr)
   DiagnoseUnusedParameters(FD->parameters());
 DiagnoseSizeOfParametersAndReturnValue(FD->parameters(),
FD->getReturnType(), FD);
Index: clang-tools-extra/clang-tidy/misc/UnusedParametersCheck.cpp
===
--- clang-tools-extra/clang-tidy/misc/UnusedParametersCheck.cpp
+++ clang-tools-extra/clang-tidy/misc/UnusedParametersCheck.cpp
@@ -174,6 +174,10 @@
   const auto *Function = Result.Nodes.getNodeAs("function");
   if (!Function->hasWrittenPrototype() || Function->isTemplateInstantiation())
 return;
+  if (Function->hasAttrs())
+for (const clang::Attr *A : Function->getAttrs())
+  if (A->getParsedKind() == Attr::AT_Naked)
+return;
   if (const auto *Method = dyn_cast(Function))
 if (Method->isLambdaStaticInvoker())
   return;


Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -14632,8 +14632,17 @@
 Diag(FD->getLocation(), diag::ext_pure_function_definition);
 
   if (!FD->isInvalidDecl()) {
-// Don't diagnose unused parameters of defaulted or deleted functions.
-if (!FD->isDeleted() && !FD->isDefaulted() && !FD->hasSkippedBody())
+bool FDHasNakedAttr{false};
+if (FD->hasAttrs())
+  for (const clang::Attr *A : FD->getAttrs())
+if (A->getParsedKind() == Attr::AT_Naked) {
+  FDHasNakedAttr = true;
+  break;
+}
+// Don't diagnose unused parameters of defaulted, deleted or naked
+// functions.
+if (!FD->isDeleted() && !FD->isDefaulted() && !FD->hasSkippedBody() &&
+!FDHasNakedAttr)
   DiagnoseUnusedParameters(FD->parameters());
 DiagnoseSizeOfParametersAndReturnValue(FD->parameters(),
FD->getReturnType(), FD);
Index: clang-tools-extra/clang-tidy/misc/UnusedParametersCheck.cpp
===
--- clang-tools-extra/clang-tidy/misc/UnusedParametersCheck.cpp
+++ clang-tools-extra/clang-tidy/misc/UnusedParametersCheck.cpp
@@ -174,6 +174,10 @@
   const auto *Function = Result.Nodes.getNodeAs("function");
   if (!Function->hasWrittenPrototype() || Function->isTemplateInstantiation())
 return;
+  if (Function->hasAttrs())
+for (const clang::Attr *A : Function->getAttrs())
+  if (A->getParsedKind() == Attr::AT_Naked)
+return;
   if (const auto *Method = dyn_cast(Function))
 if (Method->isLambdaStaticInvoker())
   return;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D116778: [clang-tidy][clang] Don't trigger unused-parameter warnings on naked functions

[Tommaso Bonvicini via Phabricator via cfe-commits]

MuAlphaOmegaEpsilon updated this revision to Diff 399793.
MuAlphaOmegaEpsilon added a comment.

Update code after review and add tests


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116778

Files:
  clang-tools-extra/clang-tidy/misc/UnusedParametersCheck.cpp
  clang-tools-extra/test/clang-tidy/checkers/misc-unused-parameters-strict.cpp
  clang-tools-extra/test/clang-tidy/checkers/misc-unused-parameters.c
  clang-tools-extra/test/clang-tidy/checkers/misc-unused-parameters.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/test/Sema/warn-unused-parameters.c
  clang/test/SemaCXX/warn-unused-parameters.cpp


Index: clang/test/SemaCXX/warn-unused-parameters.cpp
===
--- clang/test/SemaCXX/warn-unused-parameters.cpp
+++ clang/test/SemaCXX/warn-unused-parameters.cpp
@@ -32,3 +32,7 @@
   auto l = [&t...]() { return sizeof...(s); };
   return l();
 }
+
+// Do not warn on naked functions.
+[[gnu::naked]] int nakedFunction(int a, float b, const char* c) { ; }
+__attribute__((naked)) void nakedFunction(int a, int b) { ; }
Index: clang/test/Sema/warn-unused-parameters.c
===
--- clang/test/Sema/warn-unused-parameters.c
+++ clang/test/Sema/warn-unused-parameters.c
@@ -28,3 +28,5 @@
 // CHECK-everything-error: 5 errors generated
 // CHECK-everything-no-unused: 5 warnings generated
 
+// Do not warn on naked functions.
+__attribute__((naked)) void nakedFunction(int a, int b) { ; }
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -14632,8 +14632,10 @@
 Diag(FD->getLocation(), diag::ext_pure_function_definition);
 
   if (!FD->isInvalidDecl()) {
-// Don't diagnose unused parameters of defaulted or deleted functions.
-if (!FD->isDeleted() && !FD->isDefaulted() && !FD->hasSkippedBody())
+// Don't diagnose unused parameters of defaulted, deleted or naked
+// functions.
+if (!FD->isDeleted() && !FD->isDefaulted() && !FD->hasSkippedBody() &&
+!FD->hasAttr())
   DiagnoseUnusedParameters(FD->parameters());
 DiagnoseSizeOfParametersAndReturnValue(FD->parameters(),
FD->getReturnType(), FD);
Index: clang-tools-extra/test/clang-tidy/checkers/misc-unused-parameters.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/misc-unused-parameters.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/misc-unused-parameters.cpp
@@ -286,3 +286,7 @@
   f([](int I) { return; });
 }
 } // namespace lambda
+
+// Do not warn on naked functions.
+[[gnu::naked]] int nakedFunction(int a, float b, const char *c) { ; }
+__attribute__((naked)) void nakedFunction(int a, int b) { ; }
Index: clang-tools-extra/test/clang-tidy/checkers/misc-unused-parameters.c
===
--- clang-tools-extra/test/clang-tidy/checkers/misc-unused-parameters.c
+++ clang-tools-extra/test/clang-tidy/checkers/misc-unused-parameters.c
@@ -15,3 +15,5 @@
 // ===
 void h(i, c, d) int i; char *c, *d; {} // Don't mess with K&R style
 
+// Do not warn on naked functions.
+__attribute__((naked)) void nakedFunction(int a, int b) { ; }
Index: 
clang-tools-extra/test/clang-tidy/checkers/misc-unused-parameters-strict.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/misc-unused-parameters-strict.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/misc-unused-parameters-strict.cpp
@@ -22,4 +22,8 @@
 // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: parameter 'j' is unused
 // CHECK-FIXES: {{^}}  F(int  /*j*/) : i() {}{{$}}
 };
+
+// Do not warn on naked functions.
+[[gnu::naked]] int nakedFunction(int a, float b, const char *c) { ; }
+__attribute__((naked)) void nakedFunction(int a, int b) { ; }
 }
Index: clang-tools-extra/clang-tidy/misc/UnusedParametersCheck.cpp
===
--- clang-tools-extra/clang-tidy/misc/UnusedParametersCheck.cpp
+++ clang-tools-extra/clang-tidy/misc/UnusedParametersCheck.cpp
@@ -31,10 +31,11 @@
 } // namespace
 
 void UnusedParametersCheck::registerMatchers(MatchFinder *Finder) {
-  Finder->addMatcher(
-  functionDecl(isDefinition(), hasBody(stmt()), hasAnyParameter(decl()))
-  .bind("function"),
-  this);
+  Finder->addMatcher(functionDecl(isDefinition(), hasBody(stmt()),
+  hasAnyParameter(decl()),
+  unless(hasAttr(attr::Kind::Naked)))
+ .bind("function"),
+ this);
 }
 
 template 


Index: clang/test/SemaCXX/warn-unused-parameters.cpp

[PATCH] D116778: [clang-tidy][clang] Don't trigger unused-parameter warnings on naked functions

[Tommaso Bonvicini via Phabricator via cfe-commits]

MuAlphaOmegaEpsilon added a comment.

In D116778#3244233 , @aaron.ballman 
wrote:

> The changes look good aside from the precommit CI failing. Can you 
> investigate?

Absolutely, I'm already giving a look at it! :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116778

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D116778: [clang-tidy][clang] Don't trigger unused-parameter warnings on naked functions

[Tommaso Bonvicini via Phabricator via cfe-commits]

MuAlphaOmegaEpsilon updated this revision to Diff 400078.
MuAlphaOmegaEpsilon added a comment.

Fix failing test by updating clang/test/Sema/warn-unused-parameters.c expected 
warnings amount


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116778

Files:
  clang-tools-extra/clang-tidy/misc/UnusedParametersCheck.cpp
  clang-tools-extra/test/clang-tidy/checkers/misc-unused-parameters-strict.cpp
  clang-tools-extra/test/clang-tidy/checkers/misc-unused-parameters.c
  clang-tools-extra/test/clang-tidy/checkers/misc-unused-parameters.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/test/Sema/warn-unused-parameters.c
  clang/test/SemaCXX/warn-unused-parameters.cpp

Index: clang/test/SemaCXX/warn-unused-parameters.cpp
===
--- clang/test/SemaCXX/warn-unused-parameters.cpp
+++ clang/test/SemaCXX/warn-unused-parameters.cpp
@@ -32,3 +32,7 @@
   auto l = [&t...]() { return sizeof...(s); };
   return l();
 }
+
+// Do not warn on naked functions.
+[[gnu::naked]] int nakedFunction(int a, float b, const char* c) { ; }
+__attribute__((naked)) void nakedFunction(int a, int b) { ; }
Index: clang/test/Sema/warn-unused-parameters.c
===
--- clang/test/Sema/warn-unused-parameters.c
+++ clang/test/Sema/warn-unused-parameters.c
@@ -17,6 +17,9 @@
 // warnings for the above cases.
 static void achor() {};
 
+// Do not warn on naked functions.
+__attribute__((naked)) static void nakedFunction(int a, int b) { }
+
 // CHECK: 5:12: warning: unused parameter 'y'
 // CHECK: 12:15: warning: unused parameter 'y'
 // CHECK-unused: 1 warning generated
@@ -24,7 +27,6 @@
 // RUN: %clang_cc1 -fblocks -fsyntax-only -Weverything %s 2>&1 | FileCheck -check-prefix=CHECK-everything %s
 // RUN: not %clang_cc1 -fblocks -fsyntax-only -Weverything -Werror %s 2>&1 | FileCheck -check-prefix=CHECK-everything-error %s
 // RUN: %clang_cc1 -fblocks -fsyntax-only -Weverything -Wno-unused %s 2>&1 | FileCheck -check-prefix=CHECK-everything-no-unused %s
-// CHECK-everything: 6 warnings generated
+// CHECK-everything: 7 warnings generated
 // CHECK-everything-error: 5 errors generated
 // CHECK-everything-no-unused: 5 warnings generated
-
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -14632,8 +14632,10 @@
 Diag(FD->getLocation(), diag::ext_pure_function_definition);
 
   if (!FD->isInvalidDecl()) {
-// Don't diagnose unused parameters of defaulted or deleted functions.
-if (!FD->isDeleted() && !FD->isDefaulted() && !FD->hasSkippedBody())
+// Don't diagnose unused parameters of defaulted, deleted or naked
+// functions.
+if (!FD->isDeleted() && !FD->isDefaulted() && !FD->hasSkippedBody() &&
+!FD->hasAttr())
   DiagnoseUnusedParameters(FD->parameters());
 DiagnoseSizeOfParametersAndReturnValue(FD->parameters(),
FD->getReturnType(), FD);
Index: clang-tools-extra/test/clang-tidy/checkers/misc-unused-parameters.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/misc-unused-parameters.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/misc-unused-parameters.cpp
@@ -286,3 +286,7 @@
   f([](int I) { return; });
 }
 } // namespace lambda
+
+// Do not warn on naked functions.
+[[gnu::naked]] int nakedFunction(int a, float b, const char *c) { ; }
+__attribute__((naked)) void nakedFunction(int a, int b) { ; }
Index: clang-tools-extra/test/clang-tidy/checkers/misc-unused-parameters.c
===
--- clang-tools-extra/test/clang-tidy/checkers/misc-unused-parameters.c
+++ clang-tools-extra/test/clang-tidy/checkers/misc-unused-parameters.c
@@ -15,3 +15,5 @@
 // ===
 void h(i, c, d) int i; char *c, *d; {} // Don't mess with K&R style
 
+// Do not warn on naked functions.
+__attribute__((naked)) void nakedFunction(int a, int b) { ; }
Index: clang-tools-extra/test/clang-tidy/checkers/misc-unused-parameters-strict.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/misc-unused-parameters-strict.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/misc-unused-parameters-strict.cpp
@@ -22,4 +22,8 @@
 // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: parameter 'j' is unused
 // CHECK-FIXES: {{^}}  F(int  /*j*/) : i() {}{{$}}
 };
+
+// Do not warn on naked functions.
+[[gnu::naked]] int nakedFunction(int a, float b, const char *c) { ; }
+__attribute__((naked)) void nakedFunction(int a, int b) { ; }
 }
Index: clang-tools-extra/clang-tidy/misc/UnusedParametersCheck.cpp
===
--- clang-tool

[PATCH] D116778: [clang-tidy][clang] Don't trigger unused-parameter warnings on naked functions

[Tommaso Bonvicini via Phabricator via cfe-commits]

MuAlphaOmegaEpsilon added a comment.

The warning would be correct if the compiler could actually tell the parameter 
is unused, but at the moment it cannot, as far as I know.

I stumbled upon this thing myself, and there are a few ways to silence the 
warning:
-removing names
-marking every single parameter as unused

The first one makes the function less readable, and forces me to add extra 
declarations or documentation to explain to the user the purpose of each 
parameter.

The second one is quite cumbersome to implement and it clutters the code quite 
a bit.

I consider these as sub-optimal scenarios, given that whatever I choose I only 
get drawbacks in order to silence a warning that I think shouldn't really be 
there in the first place. :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116778

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D116778: [clang-tidy][clang] Don't trigger unused-parameter warnings on naked functions

[Tommaso Bonvicini via Phabricator via cfe-commits]

MuAlphaOmegaEpsilon updated this revision to Diff 400143.
MuAlphaOmegaEpsilon added a comment.

Update warning amount on CHECK-unused check


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116778

Files:
  clang-tools-extra/clang-tidy/misc/UnusedParametersCheck.cpp
  clang-tools-extra/test/clang-tidy/checkers/misc-unused-parameters-strict.cpp
  clang-tools-extra/test/clang-tidy/checkers/misc-unused-parameters.c
  clang-tools-extra/test/clang-tidy/checkers/misc-unused-parameters.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/test/Sema/warn-unused-parameters.c
  clang/test/SemaCXX/warn-unused-parameters.cpp

Index: clang/test/SemaCXX/warn-unused-parameters.cpp
===
--- clang/test/SemaCXX/warn-unused-parameters.cpp
+++ clang/test/SemaCXX/warn-unused-parameters.cpp
@@ -32,3 +32,7 @@
   auto l = [&t...]() { return sizeof...(s); };
   return l();
 }
+
+// Do not warn on naked functions.
+[[gnu::naked]] int nakedFunction(int a, float b, const char* c) { ; }
+__attribute__((naked)) void nakedFunction(int a, int b) { ; }
Index: clang/test/Sema/warn-unused-parameters.c
===
--- clang/test/Sema/warn-unused-parameters.c
+++ clang/test/Sema/warn-unused-parameters.c
@@ -17,14 +17,17 @@
 // warnings for the above cases.
 static void achor() {};
 
+// Do not warn on naked functions.
+__attribute__((naked)) static void nakedFunction(int a, int b) { }
+
 // CHECK: 5:12: warning: unused parameter 'y'
 // CHECK: 12:15: warning: unused parameter 'y'
-// CHECK-unused: 1 warning generated
+// CHECK-unused: 2 warnings generated
 
 // RUN: %clang_cc1 -fblocks -fsyntax-only -Weverything %s 2>&1 | FileCheck -check-prefix=CHECK-everything %s
 // RUN: not %clang_cc1 -fblocks -fsyntax-only -Weverything -Werror %s 2>&1 | FileCheck -check-prefix=CHECK-everything-error %s
 // RUN: %clang_cc1 -fblocks -fsyntax-only -Weverything -Wno-unused %s 2>&1 | FileCheck -check-prefix=CHECK-everything-no-unused %s
-// CHECK-everything: 6 warnings generated
+// CHECK-everything: 7 warnings generated
 // CHECK-everything-error: 5 errors generated
 // CHECK-everything-no-unused: 5 warnings generated
 
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -14632,8 +14632,10 @@
 Diag(FD->getLocation(), diag::ext_pure_function_definition);
 
   if (!FD->isInvalidDecl()) {
-// Don't diagnose unused parameters of defaulted or deleted functions.
-if (!FD->isDeleted() && !FD->isDefaulted() && !FD->hasSkippedBody())
+// Don't diagnose unused parameters of defaulted, deleted or naked
+// functions.
+if (!FD->isDeleted() && !FD->isDefaulted() && !FD->hasSkippedBody() &&
+!FD->hasAttr())
   DiagnoseUnusedParameters(FD->parameters());
 DiagnoseSizeOfParametersAndReturnValue(FD->parameters(),
FD->getReturnType(), FD);
Index: clang-tools-extra/test/clang-tidy/checkers/misc-unused-parameters.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/misc-unused-parameters.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/misc-unused-parameters.cpp
@@ -286,3 +286,7 @@
   f([](int I) { return; });
 }
 } // namespace lambda
+
+// Do not warn on naked functions.
+[[gnu::naked]] int nakedFunction(int a, float b, const char *c) { ; }
+__attribute__((naked)) void nakedFunction(int a, int b) { ; }
Index: clang-tools-extra/test/clang-tidy/checkers/misc-unused-parameters.c
===
--- clang-tools-extra/test/clang-tidy/checkers/misc-unused-parameters.c
+++ clang-tools-extra/test/clang-tidy/checkers/misc-unused-parameters.c
@@ -15,3 +15,5 @@
 // ===
 void h(i, c, d) int i; char *c, *d; {} // Don't mess with K&R style
 
+// Do not warn on naked functions.
+__attribute__((naked)) void nakedFunction(int a, int b) { ; }
Index: clang-tools-extra/test/clang-tidy/checkers/misc-unused-parameters-strict.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/misc-unused-parameters-strict.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/misc-unused-parameters-strict.cpp
@@ -22,4 +22,8 @@
 // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: parameter 'j' is unused
 // CHECK-FIXES: {{^}}  F(int  /*j*/) : i() {}{{$}}
 };
+
+// Do not warn on naked functions.
+[[gnu::naked]] int nakedFunction(int a, float b, const char *c) { ; }
+__attribute__((naked)) void nakedFunction(int a, int b) { ; }
 }
Index: clang-tools-extra/clang-tidy/misc/UnusedParametersCheck.cpp
===
--- clang-tools-extra/clang-tidy/misc/Unu

[PATCH] D116778: [clang-tidy][clang] Don't trigger unused-parameter warnings on naked functions

[Tommaso Bonvicini via Phabricator via cfe-commits]

MuAlphaOmegaEpsilon added inline comments.



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/misc-unused-parameters.cpp:290-292
+// Do not warn on naked functions.
+[[gnu::naked]] int nakedFunction(int a, float b, const char *c) { ; }
+__attribute__((naked)) void nakedFunction(int a, int b) { ; }

aaron.ballman wrote:
> Quuxplusone wrote:
> > In C++, I would expect the programmer to fix the (correct) warning simply 
> > by eliminating the unused parameter names:
> > ```
> > [[gnu::naked]] int nakedFunction(int, float, const char *) { ; }
> > __attribute__((naked)) void nakedFunction(int, int) { ; }
> > ```
> I wouldn't expect that consistently. I'd expect the programmer to see "unused 
> parameter, oh, I should remove that" at least some significant percentage of 
> the time, but with mixed results.
> 
> I think silencing the warning is a reasonable behavior in the presence of the 
> attribute. Naked functions are pretty strange beasts to begin with.
I replied to this stream with a basic comment instead of using an inline one...
I'm sorry, I just figured out how this platform works with these drafts! 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116778

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D116778: [clang-tidy][clang] Don't trigger unused-parameter warnings on naked functions

[Tommaso Bonvicini via Phabricator via cfe-commits]

MuAlphaOmegaEpsilon added a comment.

Let me know if I should rebase this onto the latest main branch, at the moment 
the Windows build is passing but the Debian build is not, failing at 
`compiler-rt/test/sanitizer_common/tsan-x86_64-Linux/Linux/Output/decorate_proc_maps.cpp`
 for no immediately apparent reason.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116778

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits