Improved loplugin:stringconstant (now that GCC 7 supports it)

Change-Id: I8f83c1941b8f39b261005939f4dcf3577ae9fc6f
Reviewed-on: https://gerrit.libreoffice.org/76702
Tested-by: Jenkins
Reviewed-by: Stephan Bergmann <sbergman@redhat.com>

3 files changed, 152 insertions, 0 deletions

diff --git a/compilerplugins/clang/compat.hxx b/compilerplugins/clang/compat.hxx
index 64ee9a8fd265..cb13f44cfa66 100644
--- a/compilerplugins/clang/compat.hxx
+++ b/compilerplugins/clang/compat.hxx
@@ -256,6 +256,18 @@ inline bool isExplicitSpecified(clang::CXXConversionDecl const * decl) {
 #endif
 }
 
+inline clang::QualType getDeclaredReturnType(clang::FunctionDecl const * decl) {
+#if CLANG_VERSION >= 80000
+    return decl->getDeclaredReturnType();
+#else
+    // <https://github.com/llvm/llvm-project/commit/4576a77b809649f5b8d0ff8c7a4be57eeee0ecf9>
+    // "PR33222: Require the declared return type not the actual return type to":
+    auto *TSI = decl->getTypeSourceInfo();
+    clang::QualType T = TSI ? TSI->getType() : decl->getType();
+    return T->castAs<clang::FunctionType>()->getReturnType();
+#endif
+}
+
 }
 
 #endif
diff --git a/compilerplugins/clang/stringconstant.cxx b/compilerplugins/clang/stringconstant.cxx
index 05cfa03ff711..8a56e8998b08 100644
--- a/compilerplugins/clang/stringconstant.cxx
+++ b/compilerplugins/clang/stringconstant.cxx
@@ -20,6 +20,7 @@
 #include <iostream>
 
 #include "check.hxx"
+#include "compat.hxx"
 #include "plugin.hxx"
 
 // Define a "string constant" to be a constant expression either of type "array
@@ -110,6 +111,70 @@ public:
 
     void run() override;
 
+    bool TraverseFunctionDecl(FunctionDecl * decl) {
+        returnTypes_.push(compat::getDeclaredReturnType(decl));
+        auto const ret = RecursiveASTVisitor::TraverseFunctionDecl(decl);
+        assert(!returnTypes_.empty());
+        assert(returnTypes_.top() == compat::getDeclaredReturnType(decl));
+        returnTypes_.pop();
+        return ret;
+    }
+
+    bool TraverseCXXDeductionGuideDecl(CXXDeductionGuideDecl * decl) {
+        returnTypes_.push(compat::getDeclaredReturnType(decl));
+        auto const ret = RecursiveASTVisitor::TraverseCXXDeductionGuideDecl(
+            decl);
+        assert(!returnTypes_.empty());
+        assert(returnTypes_.top() == compat::getDeclaredReturnType(decl));
+        returnTypes_.pop();
+        return ret;
+    }
+
+    bool TraverseCXXMethodDecl(CXXMethodDecl * decl) {
+        returnTypes_.push(compat::getDeclaredReturnType(decl));
+        auto const ret = RecursiveASTVisitor::TraverseCXXMethodDecl(decl);
+        assert(!returnTypes_.empty());
+        assert(returnTypes_.top() == compat::getDeclaredReturnType(decl));
+        returnTypes_.pop();
+        return ret;
+    }
+
+    bool TraverseCXXConstructorDecl(CXXConstructorDecl * decl) {
+        returnTypes_.push(compat::getDeclaredReturnType(decl));
+        auto const ret = RecursiveASTVisitor::TraverseCXXConstructorDecl(decl);
+        assert(!returnTypes_.empty());
+        assert(returnTypes_.top() == compat::getDeclaredReturnType(decl));
+        returnTypes_.pop();
+        return ret;
+    }
+
+    bool TraverseCXXDestructorDecl(CXXDestructorDecl * decl) {
+        returnTypes_.push(compat::getDeclaredReturnType(decl));
+        auto const ret = RecursiveASTVisitor::TraverseCXXDestructorDecl(decl);
+        assert(!returnTypes_.empty());
+        assert(returnTypes_.top() == compat::getDeclaredReturnType(decl));
+        returnTypes_.pop();
+        return ret;
+    }
+
+    bool TraverseCXXConversionDecl(CXXConversionDecl * decl) {
+        returnTypes_.push(compat::getDeclaredReturnType(decl));
+        auto const ret = RecursiveASTVisitor::TraverseCXXConversionDecl(decl);
+        assert(!returnTypes_.empty());
+        assert(returnTypes_.top() == compat::getDeclaredReturnType(decl));
+        returnTypes_.pop();
+        return ret;
+    }
+
+    bool TraverseObjCMethodDecl(ObjCMethodDecl * decl) {
+        returnTypes_.push(decl->getReturnType());
+        auto const ret = RecursiveASTVisitor::TraverseObjCMethodDecl(decl);
+        assert(!returnTypes_.empty());
+        assert(returnTypes_.top() == decl->getReturnType());
+        returnTypes_.pop();
+        return ret;
+    }
+
     bool TraverseCallExpr(CallExpr * expr);
 
     bool TraverseCXXMemberCallExpr(CXXMemberCallExpr * expr);
