[Libreoffice-commits] core.git: compilerplugins/clang solenv/CompilerTest_compilerplugins_clang.mk
Noel Grandin
noel.grandin at collabora.co.uk
Tue Jun 20 05:49:50 UTC 2017
compilerplugins/clang/plugin.cxx | 5 +
compilerplugins/clang/plugin.hxx | 2
compilerplugins/clang/pluginhandler.cxx | 16 ++++--
compilerplugins/clang/pluginhandler.hxx | 2
compilerplugins/clang/test/unusedfields.cxx | 28 ++++++++++
compilerplugins/clang/unusedfields.cxx | 70 ++++++++++++++++++++-------
solenv/CompilerTest_compilerplugins_clang.mk | 1
7 files changed, 102 insertions(+), 22 deletions(-)
New commits:
commit 3b60f59bc55824e247f6e751a9c8f5d253665f0b
Author: Noel Grandin <noel.grandin at collabora.co.uk>
Date: Mon Jun 19 13:44:14 2017 +0200
loplugin:unusedfields fix false positive
When the field in question is read from inside a constructor
initializer.
In the process, create some needed infrastructure in the plugin classes.
Change-Id: I2f440efa6912801a236727c9fe3180404616958c
Reviewed-on: https://gerrit.libreoffice.org/38960
Tested-by: Jenkins <ci at libreoffice.org>
Reviewed-by: Noel Grandin <noel.grandin at collabora.co.uk>
diff --git a/compilerplugins/clang/plugin.cxx b/compilerplugins/clang/plugin.cxx
index 3e1111c612ef..ea646d29da89 100644
--- a/compilerplugins/clang/plugin.cxx
+++ b/compilerplugins/clang/plugin.cxx
@@ -265,6 +265,11 @@ SourceLocation Plugin::locationAfterToken( SourceLocation location )
return Lexer::getLocForEndOfToken( location, 0, compiler.getSourceManager(), compiler.getLangOpts());
}
+bool Plugin::isUnitTestMode()
+ {
+ return PluginHandler::isUnitTestMode();
+ }
+
RewritePlugin::RewritePlugin( const InstantiationData& data )
: Plugin( data )
, rewriter( data.rewriter )
diff --git a/compilerplugins/clang/plugin.hxx b/compilerplugins/clang/plugin.hxx
index 413b2ab7cefb..5651b2ff02ca 100644
--- a/compilerplugins/clang/plugin.hxx
+++ b/compilerplugins/clang/plugin.hxx
@@ -78,6 +78,8 @@ class Plugin
bool isInUnoIncludeFile(const FunctionDecl*) const;
static void normalizeDotDotInFilePath(std::string&);
+
+ static bool isUnitTestMode();
private:
static void registerPlugin( Plugin* (*create)( const InstantiationData& ), const char* optionName, bool isPPCallback, bool byDefault );
template< typename T > static Plugin* createHelper( const InstantiationData& data );
diff --git a/compilerplugins/clang/pluginhandler.cxx b/compilerplugins/clang/pluginhandler.cxx
index ad043e87e58d..7c42d14f3056 100644
--- a/compilerplugins/clang/pluginhandler.cxx
+++ b/compilerplugins/clang/pluginhandler.cxx
@@ -56,13 +56,13 @@ const int MAX_PLUGINS = 100;
static PluginData plugins[ MAX_PLUGINS ];
static int pluginCount = 0;
static bool bPluginObjectsCreated = false;
+static bool unitTestMode = false;
PluginHandler::PluginHandler( CompilerInstance& compiler, const vector< string >& args )
: compiler( compiler )
, rewriter( compiler.getSourceManager(), compiler.getLangOpts())
, scope( "mainfile" )
, warningsAsErrors( false )
- , unitTestMode( false )
{
set< string > rewriters;
for( string const & arg : args )
@@ -89,6 +89,11 @@ PluginHandler::~PluginHandler()
}
}
+bool PluginHandler::isUnitTestMode()
+ {
+ return unitTestMode;
+ }
+
void PluginHandler::handleOption( const string& option )
{
if( option.substr( 0, 6 ) == "scope=" )
@@ -121,10 +126,13 @@ void PluginHandler::createPlugins( set< string > rewriters )
i < pluginCount;
++i )
{
- if( rewriters.erase( plugins[i].optionName ) != 0 )
- plugins[ i ].object = plugins[ i ].create( Plugin::InstantiationData { plugins[ i ].optionName, *this, compiler, &rewriter } );
+ const char* name = plugins[i].optionName;
+ if( rewriters.erase( name ) != 0 )
+ plugins[ i ].object = plugins[ i ].create( Plugin::InstantiationData { name, *this, compiler, &rewriter } );
else if( plugins[ i ].byDefault )
- plugins[ i ].object = plugins[ i ].create( Plugin::InstantiationData { plugins[ i ].optionName, *this, compiler, NULL } );
+ plugins[ i ].object = plugins[ i ].create( Plugin::InstantiationData { name, *this, compiler, NULL } );
+ else if( unitTestMode && strcmp(name, "unusedmethodsremove") != 0 && strcmp(name, "unusedfieldsremove") != 0)
+ plugins[ i ].object = plugins[ i ].create( Plugin::InstantiationData { name, *this, compiler, NULL } );
}
for( auto r: rewriters )
report( DiagnosticsEngine::Fatal, "unknown plugin tool %0" ) << r;
diff --git a/compilerplugins/clang/pluginhandler.hxx b/compilerplugins/clang/pluginhandler.hxx
index a2cc136f5751..222cb258e80f 100644
--- a/compilerplugins/clang/pluginhandler.hxx
+++ b/compilerplugins/clang/pluginhandler.hxx
@@ -37,6 +37,7 @@ class PluginHandler
DiagnosticBuilder report( DiagnosticsEngine::Level level, const char * plugin, StringRef message,
CompilerInstance& compiler, SourceLocation loc = SourceLocation());
bool addRemoval( SourceLocation loc );
+ static bool isUnitTestMode();
private:
void handleOption( const string& option );
void createPlugins( set< string > rewriters );
@@ -47,7 +48,6 @@ class PluginHandler
string scope;
string warningsOnly;
bool warningsAsErrors;
- bool unitTestMode;
};
/**
diff --git a/compilerplugins/clang/test/unusedfields.cxx b/compilerplugins/clang/test/unusedfields.cxx
new file mode 100644
index 000000000000..f549a9a37673
--- /dev/null
+++ b/compilerplugins/clang/test/unusedfields.cxx
@@ -0,0 +1,28 @@
+/* -*- Mode: C++; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4; fill-column: 100 -*- */
+/*
+ * This file is part of the LibreOffice project.
+ *
+ * This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/.
+ */
+
+#include <rtl/ustring.hxx>
+
+struct Foo
+// expected-error at -1 {{read m_foo1 [loplugin:unusedfields]}}
+{
+ int m_foo1;
+};
+
+struct Bar
+// expected-error at -1 {{read m_bar2 [loplugin:unusedfields]}}
+{
+ int m_bar1;
+ int m_bar2 = 1;
+ Bar(Foo const & foo) : m_bar1(foo.m_foo1) {}
+
+ int bar() { return m_bar2; }
+};
+
+/* vim:set shiftwidth=4 softtabstop=4 expandtab cinoptions=b1,g0,N-s cinkeys+=0=break: */
diff --git a/compilerplugins/clang/unusedfields.cxx b/compilerplugins/clang/unusedfields.cxx
index e77af78870ab..df64c987f347 100644
--- a/compilerplugins/clang/unusedfields.cxx
+++ b/compilerplugins/clang/unusedfields.cxx
@@ -44,6 +44,7 @@ namespace {
struct MyFieldInfo
{
+ const RecordDecl* parentRecord;
std::string parentClass;
std::string fieldName;
std::string fieldType;
@@ -71,10 +72,25 @@ class UnusedFields:
public:
explicit UnusedFields(InstantiationData const & data): Plugin(data) {}
- virtual void run() override
- {
- TraverseDecl(compiler.getASTContext().getTranslationUnitDecl());
+ virtual void run() override;
+
+ bool shouldVisitTemplateInstantiations () const { return true; }
+ bool shouldVisitImplicitCode() const { return true; }
+
+ bool VisitFieldDecl( const FieldDecl* );
+ bool VisitMemberExpr( const MemberExpr* );
+ bool VisitDeclRefExpr( const DeclRefExpr* );
+private:
+ MyFieldInfo niceName(const FieldDecl*);
+ void checkTouched(const FieldDecl* fieldDecl, const Expr* memberExpr);
+};
+void UnusedFields::run()
+{
+ TraverseDecl(compiler.getASTContext().getTranslationUnitDecl());
+
+ if (!isUnitTestMode())
+ {
// dump all our output in one write call - this is to try and limit IO "crosstalk" between multiple processes
// writing to the same logfile
std::string output;
@@ -95,17 +111,19 @@ public:
myfile << output;
myfile.close();
}
+ else
+ {
+ for (const MyFieldInfo & s : readFromSet)
+ {
+ report(
+ DiagnosticsEngine::Warning,
+ "read %0",
+ s.parentRecord->getLocStart())
+ << s.fieldName;
+ }
+ }
+}
- bool shouldVisitTemplateInstantiations () const { return true; }
- bool shouldVisitImplicitCode() const { return true; }
-
- bool VisitFieldDecl( const FieldDecl* );
- bool VisitMemberExpr( const MemberExpr* );
- bool VisitDeclRefExpr( const DeclRefExpr* );
-private:
- MyFieldInfo niceName(const FieldDecl*);
- void checkTouched(const FieldDecl* fieldDecl, const Expr* memberExpr);
-};
MyFieldInfo UnusedFields::niceName(const FieldDecl* fieldDecl)
{
@@ -117,10 +135,14 @@ MyFieldInfo UnusedFields::niceName(const FieldDecl* fieldDecl)
{
if (cxxRecordDecl->getTemplateInstantiationPattern())
cxxRecordDecl = cxxRecordDecl->getTemplateInstantiationPattern();
+ aInfo.parentRecord = cxxRecordDecl;
aInfo.parentClass = cxxRecordDecl->getQualifiedNameAsString();
}
else
+ {
+ aInfo.parentRecord = recordDecl;
aInfo.parentClass = recordDecl->getQualifiedNameAsString();
+ }
aInfo.fieldName = fieldDecl->getNameAsString();
// sometimes the name (if it's anonymous thing) contains the full path of the build folder, which we don't need
@@ -217,20 +239,33 @@ bool UnusedFields::VisitMemberExpr( const MemberExpr* memberExpr )
// for the write-only analysis
+ auto parentsRange = compiler.getASTContext().getParents(*memberExpr);
const Stmt* child = memberExpr;
- const Stmt* parent = parentStmt(memberExpr);
+ const Stmt* parent = parentsRange.begin() == parentsRange.end() ? nullptr : parentsRange.begin()->get<Stmt>();
// walk up the tree until we find something interesting
bool bPotentiallyReadFrom = false;
bool bDump = false;
do {
if (!parent) {
- return true;
+ // check if we're inside a CXXCtorInitializer
+ auto parentsRange = compiler.getASTContext().getParents(*child);
+ if ( parentsRange.begin() != parentsRange.end())
+ {
+ const Decl* decl = parentsRange.begin()->get<Decl>();
+ if (decl && isa<CXXConstructorDecl>(decl))
+ bPotentiallyReadFrom = true;
+ }
+ if (!bPotentiallyReadFrom)
+ return true;
+ else
+ break;
}
if (isa<CastExpr>(parent) || isa<MemberExpr>(parent) || isa<ParenExpr>(parent) || isa<ParenListExpr>(parent)
|| isa<ExprWithCleanups>(parent) || isa<UnaryOperator>(parent))
{
child = parent;
- parent = parentStmt(parent);
+ auto parentsRange = compiler.getASTContext().getParents(*parent);
+ parent = parentsRange.begin() == parentsRange.end() ? nullptr : parentsRange.begin()->get<Stmt>();
}
else if (isa<CaseStmt>(parent))
{
@@ -278,7 +313,8 @@ bool UnusedFields::VisitMemberExpr( const MemberExpr* memberExpr )
// so walk up the tree.
if (binaryOp->getLHS() == child && op == BO_Assign) {
child = parent;
- parent = parentStmt(parent);
+ auto parentsRange = compiler.getASTContext().getParents(*parent);
+ parent = parentsRange.begin() == parentsRange.end() ? nullptr : parentsRange.begin()->get<Stmt>();
} else {
bPotentiallyReadFrom = true;
break;
diff --git a/solenv/CompilerTest_compilerplugins_clang.mk b/solenv/CompilerTest_compilerplugins_clang.mk
index 4e65a684efd4..3de8b08b1cf7 100644
--- a/solenv/CompilerTest_compilerplugins_clang.mk
+++ b/solenv/CompilerTest_compilerplugins_clang.mk
@@ -27,6 +27,7 @@ $(eval $(call gb_CompilerTest_add_exception_objects,compilerplugins_clang, \
compilerplugins/clang/test/stringconstant \
compilerplugins/clang/test/unnecessaryoverride-dtor \
compilerplugins/clang/test/unoany \
+ compilerplugins/clang/test/unusedfields \
compilerplugins/clang/test/useuniqueptr \
compilerplugins/clang/test/vclwidgets \
))
More information about the Libreoffice-commits
mailing list