mir/libreoffice
2
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

revert part of "improve loplugin:stringconstant"

Browse Source 
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
This commit is contained in:
Noel Grandin 2019-04-30 09:26:47 +02:00
parent cd08835d1b
commit e02b9ccf58
2 changed files with 45 additions and 76 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

105
compilerplugins/clang/stringconstant.cxx
View File

@ -58,19 +58,7 @@ bool isLhsOfAssignment(FunctionDecl const * decl, unsigned parameter) {
|| (oo >= OO_PlusEqual && oo <= OO_GreaterGreaterEqual); || (oo >= OO_PlusEqual && oo <= OO_GreaterGreaterEqual);
} }
bool typecheckIsOUStringParam(const clang::QualType t) { bool hasOverloads(FunctionDecl const * decl, unsigned arguments) {
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) {
int n = 0; int n = 0;
auto ctx = decl->getDeclContext(); auto ctx = decl->getDeclContext();
if (ctx->getDeclKind() == Decl::LinkageSpec) { if (ctx->getDeclKind() == Decl::LinkageSpec) {
@ -79,32 +67,17 @@ bool hasOverloads(FunctionDecl const * decl, unsigned arguments, unsigned paramI
auto res = ctx->lookup(decl->getDeclName()); auto res = ctx->lookup(decl->getDeclName());
for (auto d = res.begin(); d != res.end(); ++d) { for (auto d = res.begin(); d != res.end(); ++d) {
FunctionDecl const * f = dyn_cast<FunctionDecl>(*d); FunctionDecl const * f = dyn_cast<FunctionDecl>(*d);
if (f == nullptr || f->getMinRequiredArguments() > arguments if (f != nullptr && f->getMinRequiredArguments() <= arguments
|| f->getNumParams() < arguments) { && f->getNumParams() >= arguments)
continue; {
} auto consDecl = dyn_cast<CXXConstructorDecl>(f);
auto consDecl = dyn_cast<CXXConstructorDecl>(f); if (consDecl && consDecl->isCopyOrMoveConstructor()) {
if (consDecl && consDecl->isCopyOrMoveConstructor()) { continue;
continue; }
} ++n;
// Deleted stuff like in ORowSetValueDecorator in connectivity can cause if (n == 2) {
// trouble. return true;
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;
} }
} }
return false; return false;
@ -296,6 +269,29 @@ bool StringConstant::VisitCallExpr(CallExpr const * expr) {
if (fdecl == nullptr) { if (fdecl == nullptr) {
return true; 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); loplugin::DeclCheck dc(fdecl);
//TODO: u.compareToAscii("foo") -> u.???("foo") //TODO: u.compareToAscii("foo") -> u.???("foo")
//TODO: u.compareToIgnoreAsciiCaseAscii("foo") -> u.???("foo") //TODO: u.compareToIgnoreAsciiCaseAscii("foo") -> u.???("foo")
@ -777,25 +773,6 @@ bool StringConstant::VisitCallExpr(CallExpr const * expr) {
} }
return true; 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; return true;
} }
@ -1199,23 +1176,27 @@ bool StringConstant::VisitCXXConstructExpr(CXXConstructExpr const * expr) {
auto consDecl = expr->getConstructor(); auto consDecl = expr->getConstructor();
for (unsigned i = 0; i != consDecl->getNumParams(); ++i) { for (unsigned i = 0; i != consDecl->getNumParams(); ++i) {
auto t = consDecl->getParamDecl(i)->getType(); 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); auto argExpr = expr->getArg(i);
if (argExpr && i <= consDecl->getNumParams()) if (argExpr && i <= consDecl->getNumParams())
{ {
if (!hasOverloads(consDecl, expr->getNumArgs(), i)) if (!hasOverloads(consDecl, expr->getNumArgs()))
{ {
handleOUStringCtor(expr, argExpr, consDecl, true); 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); auto argExpr = expr->getArg(i);
if (argExpr && i <= consDecl->getNumParams()) if (argExpr && i <= consDecl->getNumParams())
{ {
if (!hasOverloads(consDecl, expr->getNumArgs(), i)) if (!hasOverloads(consDecl, expr->getNumArgs()))
{ {
handleOStringCtor(expr, argExpr, consDecl, true); handleOStringCtor(expr, argExpr, consDecl, true);
} }

16
compilerplugins/clang/test/stringconstant.cxx
View File

@ -17,7 +17,6 @@
extern void foo(OUString const &); extern void foo(OUString const &);
struct Foo { struct Foo {
Foo(int, const OUString &) {}
Foo(OUString const &, int) {} Foo(OUString const &, int) {}
Foo(OUString const &) {} Foo(OUString const &) {}
void foo(OUString const &) const {} void foo(OUString const &) const {}
@ -29,11 +28,6 @@ struct Foo2 {
void foo(OString const &) const {} void foo(OString const &) const {}
}; };
struct NegativeFoo {
NegativeFoo(const OString&, const OString& ) {}
NegativeFoo(const OString&, const OUString& ) {}
};
int main() { int main() {
char const s1[] = "foo"; char const s1[] = "foo";
char const * const s2 = "foo"; char const * const s2 = "foo";
@ -73,15 +67,9 @@ int main() {
(void)aFoo; (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}} 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}} 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}} Foo2 aFoo3(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}} aFoo3.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;
(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_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]}} (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]}}