@@ -122,6 +187,8 @@ public:
 
     bool VisitCXXConstructExpr(CXXConstructExpr const * expr);
 
+    bool VisitReturnStmt(ReturnStmt const * stmt);
+
 private:
     enum class ContentKind { Ascii, Utf8, Arbitrary };
 
@@ -183,6 +250,7 @@ private:
     void handleFunArgOstring(
         CallExpr const * expr, unsigned arg, FunctionDecl const * callee);
 
+    std::stack<QualType> returnTypes_;
     std::stack<Expr const *> calls_;
 };
 
@@ -1207,6 +1275,54 @@ bool StringConstant::VisitCXXConstructExpr(CXXConstructExpr const * expr) {
     return true;
 }
 
+bool StringConstant::VisitReturnStmt(ReturnStmt const * stmt) {
+    if (ignoreLocation(stmt)) {
+        return true;
+    }
+    auto const e1 = stmt->getRetValue();
+    if (e1 == nullptr) {
+        return true;
+    }
+    auto const tc1 = loplugin::TypeCheck(e1->getType().getTypePtr());
+    if (!(tc1.Class("OString").Namespace("rtl").GlobalNamespace()
+          || tc1.Class("OUString").Namespace("rtl").GlobalNamespace()))
+    {
+        return true;
+    }
+    assert(!returnTypes_.empty());
+    auto const tc2 = loplugin::TypeCheck(returnTypes_.top().getTypePtr());
+    if (!(tc2.Class("OString").Namespace("rtl").GlobalNamespace()
+          || tc2.Class("OUString").Namespace("rtl").GlobalNamespace()))
+    {
+        return true;
+    }
+    auto const e2 = dyn_cast<CXXFunctionalCastExpr>(e1->IgnoreImplicit());
+    if (e2 == nullptr) {
+        return true;
+    }
+    auto const e3 = dyn_cast<CXXBindTemporaryExpr>(e2->getSubExpr());
+    if (e3 == nullptr) {
+        return true;
+    }
+    auto const e4 = dyn_cast<CXXConstructExpr>(e3->getSubExpr());
+    if (e4 == nullptr) {
+        return true;
+    }
+    if (e4->getNumArgs() != 2) {
+        return true;
+    }
+    auto const e5 = e4->getArg(1);
+    if (!(isa<CXXDefaultArgExpr>(e5)
+          && (loplugin::TypeCheck(e5->getType()).Struct("Dummy").Namespace("libreoffice_internal")
+              .Namespace("rtl").GlobalNamespace())))
+    {
+        return true;
+    }
+    report(DiagnosticsEngine::Warning, "elide constructor call", compat::getBeginLoc(e1))
+        << e1->getSourceRange();
+    return true;
+}
+
 std::string StringConstant::describeChangeKind(ChangeKind kind) {
     switch (kind) {
     case ChangeKind::Char:
diff --git a/compilerplugins/clang/test/stringconstant.cxx b/compilerplugins/clang/test/stringconstant.cxx
index 49ae3b68d035..066648c8871d 100644
--- a/compilerplugins/clang/test/stringconstant.cxx
+++ b/compilerplugins/clang/test/stringconstant.cxx
@@ -28,6 +28,30 @@ struct Foo2 {
     void foo(OString const &) const {}
 };
 
+OString ret1() {
+    return OString("foo"); // expected-error {{elide constructor call [loplugin:stringconstant]}}
+}
+
+OString const ret2() {
+    return OString("foo"); // expected-error {{elide constructor call [loplugin:stringconstant]}}
+}
+
+auto ret3() {
+    return OString("foo");
+}
+
+OUString ret4() {
+    return OUString("foo"); // expected-error {{elide constructor call [loplugin:stringconstant]}}
+}
+
+OUString const ret5() {
+    return OUString("foo"); // expected-error {{elide constructor call [loplugin:stringconstant]}}
+}
+
+auto ret6() {
+    return OUString("foo");
+}
+
 int main() {
     char const s1[] = "foo";
     char const * const s2 = "foo";