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>

6 files changed, 180 insertions, 8 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
diff --git a/solenv/CompilerTest_compilerplugins_clang.mk b/solenv/CompilerTest_compilerplugins_clang.mk
index 8308ee840008..b575074e184f 100644
--- a/solenv/CompilerTest_compilerplugins_clang.mk
+++ b/solenv/CompilerTest_compilerplugins_clang.mk
@@ -70,7 +70,8 @@ $(eval $(call gb_CompilerTest_add_exception_objects,compilerplugins_clang, \
     compilerplugins/clang/test/staticvar \
     compilerplugins/clang/test/stdfunction \
     compilerplugins/clang/test/stringbuffer \
-    compilerplugins/clang/test/stringconcat \
+    compilerplugins/clang/test/stringconcatauto \
+    compilerplugins/clang/test/stringconcatliterals \
     compilerplugins/clang/test/stringconstant \
     compilerplugins/clang/test/stringloop \
     compilerplugins/clang/test/typedefparam \
diff --git a/solenv/clang-format/blacklist b/solenv/clang-format/blacklist
index 14dcd2ae8fae..9d0ac1972cd5 100644
--- a/solenv/clang-format/blacklist
+++ b/solenv/clang-format/blacklist
@@ -1778,7 +1778,8 @@ compilerplugins/clang/store/tutorial/tutorial3.cxx
 compilerplugins/clang/store/tutorial/tutorial3.hxx
 compilerplugins/clang/store/unusedcode.cxx
 compilerplugins/clang/store/valueof.cxx
-compilerplugins/clang/stringconcat.cxx
+compilerplugins/clang/stringconcatauto.cxx
+compilerplugins/clang/stringconcatliterals.cxx
 compilerplugins/clang/stringconstant.cxx
 compilerplugins/clang/stringstatic.cxx
 compilerplugins/clang/subtlezeroinit.cxx
@@ -1815,6 +1816,7 @@ compilerplugins/clang/test/refcounting.cxx
 compilerplugins/clang/test/salbool.cxx
 compilerplugins/clang/test/salunicodeliteral.cxx
 compilerplugins/clang/test/salunicodeliteral.hxx
+compilerplugins/clang/test/stringconcatauto.cxx
 compilerplugins/clang/test/stringconstant.cxx
 compilerplugins/clang/test/unnecessarycatchthrow.cxx
 compilerplugins/clang/test/unnecessaryoverride-dtor.cxx