[Libreoffice-commits] core.git: compilerplugins/clang
Stephan Bergmann
sbergman at redhat.com
Fri Nov 10 23:05:10 UTC 2017
compilerplugins/clang/overrideparam.cxx | 51 +--------------
compilerplugins/clang/plugin.cxx | 69 +++++++++++++++++++++
compilerplugins/clang/plugin.hxx | 6 +
compilerplugins/clang/test/unnecessaryoverride.cxx | 33 ++++++++++
compilerplugins/clang/unnecessaryoverride.cxx | 16 +++-
5 files changed, 125 insertions(+), 50 deletions(-)
New commits:
commit 5d12237d79f289a1dcf8e07aa03df329e136f078
Author: Stephan Bergmann <sbergman at redhat.com>
Date: Fri Nov 10 13:40:12 2017 +0100
loplugin:unnecessaryoverride: suppress warnings when default args differ
...instead of blacklisting such cases. Reuses the
checkIdenticalDefaultArguments code that was originally in
loplugin:overrideparam (and appears to work reasonably well for the default
arguments that actually happen in practice).
Change-Id: I9cf2db17101beb135b2039a9b7ed335bd2af2c08
Reviewed-on: https://gerrit.libreoffice.org/44594
Tested-by: Jenkins <ci at libreoffice.org>
Reviewed-by: Stephan Bergmann <sbergman at redhat.com>
diff --git a/compilerplugins/clang/overrideparam.cxx b/compilerplugins/clang/overrideparam.cxx
index e3b8a4e0fd88..820127e1343d 100644
--- a/compilerplugins/clang/overrideparam.cxx
+++ b/compilerplugins/clang/overrideparam.cxx
@@ -38,7 +38,6 @@ public:
private:
bool hasSameDefaultParams(const ParmVarDecl * parmVarDecl, const ParmVarDecl * superParmVarDecl);
- bool evaluate(const Expr* expr, APSInt& x);
};
void OverrideParam::run()
@@ -148,39 +147,10 @@ bool OverrideParam::hasSameDefaultParams(const ParmVarDecl * parmVarDecl, const
if (parmVarDecl->hasUninstantiatedDefaultArg() || superParmVarDecl->hasUninstantiatedDefaultArg()) {
return true;
}
- const Expr* defaultArgExpr = parmVarDecl->getDefaultArg();
- const Expr* superDefaultArgExpr = superParmVarDecl->getDefaultArg();
-
- if (defaultArgExpr->isNullPointerConstant(compiler.getASTContext(), Expr::NPC_NeverValueDependent)
- && superDefaultArgExpr->isNullPointerConstant(compiler.getASTContext(), Expr::NPC_NeverValueDependent))
- {
- return true;
- }
- APSInt x1, x2;
- if (evaluate(defaultArgExpr, x1) && evaluate(superDefaultArgExpr, x2))
- {
- return x1 == x2;
- }
-#if CLANG_VERSION >= 30900
- APFloat f1(0.0f), f2(0.0f);
- if (defaultArgExpr->EvaluateAsFloat(f1, compiler.getASTContext())
- && superDefaultArgExpr->EvaluateAsFloat(f2, compiler.getASTContext()))
- {
- return f1.bitwiseIsEqual(f2);
- }
-#endif
- // catch params with defaults like "= OUString()"
- if (isa<MaterializeTemporaryExpr>(defaultArgExpr)
- && isa<MaterializeTemporaryExpr>(superDefaultArgExpr))
- {
- return true;
- }
- if (isa<CXXBindTemporaryExpr>(defaultArgExpr)
- && isa<CXXBindTemporaryExpr>(superDefaultArgExpr))
- {
- return true;
- }
- return true;
+ return
+ checkIdenticalDefaultArguments(
+ parmVarDecl->getDefaultArg(), superParmVarDecl->getDefaultArg())
+ != IdenticalDefaultArgumentsResult::No;
// for one, Clang 3.8 doesn't have EvaluateAsFloat; for another, since
// <http://llvm.org/viewvc/llvm-project?view=revision&revision=291318>
// "PR23135: Don't instantiate constexpr functions referenced in
@@ -196,19 +166,6 @@ bool OverrideParam::hasSameDefaultParams(const ParmVarDecl * parmVarDecl, const
// that would probably have unwanted side-effects)
}
-bool OverrideParam::evaluate(const Expr* expr, APSInt& x)
-{
- if (expr->EvaluateAsInt(x, compiler.getASTContext()))
- {
- return true;
- }
- if (isa<CXXNullPtrLiteralExpr>(expr)) {
- x = 0;
- return true;
- }
- return false;
-}
-
loplugin::Plugin::Registration< OverrideParam > X("overrideparam");
}
diff --git a/compilerplugins/clang/plugin.cxx b/compilerplugins/clang/plugin.cxx
index f8292ef661f6..6697f94b3ee8 100644
--- a/compilerplugins/clang/plugin.cxx
+++ b/compilerplugins/clang/plugin.cxx
@@ -75,6 +75,19 @@ void Plugin::registerPlugin( Plugin* (*create)( const InstantiationData& ), cons
PluginHandler::registerPlugin( create, optionName, isPPCallback, byDefault );
}
+bool Plugin::evaluate(const Expr* expr, APSInt& x)
+{
+ if (expr->EvaluateAsInt(x, compiler.getASTContext()))
+ {
+ return true;
+ }
+ if (isa<CXXNullPtrLiteralExpr>(expr)) {
+ x = 0;
+ return true;
+ }
+ return false;
+}
+
const Stmt* Plugin::getParentStmt( const Stmt* stmt )
{
auto parentsRange = compiler.getASTContext().getParents(*stmt);
@@ -203,6 +216,62 @@ bool Plugin::containsPreprocessingConditionalInclusion(SourceRange range)
return false;
}
+Plugin::IdenticalDefaultArgumentsResult Plugin::checkIdenticalDefaultArguments(
+ Expr const * argument1, Expr const * argument2)
+{
+ if ((argument1 == nullptr) != (argument2 == nullptr)) {
+ return IdenticalDefaultArgumentsResult::No;
+ }
+ if (argument1 == nullptr) {
+ return IdenticalDefaultArgumentsResult::Yes;
+ }
+ if (argument1->isNullPointerConstant(compiler.getASTContext(), Expr::NPC_NeverValueDependent)
+ && argument2->isNullPointerConstant(compiler.getASTContext(), Expr::NPC_NeverValueDependent))
+ {
+ return IdenticalDefaultArgumentsResult::Yes;
+ }
+ APSInt x1, x2;
+ if (evaluate(argument1, x1) && evaluate(argument2, x2))
+ {
+ return x1 == x2
+ ? IdenticalDefaultArgumentsResult::Yes
+ : IdenticalDefaultArgumentsResult::No;
+ }
+#if CLANG_VERSION >= 30900
+ APFloat f1(0.0f), f2(0.0f);
+ if (argument1->EvaluateAsFloat(f1, compiler.getASTContext())
+ && argument2->EvaluateAsFloat(f2, compiler.getASTContext()))
+ {
+ return f1.bitwiseIsEqual(f2)
+ ? IdenticalDefaultArgumentsResult::Yes
+ : IdenticalDefaultArgumentsResult::No;
+ }
+#endif
+ if (auto const lit1 = dyn_cast<clang::StringLiteral>(
+ argument1->IgnoreParenImpCasts()))
+ {
+ if (auto const lit2 = dyn_cast<clang::StringLiteral>(
+ argument2->IgnoreParenImpCasts()))
+ {
+ return lit1->getBytes() == lit2->getBytes()
+ ? IdenticalDefaultArgumentsResult::Yes
+ : IdenticalDefaultArgumentsResult::No;
+ }
+ }
+ // catch params with defaults like "= OUString()"
+ if (isa<MaterializeTemporaryExpr>(argument1)
+ && isa<MaterializeTemporaryExpr>(argument2))
+ {
+ return IdenticalDefaultArgumentsResult::Yes;
+ }
+ if (isa<CXXBindTemporaryExpr>(argument1)
+ && isa<CXXBindTemporaryExpr>(argument2))
+ {
+ return IdenticalDefaultArgumentsResult::Yes;
+ }
+ return IdenticalDefaultArgumentsResult::Maybe;
+}
+
RewritePlugin::RewritePlugin( const InstantiationData& data )
: Plugin( data )
, rewriter( data.rewriter )
diff --git a/compilerplugins/clang/plugin.hxx b/compilerplugins/clang/plugin.hxx
index 8a8f6f60732e..02897031cd7b 100644
--- a/compilerplugins/clang/plugin.hxx
+++ b/compilerplugins/clang/plugin.hxx
@@ -82,9 +82,15 @@ protected:
bool containsPreprocessingConditionalInclusion(SourceRange range);
+ enum class IdenticalDefaultArgumentsResult { No, Yes, Maybe };
+ IdenticalDefaultArgumentsResult checkIdenticalDefaultArguments(
+ Expr const * argument1, Expr const * argument2);
+
private:
static void registerPlugin( Plugin* (*create)( const InstantiationData& ), const char* optionName, bool isPPCallback, bool byDefault );
template< typename T > static Plugin* createHelper( const InstantiationData& data );
+ bool evaluate(const Expr* expr, APSInt& x);
+
enum { isRewriter = false };
const char* name;
};
diff --git a/compilerplugins/clang/test/unnecessaryoverride.cxx b/compilerplugins/clang/test/unnecessaryoverride.cxx
index 91e8a4003296..816feb9d6af4 100644
--- a/compilerplugins/clang/test/unnecessaryoverride.cxx
+++ b/compilerplugins/clang/test/unnecessaryoverride.cxx
@@ -7,6 +7,10 @@
* file, You can obtain one at http://mozilla.org/MPL/2.0/.
*/
+#include <sal/config.h>
+
+#include <config_clang.h>
+
struct Base
{
virtual ~Base();
@@ -15,6 +19,7 @@ struct Base
void cv() const volatile;
void ref();
static void staticFn();
+ void defaults(void* = nullptr, int = 0, double = 1, Base const& = {}, char const* = "foo");
};
struct SimpleDerived : Base
@@ -55,6 +60,34 @@ struct DerivedDifferent : Base
void cv() { Base::cv(); } // no warning
void ref() && { Base::ref(); } // no warning
void staticFn() { Base::staticFn(); } // no warning
+ void defaults(void* x1, int x2, double x3, Base const& x4, char const* x5)
+ {
+ Base::defaults(x1, x2, x3, x4, x5); // no warning
+ }
+};
+
+struct DerivedSame : Base
+{
+#if CLANG_VERSION >= 30900 // cf. corresponding condition in Plugin::checkIdenticalDefaultArguments
+ void
+ defaults( // expected-error {{public function just calls public parent [loplugin:unnecessaryoverride]}}
+ void* x1 = 0, int x2 = (1 - 1), double x3 = 1.0, Base const& x4 = (Base()),
+ char const* x5 = "f"
+ "oo")
+ {
+ Base::defaults(x1, x2, x3, x4, x5);
+ }
+#endif
+};
+
+struct DerivedSlightlyDifferent : Base
+{
+ void defaults( // no warning
+ void* x1 = nullptr, int x2 = 0, double x3 = 1, Base const& x4 = DerivedSlightlyDifferent(),
+ char const* x5 = "foo")
+ {
+ Base::defaults(x1, x2, x3, x4, x5);
+ }
};
/* vim:set shiftwidth=4 softtabstop=4 expandtab cinoptions=b1,g0,N-s cinkeys+=0=break: */
diff --git a/compilerplugins/clang/unnecessaryoverride.cxx b/compilerplugins/clang/unnecessaryoverride.cxx
index 07f902158efd..13ed723385d8 100644
--- a/compilerplugins/clang/unnecessaryoverride.cxx
+++ b/compilerplugins/clang/unnecessaryoverride.cxx
@@ -258,15 +258,25 @@ bool UnnecessaryOverride::VisitCXXMethodDecl(const CXXMethodDecl* methodDecl)
return true;
}
}
- // some very creative method hiding going on here
- if (loplugin::isSamePathname(aFileName, SRCDIR "/svx/source/dialog/checklbx.cxx"))
- return true;
const CXXMethodDecl* overriddenMethodDecl = findOverriddenOrSimilarMethodInSuperclasses(methodDecl);
if (!overriddenMethodDecl) {
return true;
}
+ // Check for differences in default parameters:
+ unsigned const numParams = methodDecl->getNumParams();
+ assert(overriddenMethodDecl->getNumParams() == numParams);
+ for (unsigned i = 0; i != numParams; ++i) {
+ if (checkIdenticalDefaultArguments(
+ methodDecl->getParamDecl(i)->getDefaultArg(),
+ overriddenMethodDecl->getParamDecl(i)->getDefaultArg())
+ != IdenticalDefaultArgumentsResult::Yes)
+ {
+ return true;
+ }
+ }
+
if (compat::getReturnType(*methodDecl).getCanonicalType()
!= compat::getReturnType(*overriddenMethodDecl).getCanonicalType())
{
More information about the Libreoffice-commits
mailing list