compilerplugins/clang/stringadd.cxx | 44 +++++++++----------------------
compilerplugins/clang/test/stringadd.cxx | 6 ++--
2 files changed, 17 insertions(+), 33 deletions(-)
New commits:
commit 2d64e046e8d11451834816fade63a5ec0890f99b
Author: Stephan Bergmann <sberg...@redhat.com>
AuthorDate: Tue Oct 15 12:11:34 2019 +0200
Commit: Stephan Bergmann <sberg...@redhat.com>
CommitDate: Tue Oct 15 16:13:54 2019 +0200
Improve loplugin:stringadd
...after 9b5dad13b56bdde7c40970351af3da3a2c3c9350 "loplugin:stringadd look
for
unnecessary temporaries". There was no reason to check for implicit
MaterializeTemporaryExpr instead of explicitly written
CXXFunctionalCastExpr,
and checking for the latter makes it easier to report the casted-from type,
which gives useful information how to change code that exhibits the warning.
See the comments at <https://gerrit.libreoffice.org/#/c/80724/>
"loplugin:stringadd look for unnecessary temporaries" for details.
(And while at it, remove some commented-out debug code that is probably no
longer relevant now.)
Change-Id: I7d4cab85432885d617dd7114c75163c1eb376fc2
Reviewed-on: https://gerrit.libreoffice.org/80823
Tested-by: Jenkins
Reviewed-by: Stephan Bergmann <sberg...@redhat.com>
diff --git a/compilerplugins/clang/stringadd.cxx
b/compilerplugins/clang/stringadd.cxx
index 69251411ae70..764ef25adecc 100644
--- a/compilerplugins/clang/stringadd.cxx
+++ b/compilerplugins/clang/stringadd.cxx
@@ -193,46 +193,30 @@ bool
StringAdd::VisitCXXOperatorCallExpr(CXXOperatorCallExpr const* operatorCall
&& !tc.Class("OString").Namespace("rtl").GlobalNamespace())
return true;
- auto check = [/* operatorCall, */ this, &tc](const
MaterializeTemporaryExpr* matTempExpr) {
- auto tc3 = loplugin::TypeCheck(matTempExpr->getType());
+ auto check = [this, &tc](const Expr* expr) {
+ auto const e =
dyn_cast<CXXFunctionalCastExpr>(expr->IgnoreParenImpCasts());
+ if (e == nullptr)
+ return;
+ auto tc3 = loplugin::TypeCheck(e->getType());
if (!tc3.Class("OUString").Namespace("rtl").GlobalNamespace()
&& !tc3.Class("OString").Namespace("rtl").GlobalNamespace())
return;
- if (auto bindTemp
- =
dyn_cast<CXXBindTemporaryExpr>(matTempExpr->GetTemporaryExpr()->IgnoreCasts()))
- {
- // ignore temporaries returned from function calls
- if (isa<CallExpr>(bindTemp->getSubExpr()))
- return;
- // we don't have OStringLiteral1, so char needs to generate a
temporary
- if (tc.Class("OString").Namespace("rtl").GlobalNamespace()
- ||
tc.Struct("OStringConcat").Namespace("rtl").GlobalNamespace())
+ // we don't have OStringLiteral1, so char needs to generate a temporary
+ if (tc.Class("OString").Namespace("rtl").GlobalNamespace()
+ || tc.Struct("OStringConcat").Namespace("rtl").GlobalNamespace())
+ if (auto bindTemp =
dyn_cast<CXXBindTemporaryExpr>(e->getSubExpr()))
if (auto cxxConstruct =
dyn_cast<CXXConstructExpr>(bindTemp->getSubExpr()))
if (loplugin::TypeCheck(
cxxConstruct->getConstructor()->getParamDecl(0)->getType())
.Char())
return;
- // calls where we pass in an explicit character encoding
- if (auto cxxTemp =
dyn_cast<CXXTemporaryObjectExpr>(bindTemp->getSubExpr()))
- if (cxxTemp->getNumArgs() > 1)
- return;
- }
- // conditional operators ( a ? b : c ) will result in temporaries
- if (isa<ConditionalOperator>(
-
matTempExpr->GetTemporaryExpr()->IgnoreCasts()->IgnoreParens()))
- return;
- report(DiagnosticsEngine::Warning, "avoid constructing temporary
copies during +",
- compat::getBeginLoc(matTempExpr))
- << matTempExpr->getSourceRange();
- // operatorCall->dump();
- // matTempExpr->getType()->dump();
- //
operatorCall->getType()->getUnqualifiedDesugaredType()->dump();
+ report(DiagnosticsEngine::Warning, "avoid constructing temporary
object from %0 during +",
+ compat::getBeginLoc(e))
+ << e->getSubExprAsWritten()->getType() << e->getSourceRange();
};
- if (auto matTempExpr =
dyn_cast<MaterializeTemporaryExpr>(operatorCall->getArg(0)))
- check(matTempExpr);
- if (auto matTempExpr =
dyn_cast<MaterializeTemporaryExpr>(operatorCall->getArg(1)))
- check(matTempExpr);
+ check(operatorCall->getArg(0));
+ check(operatorCall->getArg(1));
return true;
}
diff --git a/compilerplugins/clang/test/stringadd.cxx
b/compilerplugins/clang/test/stringadd.cxx
index 0e0986258254..fc06e5e33b5c 100644
--- a/compilerplugins/clang/test/stringadd.cxx
+++ b/compilerplugins/clang/test/stringadd.cxx
@@ -163,9 +163,9 @@ void f1(OUString s, OUString t, int i, const char* pChar)
{
// no warning expected
t = t + "xxx";
- // expected-error@+1 {{avoid constructing temporary copies during +
[loplugin:stringadd]}}
+ // expected-error@+1 {{avoid constructing temporary object from 'const
char [4]' during + [loplugin:stringadd]}}
s = s + OUString("xxx");
- // expected-error@+1 {{avoid constructing temporary copies during +
[loplugin:stringadd]}}
+ // expected-error@+1 {{avoid constructing temporary object from 'const
rtl::OUString' during + [loplugin:stringadd]}}
s = s + OUString(getByRef());
// no warning expected
@@ -183,7 +183,7 @@ void f1(OUString s, OUString t, int i, const char* pChar)
void f2(char ch)
{
OString s;
- // expected-error@+1 {{avoid constructing temporary copies during +
[loplugin:stringadd]}}
+ // expected-error@+1 {{avoid constructing temporary object from 'const
char [4]' during + [loplugin:stringadd]}}
s = s + OString("xxx");
// no warning expected, no OStringLiteral1
s = s + OString(ch);
_______________________________________________
Libreoffice-commits mailing list
libreoffice-comm...@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/libreoffice-commits
