compiler check for rtl::OUStringConcat instances
Something like auto str = "string" + OUString::number( 10 ); will not be OUString but actually rtl::OUStringConcat (which gets implicitly converted to OUString, but not with auto). Since those refer to temporaries from the expression, they should not outlive the expression. Change-Id: Ib4cde4b38befb3d49927d0cf01c52ebb2d36df89 Reviewed-on: https://gerrit.libreoffice.org/78830 Tested-by: Jenkins Reviewed-by: Luboš Luňák <l.lunak@collabora.com>
This commit is contained in:
112
compilerplugins/clang/stringconcatauto.cxx
Normal file
112
compilerplugins/clang/stringconcatauto.cxx
Normal file
@@ -0,0 +1,112 @@
|
|||||||
|
/* -*- Mode: C++; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4 -*- */
|
||||||
|
/*
|
||||||
|
* This file is part of the LibreOffice project.
|
||||||
|
*
|
||||||
|
* Based on LLVM/Clang.
|
||||||
|
*
|
||||||
|
* This file is distributed under the University of Illinois Open Source
|
||||||
|
* License. See LICENSE.TXT for details.
|
||||||
|
*
|
||||||
|
*/
|
||||||
|
|
||||||
|
/*
|
||||||
|
This is a compile check.
|
||||||
|
|
||||||
|
Warns about 'auto' declarations becoming rtl::OUStringConcat, such as
|
||||||
|
auto str = "string" + OUString::number( 10 );
|
||||||
|
The type of the expression is rtl::OUStringConcat and those refer to temporaries
|
||||||
|
and so their lifecycle should not extend the lifecycle of those temporaries.
|
||||||
|
*/
|
||||||
|
|
||||||
|
#ifndef LO_CLANG_SHARED_PLUGINS
|
||||||
|
|
||||||
|
#include "plugin.hxx"
|
||||||
|
#include "check.hxx"
|
||||||
|
|
||||||
|
#include <unistd.h>
|
||||||
|
|
||||||
|
namespace loplugin
|
||||||
|
{
|
||||||
|
|
||||||
|
class StringConcatAuto
|
||||||
|
: public FilteringPlugin< StringConcatAuto >
|
||||||
|
{
|
||||||
|
public:
|
||||||
|
StringConcatAuto( const InstantiationData& data );
|
||||||
|
virtual void run() override;
|
||||||
|
bool shouldVisitTemplateInstantiations () const { return true; }
|
||||||
|
bool VisitVarDecl( const VarDecl* decl );
|
||||||
|
bool VisitFunctionDecl( const FunctionDecl* decl );
|
||||||
|
private:
|
||||||
|
enum class Check { Var, Return };
|
||||||
|
bool checkDecl( const DeclaratorDecl* decl, const QualType type, const SourceRange& range, Check check );
|
||||||
|
};
|
||||||
|
|
||||||
|
StringConcatAuto::StringConcatAuto( const InstantiationData& data )
|
||||||
|
: FilteringPlugin( data )
|
||||||
|
{
|
||||||
|
}
|
||||||
|
|
||||||
|
void StringConcatAuto::run()
|
||||||
|
{
|
||||||
|
TraverseDecl( compiler.getASTContext().getTranslationUnitDecl());
|
||||||
|
}
|
||||||
|
|
||||||
|
bool StringConcatAuto::VisitVarDecl( const VarDecl* decl )
|
||||||
|
{
|
||||||
|
return checkDecl( decl, decl->getType(),
|
||||||
|
decl->getTypeSourceInfo()
|
||||||
|
? decl->getTypeSourceInfo()->getTypeLoc().getSourceRange()
|
||||||
|
: decl->getSourceRange(),
|
||||||
|
Check::Var );
|
||||||
|
}
|
||||||
|
|
||||||
|
bool StringConcatAuto::VisitFunctionDecl( const FunctionDecl* decl )
|
||||||
|
{
|
||||||
|
return checkDecl( decl, decl->getReturnType(), decl->getReturnTypeSourceRange(), Check::Return );
|
||||||
|
}
|
||||||
|
|
||||||
|
bool StringConcatAuto::checkDecl( const DeclaratorDecl* decl, QualType type, const SourceRange& range, Check check )
|
||||||
|
{
|
||||||
|
if( ignoreLocation( decl ))
|
||||||
|
return true;
|
||||||
|
if( isa< ParmVarDecl >( decl )) // parameters should be fine, temporaries should exist during the call
|
||||||
|
return true;
|
||||||
|
std::string fileName = getFileNameOfSpellingLoc(
|
||||||
|
compiler.getSourceManager().getSpellingLoc(compat::getBeginLoc(decl)));
|
||||||
|
loplugin::normalizeDotDotInFilePath(fileName);
|
||||||
|
if (fileName == SRCDIR "/include/rtl/string.hxx"
|
||||||
|
|| fileName == SRCDIR "/include/rtl/ustring.hxx"
|
||||||
|
|| fileName == SRCDIR "/include/rtl/strbuf.hxx"
|
||||||
|
|| fileName == SRCDIR "/include/rtl/ustrbuf.hxx"
|
||||||
|
|| fileName == SRCDIR "/include/rtl/stringconcat.hxx")
|
||||||
|
return true;
|
||||||
|
auto const tc = loplugin::TypeCheck( type.getNonReferenceType().getCanonicalType());
|
||||||
|
const char* typeString = nullptr;
|
||||||
|
if( tc.Struct("OUStringConcat").Namespace("rtl").GlobalNamespace())
|
||||||
|
typeString = "OUString";
|
||||||
|
else if( tc.Struct("OStringConcat").Namespace("rtl").GlobalNamespace())
|
||||||
|
typeString = "OString";
|
||||||
|
else
|
||||||
|
return true;
|
||||||
|
report( DiagnosticsEngine::Warning,
|
||||||
|
check == Check::Var
|
||||||
|
? "creating a variable of type %0 will make it reference temporaries"
|
||||||
|
: "returning a variable of type %0 will make it reference temporaries",
|
||||||
|
decl->getLocation())
|
||||||
|
<< type;
|
||||||
|
report( DiagnosticsEngine::Note,
|
||||||
|
"use %0 instead",
|
||||||
|
range.getBegin())
|
||||||
|
<< typeString
|
||||||
|
<< FixItHint::CreateReplacement( range, typeString );
|
||||||
|
return true;
|
||||||
|
}
|
||||||
|
|
||||||
|
static Plugin::Registration< StringConcatAuto > stringconcatauto( "stringconcatauto" );
|
||||||
|
|
||||||
|
} // namespace
|
||||||
|
|
||||||
|
#endif // LO_CLANG_SHARED_PLUGINS
|
||||||
|
|
||||||
|
/* vim:set shiftwidth=4 softtabstop=4 expandtab: */
|
@@ -47,11 +47,11 @@ Expr const * stripCtor(Expr const * expr) {
|
|||||||
return e2->getArg(0)->IgnoreParenImpCasts();
|
return e2->getArg(0)->IgnoreParenImpCasts();
|
||||||
}
|
}
|
||||||
|
|
||||||
class StringConcat:
|
class StringConcatLiterals:
|
||||||
public loplugin::FilteringPlugin<StringConcat>
|
public loplugin::FilteringPlugin<StringConcatLiterals>
|
||||||
{
|
{
|
||||||
public:
|
public:
|
||||||
explicit StringConcat(loplugin::InstantiationData const & data):
|
explicit StringConcatLiterals(loplugin::InstantiationData const & data):
|
||||||
FilteringPlugin(data) {}
|
FilteringPlugin(data) {}
|
||||||
|
|
||||||
void run() override
|
void run() override
|
||||||
@@ -63,7 +63,7 @@ private:
|
|||||||
bool isStringLiteral(Expr const * expr);
|
bool isStringLiteral(Expr const * expr);
|
||||||
};
|
};
|
||||||
|
|
||||||
bool StringConcat::VisitCallExpr(CallExpr const * expr) {
|
bool StringConcatLiterals::VisitCallExpr(CallExpr const * expr) {
|
||||||
if (ignoreLocation(expr)) {
|
if (ignoreLocation(expr)) {
|
||||||
return true;
|
return true;
|
||||||
}
|
}
|
||||||
@@ -135,7 +135,7 @@ bool StringConcat::VisitCallExpr(CallExpr const * expr) {
|
|||||||
return true;
|
return true;
|
||||||
}
|
}
|
||||||
|
|
||||||
bool StringConcat::isStringLiteral(Expr const * expr) {
|
bool StringConcatLiterals::isStringLiteral(Expr const * expr) {
|
||||||
expr = stripCtor(expr);
|
expr = stripCtor(expr);
|
||||||
if (!isa<clang::StringLiteral>(expr)) {
|
if (!isa<clang::StringLiteral>(expr)) {
|
||||||
return false;
|
return false;
|
||||||
@@ -153,7 +153,7 @@ bool StringConcat::isStringLiteral(Expr const * expr) {
|
|||||||
!= "OSL_THIS_FUNC");
|
!= "OSL_THIS_FUNC");
|
||||||
}
|
}
|
||||||
|
|
||||||
loplugin::Plugin::Registration<StringConcat> stringconcat("stringconcat");
|
loplugin::Plugin::Registration<StringConcatLiterals> stringconcatliterals("stringconcatliterals");
|
||||||
|
|
||||||
} // namespace
|
} // namespace
|
||||||
|
|
57
compilerplugins/clang/test/stringconcatauto.cxx
Normal file
57
compilerplugins/clang/test/stringconcatauto.cxx
Normal file
@@ -0,0 +1,57 @@
|
|||||||
|
/* -*- Mode: C++; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4 -*- */
|
||||||
|
/*
|
||||||
|
* This file is part of the LibreOffice project.
|
||||||
|
*
|
||||||
|
* Based on LLVM/Clang.
|
||||||
|
*
|
||||||
|
* This file is distributed under the University of Illinois Open Source
|
||||||
|
* License. See LICENSE.TXT for details.
|
||||||
|
*
|
||||||
|
*/
|
||||||
|
|
||||||
|
#include <rtl/ustring.hxx>
|
||||||
|
|
||||||
|
void foo()
|
||||||
|
{
|
||||||
|
auto str1 = "str1" + OUString::number( 10 );
|
||||||
|
// expected-error-re@-1 {{creating a variable of type 'rtl::OUStringConcat<{{.*}}>' will make it reference temporaries}}
|
||||||
|
// expected-note@-2 {{use OUString instead}}
|
||||||
|
OUString str2 = "str2" + OUString::number( 20 ) + "ing";
|
||||||
|
const auto& str3 = "str3" + OUString::number( 30 );
|
||||||
|
// expected-error-re@-1 {{creating a variable of type 'const rtl::OUStringConcat<{{.*}}> &' will make it reference temporaries}}
|
||||||
|
// expected-note@-2 {{use OUString instead}}
|
||||||
|
const auto str4 = "str4" + OString::number( 40 );
|
||||||
|
// expected-error-re@-1 {{creating a variable of type 'const rtl::OStringConcat<{{.*}}>' will make it reference temporaries}}
|
||||||
|
// expected-note@-2 {{use OString instead}}
|
||||||
|
(void) str1;
|
||||||
|
(void) str2;
|
||||||
|
(void) str3;
|
||||||
|
(void) str4;
|
||||||
|
}
|
||||||
|
|
||||||
|
struct A
|
||||||
|
{
|
||||||
|
auto bar()
|
||||||
|
// expected-error-re@-1 {{returning a variable of type 'rtl::OStringConcat<{{.*}}>' will make it reference temporaries}}
|
||||||
|
// expected-note@-2 {{use OString instead}}
|
||||||
|
{
|
||||||
|
return "bar" + OString::number( 110 );
|
||||||
|
}
|
||||||
|
};
|
||||||
|
|
||||||
|
template< typename T >
|
||||||
|
void fun( const T& par )
|
||||||
|
// parameters are without warnings
|
||||||
|
{
|
||||||
|
const T& var = par;
|
||||||
|
// expected-error-re@-1 {{creating a variable of type 'const rtl::OUStringConcat<{{.*}}> &' will make it reference temporaries}}
|
||||||
|
// expected-note@-2 {{use OUString instead}}
|
||||||
|
(void) var;
|
||||||
|
}
|
||||||
|
|
||||||
|
void testfun()
|
||||||
|
{
|
||||||
|
fun( "fun" + OUString::number( 200 ));
|
||||||
|
}
|
||||||
|
|
||||||
|
/* vim:set shiftwidth=4 softtabstop=4 expandtab: */
|
@@ -70,7 +70,8 @@ $(eval $(call gb_CompilerTest_add_exception_objects,compilerplugins_clang, \
|
|||||||
compilerplugins/clang/test/staticvar \
|
compilerplugins/clang/test/staticvar \
|
||||||
compilerplugins/clang/test/stdfunction \
|
compilerplugins/clang/test/stdfunction \
|
||||||
compilerplugins/clang/test/stringbuffer \
|
compilerplugins/clang/test/stringbuffer \
|
||||||
compilerplugins/clang/test/stringconcat \
|
compilerplugins/clang/test/stringconcatauto \
|
||||||
|
compilerplugins/clang/test/stringconcatliterals \
|
||||||
compilerplugins/clang/test/stringconstant \
|
compilerplugins/clang/test/stringconstant \
|
||||||
compilerplugins/clang/test/stringloop \
|
compilerplugins/clang/test/stringloop \
|
||||||
compilerplugins/clang/test/typedefparam \
|
compilerplugins/clang/test/typedefparam \
|
||||||
|
@@ -1778,7 +1778,8 @@ compilerplugins/clang/store/tutorial/tutorial3.cxx
|
|||||||
compilerplugins/clang/store/tutorial/tutorial3.hxx
|
compilerplugins/clang/store/tutorial/tutorial3.hxx
|
||||||
compilerplugins/clang/store/unusedcode.cxx
|
compilerplugins/clang/store/unusedcode.cxx
|
||||||
compilerplugins/clang/store/valueof.cxx
|
compilerplugins/clang/store/valueof.cxx
|
||||||
compilerplugins/clang/stringconcat.cxx
|
compilerplugins/clang/stringconcatauto.cxx
|
||||||
|
compilerplugins/clang/stringconcatliterals.cxx
|
||||||
compilerplugins/clang/stringconstant.cxx
|
compilerplugins/clang/stringconstant.cxx
|
||||||
compilerplugins/clang/stringstatic.cxx
|
compilerplugins/clang/stringstatic.cxx
|
||||||
compilerplugins/clang/subtlezeroinit.cxx
|
compilerplugins/clang/subtlezeroinit.cxx
|
||||||
@@ -1815,6 +1816,7 @@ compilerplugins/clang/test/refcounting.cxx
|
|||||||
compilerplugins/clang/test/salbool.cxx
|
compilerplugins/clang/test/salbool.cxx
|
||||||
compilerplugins/clang/test/salunicodeliteral.cxx
|
compilerplugins/clang/test/salunicodeliteral.cxx
|
||||||
compilerplugins/clang/test/salunicodeliteral.hxx
|
compilerplugins/clang/test/salunicodeliteral.hxx
|
||||||
|
compilerplugins/clang/test/stringconcatauto.cxx
|
||||||
compilerplugins/clang/test/stringconstant.cxx
|
compilerplugins/clang/test/stringconstant.cxx
|
||||||
compilerplugins/clang/test/unnecessarycatchthrow.cxx
|
compilerplugins/clang/test/unnecessarycatchthrow.cxx
|
||||||
compilerplugins/clang/test/unnecessaryoverride-dtor.cxx
|
compilerplugins/clang/test/unnecessaryoverride-dtor.cxx
|
||||||
|
Reference in New Issue
Block a user