From 8b1fcedcd10366523967ebcbd463b2c0c6b5b17a Mon Sep 17 00:00:00 2001 From: Stephan Bergmann Date: Fri, 19 May 2017 23:51:46 +0200 Subject: ...and re-eanble loplugin:stringcopy again "Make CastExpr::getSubExprAsWritten look through implicit temporary under CK_ConstructorConversion" was biting me again. (I had originally developed loplugin:stringcopy against a Clang build that includes my local fix for that issue. I really need to see to get that resolved upstream...) (And while 957874168491f4b030fda85c65dd969aae82a670 "loplugin:stringcopy" was actually a false positive, it doesn't hurt either, so just keep it.) Change-Id: I726956adfbe67681005173cfdfed2e4b4cd6253d --- compilerplugins/clang/compat.hxx | 73 +++++++++++++++++++++++++++++++ compilerplugins/clang/redundantcast.cxx | 77 ++------------------------------- compilerplugins/clang/stringcopy.cxx | 5 ++- 3 files changed, 79 insertions(+), 76 deletions(-) (limited to 'compilerplugins') diff --git a/compilerplugins/clang/compat.hxx b/compilerplugins/clang/compat.hxx index 28dbeec71e20..a493cdc7b209 100644 --- a/compilerplugins/clang/compat.hxx +++ b/compilerplugins/clang/compat.hxx @@ -17,6 +17,7 @@ #include "clang/AST/DeclBase.h" #include "clang/AST/DeclCXX.h" #include "clang/AST/Expr.h" +#include "clang/AST/ExprCXX.h" #include "clang/AST/Type.h" #include "clang/Basic/Diagnostic.h" #include "clang/Basic/DiagnosticIDs.h" @@ -244,6 +245,78 @@ inline bool isStdNamespace(clang::DeclContext const & context) { #endif } +// Work around : +// +// SfxErrorHandler::GetClassString (svtools/source/misc/ehdl.cxx): +// +// ErrorResource_Impl aEr(aId, (sal_uInt16)lClassId); +// if(aEr) +// { +// rStr = static_cast(aEr).GetString(); +// } +// +// expr->dump(): +// CXXStaticCastExpr 0x2b74e8e657b8 'class ResString' static_cast +// `-CXXBindTemporaryExpr 0x2b74e8e65798 'class ResString' (CXXTemporary 0x2b74e8e65790) +// `-CXXConstructExpr 0x2b74e8e65758 'class ResString' 'void (class ResString &&) noexcept(false)' elidable +// `-MaterializeTemporaryExpr 0x2b74e8e65740 'class ResString' xvalue +// `-CXXBindTemporaryExpr 0x2b74e8e65720 'class ResString' (CXXTemporary 0x2b74e8e65718) +// `-ImplicitCastExpr 0x2b74e8e65700 'class ResString' +// `-CXXMemberCallExpr 0x2b74e8e656d8 'class ResString' +// `-MemberExpr 0x2b74e8e656a0 '' .operator ResString 0x2b74e8dc1f00 +// `-DeclRefExpr 0x2b74e8e65648 'struct ErrorResource_Impl' lvalue Var 0x2b74e8e653b0 'aEr' 'struct ErrorResource_Impl' +// expr->getSubExprAsWritten()->dump(): +// MaterializeTemporaryExpr 0x2b74e8e65740 'class ResString' xvalue +// `-CXXBindTemporaryExpr 0x2b74e8e65720 'class ResString' (CXXTemporary 0x2b74e8e65718) +// `-ImplicitCastExpr 0x2b74e8e65700 'class ResString' +// `-CXXMemberCallExpr 0x2b74e8e656d8 'class ResString' +// `-MemberExpr 0x2b74e8e656a0 '' .operator ResString 0x2b74e8dc1f00 +// `-DeclRefExpr 0x2b74e8e65648 'struct ErrorResource_Impl' lvalue Var 0x2b74e8e653b0 'aEr' 'struct ErrorResource_Impl' +// +// Copies code from Clang's lib/AST/Expr.cpp: +namespace detail { + inline clang::Expr *skipImplicitTemporary(clang::Expr *expr) { + // Skip through reference binding to temporary. + if (clang::MaterializeTemporaryExpr *Materialize + = clang::dyn_cast(expr)) + expr = Materialize->GetTemporaryExpr(); + + // Skip any temporary bindings; they're implicit. + if (clang::CXXBindTemporaryExpr *Binder = clang::dyn_cast(expr)) + expr = Binder->getSubExpr(); + + return expr; + } +} +inline clang::Expr *getSubExprAsWritten(clang::CastExpr *This) { + clang::Expr *SubExpr = nullptr; + clang::CastExpr *E = This; + do { + SubExpr = detail::skipImplicitTemporary(E->getSubExpr()); + + // Conversions by constructor and conversion functions have a + // subexpression describing the call; strip it off. + if (E->getCastKind() == clang::CK_ConstructorConversion) + SubExpr = + detail::skipImplicitTemporary(clang::cast(SubExpr)->getArg(0)); + else if (E->getCastKind() == clang::CK_UserDefinedConversion) { + assert((clang::isa(SubExpr) || + clang::isa(SubExpr)) && + "Unexpected SubExpr for CK_UserDefinedConversion."); + if (clang::isa(SubExpr)) + SubExpr = clang::cast(SubExpr)->getImplicitObjectArgument(); + } + + // If the subexpression we're left with is an implicit cast, look + // through that, too. + } while ((E = clang::dyn_cast(SubExpr))); + + return SubExpr; +} +inline const clang::Expr *getSubExprAsWritten(const clang::CastExpr *This) { + return getSubExprAsWritten(const_cast(This)); +} + } #endif diff --git a/compilerplugins/clang/redundantcast.cxx b/compilerplugins/clang/redundantcast.cxx index b7fbfd3c9010..3c38dd3ef495 100644 --- a/compilerplugins/clang/redundantcast.cxx +++ b/compilerplugins/clang/redundantcast.cxx @@ -26,82 +26,11 @@ #include "clang/Sema/Sema.h" #include "check.hxx" +#include "compat.hxx" #include "plugin.hxx" namespace { -// Work around : -// -// SfxErrorHandler::GetClassString (svtools/source/misc/ehdl.cxx): -// -// ErrorResource_Impl aEr(aId, (sal_uInt16)lClassId); -// if(aEr) -// { -// rStr = static_cast(aEr).GetString(); -// } -// -// expr->dump(): -// CXXStaticCastExpr 0x2b74e8e657b8 'class ResString' static_cast -// `-CXXBindTemporaryExpr 0x2b74e8e65798 'class ResString' (CXXTemporary 0x2b74e8e65790) -// `-CXXConstructExpr 0x2b74e8e65758 'class ResString' 'void (class ResString &&) noexcept(false)' elidable -// `-MaterializeTemporaryExpr 0x2b74e8e65740 'class ResString' xvalue -// `-CXXBindTemporaryExpr 0x2b74e8e65720 'class ResString' (CXXTemporary 0x2b74e8e65718) -// `-ImplicitCastExpr 0x2b74e8e65700 'class ResString' -// `-CXXMemberCallExpr 0x2b74e8e656d8 'class ResString' -// `-MemberExpr 0x2b74e8e656a0 '' .operator ResString 0x2b74e8dc1f00 -// `-DeclRefExpr 0x2b74e8e65648 'struct ErrorResource_Impl' lvalue Var 0x2b74e8e653b0 'aEr' 'struct ErrorResource_Impl' -// expr->getSubExprAsWritten()->dump(): -// MaterializeTemporaryExpr 0x2b74e8e65740 'class ResString' xvalue -// `-CXXBindTemporaryExpr 0x2b74e8e65720 'class ResString' (CXXTemporary 0x2b74e8e65718) -// `-ImplicitCastExpr 0x2b74e8e65700 'class ResString' -// `-CXXMemberCallExpr 0x2b74e8e656d8 'class ResString' -// `-MemberExpr 0x2b74e8e656a0 '' .operator ResString 0x2b74e8dc1f00 -// `-DeclRefExpr 0x2b74e8e65648 'struct ErrorResource_Impl' lvalue Var 0x2b74e8e653b0 'aEr' 'struct ErrorResource_Impl' -// -// Copies code from Clang's lib/AST/Expr.cpp: -namespace { - Expr *skipImplicitTemporary(Expr *expr) { - // Skip through reference binding to temporary. - if (MaterializeTemporaryExpr *Materialize - = dyn_cast(expr)) - expr = Materialize->GetTemporaryExpr(); - - // Skip any temporary bindings; they're implicit. - if (CXXBindTemporaryExpr *Binder = dyn_cast(expr)) - expr = Binder->getSubExpr(); - - return expr; - } -} -Expr *getSubExprAsWritten(CastExpr *This) { - Expr *SubExpr = nullptr; - CastExpr *E = This; - do { - SubExpr = skipImplicitTemporary(E->getSubExpr()); - - // Conversions by constructor and conversion functions have a - // subexpression describing the call; strip it off. - if (E->getCastKind() == CK_ConstructorConversion) - SubExpr = - skipImplicitTemporary(cast(SubExpr)->getArg(0)); - else if (E->getCastKind() == CK_UserDefinedConversion) { - assert((isa(SubExpr) || - isa(SubExpr)) && - "Unexpected SubExpr for CK_UserDefinedConversion."); - if (isa(SubExpr)) - SubExpr = cast(SubExpr)->getImplicitObjectArgument(); - } - - // If the subexpression we're left with is an implicit cast, look - // through that, too. - } while ((E = dyn_cast(SubExpr))); - - return SubExpr; -} -const Expr *getSubExprAsWritten(const CastExpr *This) { - return getSubExprAsWritten(const_cast(This)); -} - bool isVoidPointer(QualType type) { return type->isPointerType() && type->getAs()->getPointeeType()->isVoidType(); @@ -322,7 +251,7 @@ bool RedundantCast::VisitCStyleCastExpr(CStyleCastExpr const * expr) { if (isInUnoIncludeFile(compiler.getSourceManager().getSpellingLoc(expr->getLocStart()))) { return true; } - auto t1 = getSubExprAsWritten(expr)->getType(); + auto t1 = compat::getSubExprAsWritten(expr)->getType(); auto t2 = expr->getTypeAsWritten(); if (t1 != t2) { return true; @@ -386,7 +315,7 @@ bool RedundantCast::VisitCXXStaticCastExpr(CXXStaticCastExpr const * expr) { if (ignoreLocation(expr)) { return true; } - auto t1 = getSubExprAsWritten(expr)->getType(); + auto t1 = compat::getSubExprAsWritten(expr)->getType(); auto t2 = expr->getTypeAsWritten(); if (t1.getCanonicalType() != t2.getCanonicalType()) { return true; diff --git a/compilerplugins/clang/stringcopy.cxx b/compilerplugins/clang/stringcopy.cxx index c572b965bb9a..b1d8e22216ec 100644 --- a/compilerplugins/clang/stringcopy.cxx +++ b/compilerplugins/clang/stringcopy.cxx @@ -8,6 +8,7 @@ */ #include "check.hxx" +#include "compat.hxx" #include "plugin.hxx" namespace { @@ -23,7 +24,7 @@ public: return true; } auto const t1 = expr->getTypeAsWritten(); - auto const t2 = expr->getSubExprAsWritten()->getType(); + auto const t2 = compat::getSubExprAsWritten(expr)->getType(); if ((t1.getCanonicalType().getTypePtr() != t2.getCanonicalType().getTypePtr()) || !(loplugin::TypeCheck(t1).Class("OUString").Namespace("rtl") @@ -46,7 +47,7 @@ private: } }; -static loplugin::Plugin::Registration reg("stringcopy", false); +static loplugin::Plugin::Registration reg("stringcopy"); } -- cgit