diff options
author | Noel Grandin <noel.grandin@collabora.co.uk> | 2019-08-27 11:06:41 +0200 |
---|---|---|
committer | Noel Grandin <noel.grandin@collabora.co.uk> | 2019-08-27 18:42:13 +0200 |
commit | 9e0b3423f21c23a8ef2fe749ea7dd7c1194c2f9a (patch) | |
tree | 26f2674f88359c5e6c3e1d0f888c7b31836c36ee /compilerplugins | |
parent | bbd500e14fce92b27cfc09e7cffd346e36eb5fb0 (diff) |
loplugin:referencecasting find more redundant static_cast
Change-Id: I3a51812bbd3fcdc6b11e47cb12962f0d4fa7a2ae
Reviewed-on: https://gerrit.libreoffice.org/78191
Tested-by: Jenkins
Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk>
Diffstat (limited to 'compilerplugins')
-rw-r--r-- | compilerplugins/clang/plugin.cxx | 50 | ||||
-rw-r--r-- | compilerplugins/clang/plugin.hxx | 6 | ||||
-rw-r--r-- | compilerplugins/clang/redundantcast.cxx | 54 | ||||
-rw-r--r-- | compilerplugins/clang/referencecasting.cxx | 42 | ||||
-rw-r--r-- | compilerplugins/clang/test/redundantcast.cxx | 14 |
5 files changed, 125 insertions, 41 deletions
diff --git a/compilerplugins/clang/plugin.cxx b/compilerplugins/clang/plugin.cxx index 7e8dfb664b15..bf48e8822a03 100644 --- a/compilerplugins/clang/plugin.cxx +++ b/compilerplugins/clang/plugin.cxx @@ -697,6 +697,56 @@ bool hasCLanguageLinkageType(FunctionDecl const * decl) { return false; } +static const CXXRecordDecl* stripTypeSugar(QualType qt) +{ + const clang::Type* t = qt.getTypePtr(); + while (auto elaboratedType = dyn_cast<ElaboratedType>(t)) + t = elaboratedType->desugar().getTypePtr(); + auto recordType = dyn_cast<RecordType>(t); + if (!recordType) + return nullptr; + return dyn_cast_or_null<CXXRecordDecl>(recordType->getDecl()); +} + +int derivedFromCount(const CXXRecordDecl* subclassRecordDecl, const CXXRecordDecl* baseclassRecordDecl) +{ + if (!subclassRecordDecl || !baseclassRecordDecl) + return 0; + int derivedCount = 0; + if (subclassRecordDecl == baseclassRecordDecl) + derivedCount++; + if (!subclassRecordDecl->hasDefinition()) + return derivedCount; + for (auto it = subclassRecordDecl->bases_begin(); it != subclassRecordDecl->bases_end(); ++it) + { + derivedCount += derivedFromCount(stripTypeSugar(it->getType()), baseclassRecordDecl); + // short-circuit, we only care about 0,1,2 + if (derivedCount > 1) + return derivedCount; + } + for (auto it = subclassRecordDecl->vbases_begin(); it != subclassRecordDecl->vbases_end(); ++it) + { + derivedCount += derivedFromCount(stripTypeSugar(it->getType()), baseclassRecordDecl); + // short-circuit, we only care about 0,1,2 + if (derivedCount > 1) + return derivedCount; + } + return derivedCount; +} + +int derivedFromCount(QualType subclassQt, QualType baseclassQt) +{ + auto baseclassRecordDecl = stripTypeSugar(baseclassQt); + if (!baseclassRecordDecl) + return 0; + auto subclassRecordDecl = stripTypeSugar(subclassQt); + if (!subclassRecordDecl) + return 0; + + return derivedFromCount(subclassRecordDecl, baseclassRecordDecl); +} + + } // namespace /* vim:set shiftwidth=4 softtabstop=4 expandtab: */ diff --git a/compilerplugins/clang/plugin.hxx b/compilerplugins/clang/plugin.hxx index 12ae6f7ba7c2..0712a22afc2a 100644 --- a/compilerplugins/clang/plugin.hxx +++ b/compilerplugins/clang/plugin.hxx @@ -283,6 +283,12 @@ bool isSamePathname(StringRef pathname, StringRef other); // "Language linkage of function type": bool hasCLanguageLinkageType(FunctionDecl const * decl); +// Count the number of times the base class is present in the subclass hierarchy +// +int derivedFromCount(clang::QualType subclassType, clang::QualType baseclassType); +int derivedFromCount(const CXXRecordDecl* subtypeRecord, const CXXRecordDecl* baseRecord); + + } // namespace #endif // COMPILEPLUGIN_H diff --git a/compilerplugins/clang/redundantcast.cxx b/compilerplugins/clang/redundantcast.cxx index 3d9ff97b473b..d4e76846cbfa 100644 --- a/compilerplugins/clang/redundantcast.cxx +++ b/compilerplugins/clang/redundantcast.cxx @@ -28,6 +28,7 @@ #include "check.hxx" #include "compat.hxx" #include "plugin.hxx" +#include <iostream> namespace { @@ -136,8 +137,12 @@ public: bool VisitBinNE(BinaryOperator const * expr) { return visitBinOp(expr); } + bool VisitBinAssign(BinaryOperator const * binaryOperator); + bool VisitVarDecl(VarDecl const * varDecl); + private: bool visitBinOp(BinaryOperator const * expr); + void visitAssign(QualType lhs, Expr const * rhs); bool isOverloadedFunction(FunctionDecl const * decl); }; @@ -334,6 +339,55 @@ bool RedundantCast::VisitCStyleCastExpr(CStyleCastExpr const * expr) { return true; } +bool RedundantCast::VisitBinAssign(BinaryOperator const * binaryOperator) { + if (ignoreLocation(binaryOperator)) { + return true; + } + visitAssign(binaryOperator->getLHS()->getType(), binaryOperator->getRHS()); + return true; +} + +bool RedundantCast::VisitVarDecl(VarDecl const * varDecl) { + if (ignoreLocation(varDecl)) { + return true; + } + if (!varDecl->getInit()) + return true; + visitAssign(varDecl->getType(), varDecl->getInit()); + return true; +} + +void RedundantCast::visitAssign(QualType t1, Expr const * rhs) +{ + auto staticCastExpr = dyn_cast<CXXStaticCastExpr>(rhs->IgnoreImplicit()); + if (!staticCastExpr) + return; + + auto const t2 = staticCastExpr->getSubExpr()->IgnoreImplicit()->getType(); + + // if there is more than one copy of the LHS, this cast is resolving ambiguity + bool foundOne = false; + if (t1->isRecordType()) + { + foundOne = loplugin::derivedFromCount(t2, t1) == 1; + } + else + { + auto pointee1 = t1->getPointeeCXXRecordDecl(); + auto pointee2 = t2->getPointeeCXXRecordDecl(); + if (pointee1 && pointee2) + foundOne = loplugin::derivedFromCount(pointee2, pointee1) == 1; + } + + if (foundOne) + { + report( + DiagnosticsEngine::Warning, "redundant static_cast from %0 to %1", + staticCastExpr->getExprLoc()) + << t2 << t1 << staticCastExpr->getSourceRange(); + } +} + bool RedundantCast::VisitCXXStaticCastExpr(CXXStaticCastExpr const * expr) { if (ignoreLocation(expr)) { return true; diff --git a/compilerplugins/clang/referencecasting.cxx b/compilerplugins/clang/referencecasting.cxx index e5e64f1d133e..6b1e3a47e920 100644 --- a/compilerplugins/clang/referencecasting.cxx +++ b/compilerplugins/clang/referencecasting.cxx @@ -342,46 +342,6 @@ static const RecordType* extractTemplateType(const clang::Type* cceType) return dyn_cast<RecordType>(templateParamType); } -static int derivedFromCount(QualType qt, const CXXRecordDecl* baseRecord); - -static int derivedFromCount(const CXXRecordDecl* subtypeRecord, const CXXRecordDecl* baseRecord) -{ - if (!subtypeRecord->hasDefinition()) - return 0; - int derivedCount = 0; - for (auto it = subtypeRecord->bases_begin(); it != subtypeRecord->bases_end(); ++it) - { - derivedCount += derivedFromCount(it->getType(), baseRecord); - if (derivedCount > 1) - return derivedCount; - } - for (auto it = subtypeRecord->vbases_begin(); it != subtypeRecord->vbases_end(); ++it) - { - derivedCount += derivedFromCount(it->getType(), baseRecord); - if (derivedCount > 1) - return derivedCount; - } - return derivedCount; -} - -static int derivedFromCount(QualType qt, const CXXRecordDecl* baseRecord) -{ - int derivedCount = 0; - auto type = qt.getTypePtr(); - if (auto elaboratedType = dyn_cast<ElaboratedType>(type)) - type = elaboratedType->desugar().getTypePtr(); - if (auto recordType = dyn_cast<RecordType>(type)) - { - if (auto cxxRecordDecl = dyn_cast<CXXRecordDecl>(recordType->getDecl())) - { - if (cxxRecordDecl->Equals(baseRecord)) - derivedCount++; - derivedCount += derivedFromCount(cxxRecordDecl, baseRecord); - } - } - return derivedCount; -} - /** Implement my own isDerived because we can't always see all the definitions of the classes involved. which will cause an assert with the normal clang isDerivedFrom code. @@ -390,7 +350,7 @@ static bool isDerivedFrom(const CXXRecordDecl* subtypeRecord, const CXXRecordDec { // if there is more than one case, then we have an ambiguous conversion, and we can't change the code // to use the upcasting constructor. - return derivedFromCount(subtypeRecord, baseRecord) == 1; + return loplugin::derivedFromCount(subtypeRecord, baseRecord) == 1; } loplugin::Plugin::Registration<ReferenceCasting> referencecasting("referencecasting"); diff --git a/compilerplugins/clang/test/redundantcast.cxx b/compilerplugins/clang/test/redundantcast.cxx index be34a57729c6..a876fae6e4b3 100644 --- a/compilerplugins/clang/test/redundantcast.cxx +++ b/compilerplugins/clang/test/redundantcast.cxx @@ -392,6 +392,20 @@ void testArrayDecay() { (void) reinterpret_cast<char const *>(u8""); } +void testNew() { + class A {}; + class B : public A {}; + A* p = static_cast<A*>(new B); // expected-error {{redundant static_cast from 'B *' to 'A *' [loplugin:redundantcast]}} + (void)p; + // no warning expected for resolving-ambiguity cast + class C : public A {}; + class D : public B, public C {}; + p = static_cast<B*>(new D); + // no warning expected for down-cast + auto p2 = static_cast<B*>(p); + (void)p2; +} + int main() { testConstCast(); testStaticCast(); |