[Libreoffice-commits] core.git: compilerplugins/clang
Noel Grandin (via logerrit)
logerrit at kemper.freedesktop.org
Tue Apr 30 07:34:04 UTC 2019
compilerplugins/clang/stringconstant.cxx | 105 ++++++++++----------------
compilerplugins/clang/test/stringconstant.cxx | 16 ---
2 files changed, 45 insertions(+), 76 deletions(-)
New commits:
commit e02b9ccf58e9eda01f8588868ec2f367f7075dcf
Author: Noel Grandin <noel.grandin at collabora.co.uk>
AuthorDate: Tue Apr 30 09:26:47 2019 +0200
Commit: Noel Grandin <noel.grandin at collabora.co.uk>
CommitDate: Tue Apr 30 09:28:40 2019 +0200
revert part of "improve loplugin:stringconstant"
Revert part of
commit dd8d5e5795358d732a9f7a8af7c35f662321e332
Date: Mon Apr 29 11:18:21 2019 +0200
improve loplugin:stringconstant
sberg's original gerrit comment:
but there can also be other problematic overloads for parameters like
`void const *` or `std::string_view`. I'm not sure this change is worth
the potential false positives.
and continuing IRC discussion:
<noelgrandin> I'll revert the compilerplugins/ part
<sberg> noelgrandin, my main concern is that /if/ somebody eventually
runs into such an overload situation, it's really hard to get the
warnings/errors fixed for those people, short of going into the plugin
itself
Change-Id: I4916ce8943c4319d7ef9084e22d6a0eeb430b15c
diff --git a/compilerplugins/clang/stringconstant.cxx b/compilerplugins/clang/stringconstant.cxx
index 6a009e510297..05cfa03ff711 100644
--- a/compilerplugins/clang/stringconstant.cxx
+++ b/compilerplugins/clang/stringconstant.cxx
@@ -58,19 +58,7 @@ bool isLhsOfAssignment(FunctionDecl const * decl, unsigned parameter) {
|| (oo >= OO_PlusEqual && oo <= OO_GreaterGreaterEqual);
}
-bool typecheckIsOUStringParam(const clang::QualType t) {
- return bool(loplugin::TypeCheck(t).NotSubstTemplateTypeParmType()
- .LvalueReference().Const().NotSubstTemplateTypeParmType()
- .Class("OUString").Namespace("rtl").GlobalNamespace());
-}
-
-bool typecheckIsOStringParam(const clang::QualType t) {
- return bool(loplugin::TypeCheck(t).NotSubstTemplateTypeParmType()
- .LvalueReference().Const().NotSubstTemplateTypeParmType()
- .Class("OString").Namespace("rtl").GlobalNamespace());
-}
-
-bool hasOverloads(FunctionDecl const * decl, unsigned arguments, unsigned paramIndex) {
+bool hasOverloads(FunctionDecl const * decl, unsigned arguments) {
int n = 0;
auto ctx = decl->getDeclContext();
if (ctx->getDeclKind() == Decl::LinkageSpec) {
@@ -79,32 +67,17 @@ bool hasOverloads(FunctionDecl const * decl, unsigned arguments, unsigned paramI
auto res = ctx->lookup(decl->getDeclName());
for (auto d = res.begin(); d != res.end(); ++d) {
FunctionDecl const * f = dyn_cast<FunctionDecl>(*d);
- if (f == nullptr || f->getMinRequiredArguments() > arguments
- || f->getNumParams() < arguments) {
- continue;
- }
- auto consDecl = dyn_cast<CXXConstructorDecl>(f);
- if (consDecl && consDecl->isCopyOrMoveConstructor()) {
- continue;
- }
- // Deleted stuff like in ORowSetValueDecorator in connectivity can cause
- // trouble.
- if (consDecl && consDecl->isDeleted()) {
- return true;
- }
- if (paramIndex >= f->getNumParams()) {
- continue;
- }
- auto t = f->getParamDecl(paramIndex)->getType();
- // bool because 'const char *' converts to bool
- if (!typecheckIsOUStringParam(t) && !typecheckIsOStringParam(t)
- && !loplugin::TypeCheck(t).Pointer().Const().Char()
- && !loplugin::TypeCheck(t).AnyBoolean()) {
- continue;
- }
- ++n;
- if (n == 2) {
- return true;
+ if (f != nullptr && f->getMinRequiredArguments() <= arguments
+ && f->getNumParams() >= arguments)
+ {
+ auto consDecl = dyn_cast<CXXConstructorDecl>(f);
+ if (consDecl && consDecl->isCopyOrMoveConstructor()) {
+ continue;
+ }
+ ++n;
+ if (n == 2) {
+ return true;
+ }
}
}
return false;
@@ -296,6 +269,29 @@ bool StringConstant::VisitCallExpr(CallExpr const * expr) {
if (fdecl == nullptr) {
return true;
}
+ for (unsigned i = 0; i != fdecl->getNumParams(); ++i) {
+ auto t = fdecl->getParamDecl(i)->getType();
+ if (loplugin::TypeCheck(t).NotSubstTemplateTypeParmType()
+ .LvalueReference().Const().NotSubstTemplateTypeParmType()
+ .Class("OUString").Namespace("rtl").GlobalNamespace())
+ {
+ if (!(isLhsOfAssignment(fdecl, i)
+ || hasOverloads(fdecl, expr->getNumArgs())))
+ {
+ handleOUStringCtor(expr, i, fdecl, true);
+ }
+ }
+ if (loplugin::TypeCheck(t).NotSubstTemplateTypeParmType()
+ .LvalueReference().Const().NotSubstTemplateTypeParmType()
+ .Class("OString").Namespace("rtl").GlobalNamespace())
+ {
+ if (!(isLhsOfAssignment(fdecl, i)
+ || hasOverloads(fdecl, expr->getNumArgs())))
+ {
+ handleOStringCtor(expr, i, fdecl, true);
+ }
+ }
+ }
loplugin::DeclCheck dc(fdecl);
//TODO: u.compareToAscii("foo") -> u.???("foo")
//TODO: u.compareToIgnoreAsciiCaseAscii("foo") -> u.???("foo")
@@ -777,25 +773,6 @@ bool StringConstant::VisitCallExpr(CallExpr const * expr) {
}
return true;
}
- for (unsigned i = 0; i != fdecl->getNumParams(); ++i) {
- auto t = fdecl->getParamDecl(i)->getType();
- if (typecheckIsOUStringParam(t))
- {
- if (!(isLhsOfAssignment(fdecl, i)
- || hasOverloads(fdecl, expr->getNumArgs(), i)))
- {
- handleOUStringCtor(expr, i, fdecl, true);
- }
- }
- if (typecheckIsOStringParam(t))
- {
- if (!(isLhsOfAssignment(fdecl, i)
- || hasOverloads(fdecl, expr->getNumArgs(), i)))
- {
- handleOStringCtor(expr, i, fdecl, true);
- }
- }
- }
return true;
}
@@ -1199,23 +1176,27 @@ bool StringConstant::VisitCXXConstructExpr(CXXConstructExpr const * expr) {
auto consDecl = expr->getConstructor();
for (unsigned i = 0; i != consDecl->getNumParams(); ++i) {
auto t = consDecl->getParamDecl(i)->getType();
- if (typecheckIsOUStringParam(t))
+ if (loplugin::TypeCheck(t).NotSubstTemplateTypeParmType()
+ .LvalueReference().Const().NotSubstTemplateTypeParmType()
+ .Class("OUString").Namespace("rtl").GlobalNamespace())
{
auto argExpr = expr->getArg(i);
if (argExpr && i <= consDecl->getNumParams())
{
- if (!hasOverloads(consDecl, expr->getNumArgs(), i))
+ if (!hasOverloads(consDecl, expr->getNumArgs()))
{
handleOUStringCtor(expr, argExpr, consDecl, true);
}
}
}
- if (typecheckIsOStringParam(t))
+ if (loplugin::TypeCheck(t).NotSubstTemplateTypeParmType()
+ .LvalueReference().Const().NotSubstTemplateTypeParmType()
+ .Class("OString").Namespace("rtl").GlobalNamespace())
{
auto argExpr = expr->getArg(i);
if (argExpr && i <= consDecl->getNumParams())
{
- if (!hasOverloads(consDecl, expr->getNumArgs(), i))
+ if (!hasOverloads(consDecl, expr->getNumArgs()))
{
handleOStringCtor(expr, argExpr, consDecl, true);
}
diff --git a/compilerplugins/clang/test/stringconstant.cxx b/compilerplugins/clang/test/stringconstant.cxx
index 1eda24580ab2..49ae3b68d035 100644
--- a/compilerplugins/clang/test/stringconstant.cxx
+++ b/compilerplugins/clang/test/stringconstant.cxx
@@ -17,7 +17,6 @@
extern void foo(OUString const &);
struct Foo {
- Foo(int, const OUString &) {}
Foo(OUString const &, int) {}
Foo(OUString const &) {}
void foo(OUString const &) const {}
@@ -29,11 +28,6 @@ struct Foo2 {
void foo(OString const &) const {}
};
-struct NegativeFoo {
- NegativeFoo(const OString&, const OString& ) {}
- NegativeFoo(const OString&, const OUString& ) {}
-};
-
int main() {
char const s1[] = "foo";
char const * const s2 = "foo";
@@ -73,15 +67,9 @@ int main() {
(void)aFoo;
Foo aFoo2(OUString("xxx")); // expected-error {{in call of 'Foo::Foo', replace 'OUString' constructed from a string literal directly with the string literal}}
aFoo2.foo(OUString("xxx")); // expected-error {{in call of 'Foo::foo', replace 'OUString' constructed from a string literal directly with the string literal}}
- Foo aFoo3(1, OUString("xxx")); // expected-error {{in call of 'Foo::Foo', replace 'OUString' constructed from a string literal directly with the string literal}}
- (void)aFoo3;
-
- Foo2 aFoo4(OString("xxx")); // expected-error {{in call of 'Foo2::Foo2', replace 'OUString' constructed from a string literal directly with the string literal}}
- aFoo4.foo(OString("xxx")); // expected-error {{in call of 'Foo2::foo', replace 'OUString' constructed from a string literal directly with the string literal}}
- // no warning expected
- NegativeFoo aNegativeFoo(OString("xxx"), OUString("1"));
- (void)aNegativeFoo;
+ Foo2 aFoo3(OString("xxx")); // expected-error {{in call of 'Foo2::Foo2', replace 'OUString' constructed from a string literal directly with the string literal}}
+ aFoo3.foo(OString("xxx")); // expected-error {{in call of 'Foo2::foo', replace 'OUString' constructed from a string literal directly with the string literal}}
(void) OUString("xxx", 3, RTL_TEXTENCODING_ASCII_US); // expected-error {{simplify construction of 'OUString' with string constant argument [loplugin:stringconstant]}}
(void) OUString("xxx", 3, RTL_TEXTENCODING_ISO_8859_1); // expected-error {{suspicious 'rtl::OUString' constructor with text encoding 12 but plain ASCII content; use 'RTL_TEXTENCODING_ASCII_US' instead [loplugin:stringconstant]}}
More information about the Libreoffice-commits
mailing list