<div dir="ltr">Hi Eli,<div><br></div><div>Recently sanitizer-x86_64-linux buildbot which bootstraps Clang started failing with -Wunused warnings which looks like false positives to me:</div><div><a href="http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux/builds/848/steps/build%2064-bit%20llvm%20using%20clang/logs/stdio">http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux/builds/848/steps/build%2064-bit%20llvm%20using%20clang/logs/stdio</a></div>
<div><div><br></div><div><div>/build/llvm/lib/Transforms/Instrumentation/ThreadSanitizer.cpp:108:23: error: variable 'kNumberOfAccessSizes' is not needed and will not be emitted [-Werror,-Wunneeded-internal-declaration]</div>
<div> static const size_t kNumberOfAccessSizes = 5;</div></div></div><div><br></div><div>This static member is actually used to define array sizes, and in the function code as well.</div><div><br></div><div>Can this be related to your change?</div>
</div><div class="gmail_extra"><br><br><div class="gmail_quote">On Tue, Sep 10, 2013 at 7:05 AM, Eli Friedman <span dir="ltr"><<a href="mailto:eli.friedman@gmail.com" target="_blank">eli.friedman@gmail.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: efriedma<br>
Date: Mon Sep 9 22:05:56 2013<br>
New Revision: 190382<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=190382&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=190382&view=rev</a><br>
Log:<br>
Make -Wunused warning rules more consistent.<br>
<br>
This patch does a few different things.<br>
<br>
This patch improves unused var diags for const vars: we no longer<br>
unconditionally suppress diagnostics for const vars, instead only suppressing<br>
the diagnostic when the declaration appears to be useful.<br>
<br>
This patch also makes us more consistently use whether a variable/function<br>
is declared in the main file to suppress diagnostics where appropriate.<br>
<br>
Fixes <rdar://problem/14907887>.<br>
<br>
Modified:<br>
cfe/trunk/lib/Sema/Sema.cpp<br>
cfe/trunk/lib/Sema/SemaDecl.cpp<br>
cfe/trunk/test/SemaCXX/Inputs/warn-unused-variables.h<br>
cfe/trunk/test/SemaCXX/warn-unused-filescoped.cpp<br>
<br>
Modified: cfe/trunk/lib/Sema/Sema.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/Sema.cpp?rev=190382&r1=190381&r2=190382&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/Sema.cpp?rev=190382&r1=190381&r2=190382&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/lib/Sema/Sema.cpp (original)<br>
+++ cfe/trunk/lib/Sema/Sema.cpp Mon Sep 9 22:05:56 2013<br>
@@ -749,11 +749,7 @@ void Sema::ActOnEndOfTranslationUnit() {<br>
if (DiagD->isReferenced()) {<br>
Diag(DiagD->getLocation(), diag::warn_unneeded_internal_decl)<br>
<< /*variable*/1 << DiagD->getDeclName();<br>
- } else if (SourceMgr.isInMainFile(DiagD->getLocation())) {<br>
- // If the declaration is in a header which is included into multiple<br>
- // TUs, it will declare one variable per TU, and one of the other<br>
- // variables may be used. So, only warn if the declaration is in the<br>
- // main file.<br>
+ } else {<br>
Diag(DiagD->getLocation(), diag::warn_unused_variable)<br>
<< DiagD->getDeclName();<br>
}<br>
<br>
Modified: cfe/trunk/lib/Sema/SemaDecl.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDecl.cpp?rev=190382&r1=190381&r2=190382&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDecl.cpp?rev=190382&r1=190381&r2=190382&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/lib/Sema/SemaDecl.cpp (original)<br>
+++ cfe/trunk/lib/Sema/SemaDecl.cpp Mon Sep 9 22:05:56 2013<br>
@@ -1177,6 +1177,14 @@ bool Sema::mightHaveNonExternalLinkage(c<br>
return !D->isExternallyVisible();<br>
}<br>
<br>
+// FIXME: This needs to be refactored; some other isInMainFile users want<br>
+// these semantics.<br>
+static bool isMainFileLoc(const Sema &S, SourceLocation Loc) {<br>
+ if (S.TUKind != TU_Complete)<br>
+ return false;<br>
+ return S.SourceMgr.isInMainFile(Loc);<br>
+}<br>
+<br>
bool Sema::ShouldWarnIfUnusedFileScopedDecl(const DeclaratorDecl *D) const {<br>
assert(D);<br>
<br>
@@ -1196,12 +1204,9 @@ bool Sema::ShouldWarnIfUnusedFileScopedD<br>
if (MD->isVirtual() || IsDisallowedCopyOrAssign(MD))<br>
return false;<br>
} else {<br>
- // 'static inline' functions are used in headers; don't warn.<br>
- // Make sure we get the storage class from the canonical declaration,<br>
- // since otherwise we will get spurious warnings on specialized<br>
- // static template functions.<br>
- if (FD->getCanonicalDecl()->getStorageClass() == SC_Static &&<br>
- FD->isInlineSpecified())<br>
+ // 'static inline' functions are defined in headers; don't warn.<br>
+ if (FD->isInlineSpecified() &&<br>
+ !isMainFileLoc(*this, FD->getLocation()))<br>
return false;<br>
}<br>
<br>
@@ -1209,21 +1214,26 @@ bool Sema::ShouldWarnIfUnusedFileScopedD<br>
Context.DeclMustBeEmitted(FD))<br>
return false;<br>
} else if (const VarDecl *VD = dyn_cast<VarDecl>(D)) {<br>
- // Don't warn on variables of const-qualified or reference type, since their<br>
- // values can be used even if though they're not odr-used, and because const<br>
- // qualified variables can appear in headers in contexts where they're not<br>
- // intended to be used.<br>
- // FIXME: Use more principled rules for these exemptions.<br>
- if (!VD->isFileVarDecl() ||<br>
- VD->getType().isConstQualified() ||<br>
- VD->getType()->isReferenceType() ||<br>
- Context.DeclMustBeEmitted(VD))<br>
+ // Constants and utility variables are defined in headers with internal<br>
+ // linkage; don't warn. (Unlike functions, there isn't a convenient marker<br>
+ // like "inline".)<br>
+ if (!isMainFileLoc(*this, VD->getLocation()))<br>
+ return false;<br>
+<br>
+ // If a variable usable in constant expressions is referenced,<br>
+ // don't warn if it isn't used: if the value of a variable is required<br>
+ // for the computation of a constant expression, it doesn't make sense to<br>
+ // warn even if the variable isn't odr-used. (isReferenced doesn't<br>
+ // precisely reflect that, but it's a decent approximation.)<br>
+ if (VD->isReferenced() && VD->isUsableInConstantExpressions(Context))<br>
+ return false;<br>
+<br>
+ if (Context.DeclMustBeEmitted(VD))<br>
return false;<br>
<br>
if (VD->isStaticDataMember() &&<br>
VD->getTemplateSpecializationKind() == TSK_ImplicitInstantiation)<br>
return false;<br>
-<br>
} else {<br>
return false;<br>
}<br>
<br>
Modified: cfe/trunk/test/SemaCXX/Inputs/warn-unused-variables.h<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/Inputs/warn-unused-variables.h?rev=190382&r1=190381&r2=190382&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/Inputs/warn-unused-variables.h?rev=190382&r1=190381&r2=190382&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/test/SemaCXX/Inputs/warn-unused-variables.h (original)<br>
+++ cfe/trunk/test/SemaCXX/Inputs/warn-unused-variables.h Mon Sep 9 22:05:56 2013<br>
@@ -7,5 +7,7 @@ class A {};<br>
<br>
class B {<br>
static A a;<br>
+ static A b;<br>
+ static const int x = sizeof(b);<br>
};<br>
}<br>
<br>
Modified: cfe/trunk/test/SemaCXX/warn-unused-filescoped.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/warn-unused-filescoped.cpp?rev=190382&r1=190381&r2=190382&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/warn-unused-filescoped.cpp?rev=190382&r1=190381&r2=190382&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/test/SemaCXX/warn-unused-filescoped.cpp (original)<br>
+++ cfe/trunk/test/SemaCXX/warn-unused-filescoped.cpp Mon Sep 9 22:05:56 2013<br>
@@ -1,6 +1,41 @@<br>
// RUN: %clang_cc1 -fsyntax-only -verify -Wunused -Wunused-member-function -Wno-c++11-extensions -std=c++98 %s<br>
// RUN: %clang_cc1 -fsyntax-only -verify -Wunused -Wunused-member-function -std=c++11 %s<br>
<br>
+#ifdef HEADER<br>
+<br>
+static void headerstatic() {} // expected-warning{{unused}}<br>
+static inline void headerstaticinline() {}<br>
+<br>
+namespace {<br>
+ void headeranon() {} // expected-warning{{unused}}<br>
+ inline void headerinlineanon() {}<br>
+}<br>
+<br>
+namespace test7<br>
+{<br>
+ template<typename T><br>
+ static inline void foo(T) { }<br>
+<br>
+ // This should not emit an unused-function warning since it inherits<br>
+ // the static storage type from the base template.<br>
+ template<><br>
+ inline void foo(int) { }<br>
+<br>
+ // Partial specialization<br>
+ template<typename T, typename U><br>
+ static inline void bar(T, U) { }<br>
+<br>
+ template<typename U><br>
+ inline void bar(int, U) { }<br>
+<br>
+ template<><br>
+ inline void bar(int, int) { }<br>
+};<br>
+<br>
+#else<br>
+#define HEADER<br>
+#include "warn-unused-filescoped.cpp"<br>
+<br>
static void f1(); // expected-warning{{unused}}<br>
<br>
namespace {<br>
@@ -37,8 +72,10 @@ namespace {<br>
<br>
void S::m3() { } // expected-warning{{unused}}<br>
<br>
-static inline void f4() { }<br>
-const unsigned int cx = 0;<br>
+static inline void f4() { } // expected-warning{{unused}}<br>
+const unsigned int cx = 0; // expected-warning{{unused}}<br>
+const unsigned int cy = 0;<br>
+int f5() { return cy; }<br>
<br>
static int x1; // expected-warning{{unused}}<br>
<br>
@@ -98,7 +135,7 @@ namespace test5 {<br>
// FIXME: We should produce warnings for both of these.<br>
static const int m = n;<br>
int x = sizeof(m);<br>
- static const double d = 0.0;<br>
+ static const double d = 0.0; // expected-warning{{not needed and will not be emitted}}<br>
int y = sizeof(d);<br>
}<br>
<br>
@@ -133,27 +170,6 @@ namespace test6 {<br>
};<br>
}<br>
<br>
-namespace test7<br>
-{<br>
- template<typename T><br>
- static inline void foo(T) { }<br>
-<br>
- // This should not emit an unused-function warning since it inherits<br>
- // the static storage type from the base template.<br>
- template<><br>
- inline void foo(int) { }<br>
-<br>
- // Partial specialization<br>
- template<typename T, typename U><br>
- static inline void bar(T, U) { }<br>
-<br>
- template<typename U><br>
- inline void bar(int, U) { }<br>
-<br>
- template<><br>
- inline void bar(int, int) { }<br>
-};<br>
-<br>
namespace pr14776 {<br>
namespace {<br>
struct X {};<br>
@@ -161,3 +177,5 @@ namespace pr14776 {<br>
X a = X(); // expected-warning {{unused variable 'a'}}<br>
auto b = X(); // expected-warning {{unused variable 'b'}}<br>
}<br>
+<br>
+#endif<br>
<br>
<br>
_______________________________________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@cs.uiuc.edu">cfe-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits</a><br>
</blockquote></div><br><br clear="all"><div><br></div>-- <br><div>Alexey Samsonov, MSK</div>
</div>