[clang][Sema] Refine unused-member-function diagnostic message for constructors #84515
Conversation
|
Thank you for submitting a Pull Request (PR) to the LLVM Project! This PR will be automatically labeled and the relevant teams will be If you wish to, you can add reviewers by using the "Reviewers" section on this page. If this is not working for you, it is probably because you do not have write If you have received no comments on your PR for a week, you can request a review If you have further questions, they may be answered by the LLVM GitHub User Guide. You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums. |
llvmbot
added
clang
|
@llvm/pr-subscribers-clang Author: None (guillem-bartina-sonarsource) ChangesThe current diagnostic message is Full diff: https://github.com/llvm/llvm-project/pull/84515.diff 3 Files Affected:
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 6da49facd27ec2..770bda6b3ba193 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -400,7 +400,7 @@ def warn_unused_function : Warning<"unused function %0">,
InGroup<UnusedFunction>, DefaultIgnore;
def warn_unused_template : Warning<"unused %select{function|variable}0 template %1">,
InGroup<UnusedTemplate>, DefaultIgnore;
-def warn_unused_member_function : Warning<"unused member function %0">,
+def warn_unused_member_function : Warning<"unused %select{member function|constructor}0 %1">,
InGroup<UnusedMemberFunction>, DefaultIgnore;
def warn_used_but_marked_unused: Warning<"%0 was marked unused but was used">,
InGroup<UsedButMarkedUnused>, DefaultIgnore;
diff --git a/clang/lib/Sema/Sema.cpp b/clang/lib/Sema/Sema.cpp
index 720d5fd5f0428d..163ab48998d5e0 100644
--- a/clang/lib/Sema/Sema.cpp
+++ b/clang/lib/Sema/Sema.cpp
@@ -1398,11 +1398,16 @@ void Sema::ActOnEndOfTranslationUnit() {
if (FD->getDescribedFunctionTemplate())
Diag(DiagD->getLocation(), diag::warn_unused_template)
<< /*function=*/0 << DiagD << DiagRange;
- else
- Diag(DiagD->getLocation(), isa<CXXMethodDecl>(DiagD)
- ? diag::warn_unused_member_function
- : diag::warn_unused_function)
- << DiagD << DiagRange;
+ else {
+ if (isa<CXXMethodDecl>(DiagD))
+ Diag(DiagD->getLocation(), diag::warn_unused_member_function)
+ << (!isa<CXXConstructorDecl>(DiagD) ? /*member function=*/0
+ : /*constructor=*/1)
+ << DiagD << DiagRange;
+ else
+ Diag(DiagD->getLocation(), diag::warn_unused_function)
+ << DiagD << DiagRange;
+ }
}
} else {
const VarDecl *DiagD = cast<VarDecl>(*I)->getDefinition();
diff --git a/clang/test/SemaCXX/warn-unused-filescoped.cpp b/clang/test/SemaCXX/warn-unused-filescoped.cpp
index be8d350855c078..b3d1bb4661a5f4 100644
--- a/clang/test/SemaCXX/warn-unused-filescoped.cpp
+++ b/clang/test/SemaCXX/warn-unused-filescoped.cpp
@@ -76,10 +76,33 @@ struct S {
struct SVS : public VS {
void vm() { }
};
+
+ struct CS {
+ CS() {}
+ CS(bool a) {}
+ CS(int b) {} // expected-warning{{unused constructor 'CS'}}
+ CS(float c);
+ };
+
+ struct DCS : public CS {
+ DCS() = default; // expected-warning{{unused constructor 'DCS'}}
+ DCS(bool a) : CS(a) {} // expected-warning{{unused constructor 'DCS'}}
+ DCS(const DCS&) {}
+ DCS(DCS&&) {} // expected-warning{{unused constructor 'DCS'}}
+ };
+
+ template<typename T>
+ struct TCS {
+ TCS();
+ };
+ template <typename T> TCS<T>::TCS() {}
+ template <> TCS<int>::TCS() {} // expected-warning{{unused constructor 'TCS'}}
}
void S::m3() {} // expected-warning{{unused member function 'm3'}}
+CS::CS(float c) {} // expected-warning{{unused constructor 'CS'}}
+
static inline void f4() {} // expected-warning{{unused function 'f4'}}
const unsigned int cx = 0; // expected-warning{{unused variable 'cx'}}
const unsigned int cy = 0;
|
|
Looks like the build failed b/c you did not run |
clang/lib/Sema/Sema.cpp
Outdated
| else { | ||
| if (isa<CXXMethodDecl>(DiagD)) | ||
| Diag(DiagD->getLocation(), diag::warn_unused_member_function) | ||
| << (!isa<CXXConstructorDecl>(DiagD) ? /*member function=*/0 | ||
| : /*constructor=*/1) | ||
| << DiagD << DiagRange; | ||
| else | ||
| Diag(DiagD->getLocation(), diag::warn_unused_function) | ||
| << DiagD << DiagRange; | ||
| } |
There was a problem hiding this comment.
| else { | |
| if (isa<CXXMethodDecl>(DiagD)) | |
| Diag(DiagD->getLocation(), diag::warn_unused_member_function) | |
| << (!isa<CXXConstructorDecl>(DiagD) ? /*member function=*/0 | |
| : /*constructor=*/1) | |
| << DiagD << DiagRange; | |
| else | |
| Diag(DiagD->getLocation(), diag::warn_unused_function) | |
| << DiagD << DiagRange; | |
| } | |
| else { | |
| Diag(DiagD->getLocation(), isa<CXXMethodDecl>(DiagD) | |
| ? diag::warn_unused_member_function | |
| : diag::warn_unused_function) | |
| << DiagD | |
| << (!isa<CXXConstructorDecl>(DiagD) ? /*member function=*/0 | |
| : /*constructor=*/1 | |
| << DiagRange; | |
| } |
I think you can simplify like that.
And then change warn_unused_member_function : Warning<"unused %select{member function|constructor}1 %0"
There was a problem hiding this comment.
Note that warn_unused_function and warn_unused_member_function are two different diagnostics, with different messages. The former has only one argument, whereas the latter now has two (one of them selects between member function and constructor).
Unless I'm missing some hidden logic regarding diagnostic builders and the '<<' operator, we can't merge the two because the two diagnostics have different number of arguments.
guillem-bartina-sonarsource
force-pushed
the
gb/unused-constructor
branch
from
f8f93fe
to
473e8bb
Compare
|
Ping |
|
Please add a release note line to |
The current diagnostic message is
unused member function Afor every kind of method, including constructors. The idea is to refine the message tounused constructorwhen the method is a constructor.