diff options
author | Luboš Luňák <l.lunak@collabora.com> | 2019-09-12 15:38:05 +0200 |
---|---|---|
committer | Luboš Luňák <l.lunak@collabora.com> | 2019-09-24 12:53:39 +0200 |
commit | a7d40f575463467698df76f041e558cb3bea7c85 (patch) | |
tree | 49f60f551e72c1a0a802ec40a39bd0543d654f33 /compilerplugins/clang | |
parent | 417d993b8b8a86c019758ee0850e4b42967e2afa (diff) |
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>
Diffstat (limited to 'compilerplugins/clang')
-rw-r--r-- | compilerplugins/clang/stringconcatauto.cxx | 112 | ||||
-rw-r--r-- | compilerplugins/clang/stringconcatliterals.cxx (renamed from compilerplugins/clang/stringconcat.cxx) | 12 | ||||
-rw-r--r-- | compilerplugins/clang/test/stringconcatauto.cxx | 57 | ||||
-rw-r--r-- | compilerplugins/clang/test/stringconcatliterals.cxx (renamed from compilerplugins/clang/test/stringconcat.cxx) | 0 |
4 files changed, 175 insertions, 6 deletions
diff --git a/compilerplugins/clang/stringconcatauto.cxx b/compilerplugins/clang/stringconcatauto.cxx new file mode 100644 index 000000000000..1437b7537323 --- /dev/null +++ b/compilerplugins/clang/stringconcatauto.cxx @@ -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: */ diff --git a/compilerplugins/clang/stringconcat.cxx b/compilerplugins/clang/stringconcatliterals.cxx index 8511f849d64f..f6008175f38d 100644 --- a/compilerplugins/clang/stringconcat.cxx +++ b/compilerplugins/clang/stringconcatliterals.cxx @@ -47,11 +47,11 @@ Expr const * stripCtor(Expr const * expr) { return e2->getArg(0)->IgnoreParenImpCasts(); } -class StringConcat: - public loplugin::FilteringPlugin<StringConcat> +class StringConcatLiterals: + public loplugin::FilteringPlugin<StringConcatLiterals> { public: - explicit StringConcat(loplugin::InstantiationData const & data): + explicit StringConcatLiterals(loplugin::InstantiationData const & data): FilteringPlugin(data) {} void run() override @@ -63,7 +63,7 @@ private: bool isStringLiteral(Expr const * expr); }; -bool StringConcat::VisitCallExpr(CallExpr const * expr) { +bool StringConcatLiterals::VisitCallExpr(CallExpr const * expr) { if (ignoreLocation(expr)) { return true; } @@ -135,7 +135,7 @@ bool StringConcat::VisitCallExpr(CallExpr const * expr) { return true; } -bool StringConcat::isStringLiteral(Expr const * expr) { +bool StringConcatLiterals::isStringLiteral(Expr const * expr) { expr = stripCtor(expr); if (!isa<clang::StringLiteral>(expr)) { return false; @@ -153,7 +153,7 @@ bool StringConcat::isStringLiteral(Expr const * expr) { != "OSL_THIS_FUNC"); } -loplugin::Plugin::Registration<StringConcat> stringconcat("stringconcat"); +loplugin::Plugin::Registration<StringConcatLiterals> stringconcatliterals("stringconcatliterals"); } // namespace diff --git a/compilerplugins/clang/test/stringconcatauto.cxx b/compilerplugins/clang/test/stringconcatauto.cxx new file mode 100644 index 000000000000..777da46e84a4 --- /dev/null +++ b/compilerplugins/clang/test/stringconcatauto.cxx @@ -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: */ diff --git a/compilerplugins/clang/test/stringconcat.cxx b/compilerplugins/clang/test/stringconcatliterals.cxx index e2d5a8ce20e3..e2d5a8ce20e3 100644 --- a/compilerplugins/clang/test/stringconcat.cxx +++ b/compilerplugins/clang/test/stringconcatliterals.cxx |