[Libreoffice-commits] core.git: compilerplugins/clang l10ntools/inc l10ntools/source

Noel Grandin noel.grandin at collabora.co.uk
Thu Jan 11 06:37:05 UTC 2018


 compilerplugins/clang/test/useuniqueptr.cxx |   15 -
 compilerplugins/clang/useuniqueptr.cxx      |  412 +++++++++++-----------------
 l10ntools/inc/cfgmerge.hxx                  |    2 
 l10ntools/inc/lngmerge.hxx                  |    4 
 l10ntools/inc/xrmmerge.hxx                  |    2 
 l10ntools/source/cfgmerge.cxx               |    8 
 l10ntools/source/lngmerge.cxx               |   50 +--
 l10ntools/source/xrmmerge.cxx               |    5 
 8 files changed, 206 insertions(+), 292 deletions(-)

New commits:
commit 96d9bd226215194632b6b0b7b0153f41ade1fc47
Author: Noel Grandin <noel.grandin at collabora.co.uk>
Date:   Wed Jan 10 11:29:36 2018 +0200

    loplugin:useuniqueptr in l10ntools
    
    update plugin to find all places where we are unconditionally deleting
    stuff in a destructor
    
    Change-Id: Ia0fedc2420c7717ed2bdd8d3bb00262d2a63e0bc
    Reviewed-on: https://gerrit.libreoffice.org/47724
    Tested-by: Jenkins <ci at libreoffice.org>
    Reviewed-by: Noel Grandin <noel.grandin at collabora.co.uk>

diff --git a/compilerplugins/clang/test/useuniqueptr.cxx b/compilerplugins/clang/test/useuniqueptr.cxx
index b45781925630..2ffdecc33a04 100644
--- a/compilerplugins/clang/test/useuniqueptr.cxx
+++ b/compilerplugins/clang/test/useuniqueptr.cxx
@@ -18,7 +18,7 @@ class Foo1 {
     XXX* m_pbar; // expected-note {{member is here [loplugin:useuniqueptr]}}
     ~Foo1()
     {
-        delete m_pbar; // expected-error {{a destructor with only a single unconditional call to delete on a member, is a sure sign it should be using std::unique_ptr for that field [loplugin:useuniqueptr]}}
+        delete m_pbar; // expected-error {{unconditional call to delete on a member, should be using std::unique_ptr [loplugin:useuniqueptr]}}
         m_pbar = nullptr;
     }
 };
@@ -29,8 +29,8 @@ class Foo2 {
     char* m_pbar2; // expected-note {{member is here [loplugin:useuniqueptr]}}
     ~Foo2()
     {
-        delete[] m_pbar1; // expected-error {{managing POD type 'char' manually, rather use std::vector / std::array / std::unique_ptr [loplugin:useuniqueptr]}}
-        delete[] m_pbar2; // expected-error {{managing POD type 'char' manually, rather use std::vector / std::array / std::unique_ptr [loplugin:useuniqueptr]}}
+        delete[] m_pbar1; // expected-error {{unconditional call to delete on a member, should be using std::unique_ptr [loplugin:useuniqueptr]}}
+        delete[] m_pbar2; // expected-error {{unconditional call to delete on a member, should be using std::unique_ptr [loplugin:useuniqueptr]}}
     }
 };
 
@@ -85,4 +85,13 @@ class Class8 {
             delete i.second;
     }
 };
+class Foo8 {
+    XXX* m_pbar1; // expected-note {{member is here [loplugin:useuniqueptr]}}
+    XXX* m_pbar2; // expected-note {{member is here [loplugin:useuniqueptr]}}
+    ~Foo8()
+    {
+        delete m_pbar1; // expected-error {{unconditional call to delete on a member, should be using std::unique_ptr [loplugin:useuniqueptr]}}
+        delete m_pbar2; // expected-error {{unconditional call to delete on a member, should be using std::unique_ptr [loplugin:useuniqueptr]}}
+    }
+};
 /* vim:set shiftwidth=4 softtabstop=4 expandtab cinoptions=b1,g0,N-s cinkeys+=0=break: */
diff --git a/compilerplugins/clang/useuniqueptr.cxx b/compilerplugins/clang/useuniqueptr.cxx
index c35426727cdc..0c0e25d2ebee 100644
--- a/compilerplugins/clang/useuniqueptr.cxx
+++ b/compilerplugins/clang/useuniqueptr.cxx
@@ -38,10 +38,9 @@ public:
     bool VisitCXXDestructorDecl(const CXXDestructorDecl* );
     bool VisitCompoundStmt(const CompoundStmt* );
 private:
-    void CheckForSingleUnconditionalDelete(const CXXDestructorDecl*, const CompoundStmt* );
+    void CheckForUnconditionalDelete(const CXXDestructorDecl*, const CompoundStmt* );
     void CheckForForLoopDelete(const CXXDestructorDecl*, const CompoundStmt* );
     void CheckForRangedLoopDelete(const CXXDestructorDecl*, const CompoundStmt* );
-    void CheckForDeleteOfPOD(const CompoundStmt* );
 };
 
 bool UseUniquePtr::VisitCXXDestructorDecl(const CXXDestructorDecl* destructorDecl)
@@ -52,215 +51,197 @@ bool UseUniquePtr::VisitCXXDestructorDecl(const CXXDestructorDecl* destructorDec
         return true;
 
     const CompoundStmt* compoundStmt = dyn_cast_or_null< CompoundStmt >( destructorDecl->getBody() );
-    if (!compoundStmt)
+    if (!compoundStmt || compoundStmt->size() == 0)
         return true;
 
-    if (compoundStmt->size() > 0)
-    {
-        CheckForSingleUnconditionalDelete(destructorDecl, compoundStmt);
-        CheckForDeleteOfPOD(compoundStmt);
-        CheckForForLoopDelete(destructorDecl, compoundStmt);
-        CheckForRangedLoopDelete(destructorDecl, compoundStmt);
-    }
+    CheckForUnconditionalDelete(destructorDecl, compoundStmt);
+    CheckForForLoopDelete(destructorDecl, compoundStmt);
+    CheckForRangedLoopDelete(destructorDecl, compoundStmt);
 
     return true;
 }
 
-void UseUniquePtr::CheckForSingleUnconditionalDelete(const CXXDestructorDecl* destructorDecl, const CompoundStmt* compoundStmt)
+void UseUniquePtr::CheckForUnconditionalDelete(const CXXDestructorDecl* destructorDecl, const CompoundStmt* compoundStmt)
 {
-    const CXXDeleteExpr* deleteExpr = nullptr;
-    if (compoundStmt->size() == 1) {
-        deleteExpr = dyn_cast<CXXDeleteExpr>(compoundStmt->body_front());
-    }
-    else if (compoundStmt->size() == 2) {
-        // ignore SAL_INFO type stuff
-        // @TODO should probably be a little more specific here
-        if (isa<DoStmt>(compoundStmt->body_front())) {
-            deleteExpr = dyn_cast<CXXDeleteExpr>(compoundStmt->body_back());
-        }
-        // look for the following pattern:
-        // delete m_pbar;
-        // m_pbar = nullptr;
-        else if (auto binaryOp = dyn_cast<BinaryOperator>(compoundStmt->body_back())) {
-            if (binaryOp->getOpcode() == BO_Assign)
-                deleteExpr = dyn_cast<CXXDeleteExpr>(compoundStmt->body_front());
-        }
-    } else {
-        // look for the following pattern:
-        // delete m_pbar;
-        // m_pbar = nullptr;
-        if (auto binaryOp = dyn_cast<BinaryOperator>(compoundStmt->body_back())) {
-            if (binaryOp->getOpcode() == BO_Assign)
-                deleteExpr = dyn_cast<CXXDeleteExpr>(*(++compoundStmt->body_rbegin()));
-        }
-    }
-    if (!deleteExpr)
-        return;
+    for (auto i = compoundStmt->body_begin(); i != compoundStmt->body_end(); ++i)
+    {
+        auto deleteExpr = dyn_cast<CXXDeleteExpr>(*i);
+        if (!deleteExpr)
+            continue;
 
-    const ImplicitCastExpr* pCastExpr = dyn_cast<ImplicitCastExpr>(deleteExpr->getArgument());
-    if (!pCastExpr)
-        return;
-    const MemberExpr* pMemberExpr = dyn_cast<MemberExpr>(pCastExpr->getSubExpr());
-    if (!pMemberExpr)
-        return;
-
-    // ignore union games
-    const FieldDecl* pFieldDecl = dyn_cast<FieldDecl>(pMemberExpr->getMemberDecl());
-    if (!pFieldDecl)
-        return;
-    TagDecl const * td = dyn_cast<TagDecl>(pFieldDecl->getDeclContext());
-    if (td->isUnion())
-        return;
-
-    // ignore calling delete on someone else's field
-    if (pFieldDecl->getParent() != destructorDecl->getParent() )
-        return;
-
-    if (ignoreLocation(pFieldDecl))
-        return;
-    // to ignore things like the CPPUNIT macros
-    StringRef aFileName = compiler.getSourceManager().getFilename(compiler.getSourceManager().getSpellingLoc(pFieldDecl->getLocStart()));
-    if (loplugin::hasPathnamePrefix(aFileName, WORKDIR))
-        return;
-    // passes and stores pointers to member fields
-    if (loplugin::hasPathnamePrefix(aFileName, SRCDIR "/sot/source/sdstor/stgdir.hxx"))
-        return;
-    // something platform-specific
-    if (loplugin::hasPathnamePrefix(aFileName, SRCDIR "/hwpfilter/source/htags.h"))
-        return;
-    // passes pointers to member fields
-    if (loplugin::hasPathnamePrefix(aFileName, SRCDIR "/sd/inc/sdpptwrp.hxx"))
-        return;
-    // @TODO intrusive linked-lists here, with some trickiness
-    if (loplugin::hasPathnamePrefix(aFileName, SRCDIR "/sw/source/filter/html/parcss1.hxx"))
-        return;
-    // @TODO SwDoc has some weird ref-counting going on
-    if (loplugin::hasPathnamePrefix(aFileName, SRCDIR "/sw/inc/shellio.hxx"))
-        return;
-    // @TODO it's sharing pointers with another class
-    if (loplugin::hasPathnamePrefix(aFileName, SRCDIR "/sc/inc/formulacell.hxx"))
-        return;
+        const ImplicitCastExpr* pCastExpr = dyn_cast<ImplicitCastExpr>(deleteExpr->getArgument());
+        if (!pCastExpr)
+            continue;
+        const MemberExpr* pMemberExpr = dyn_cast<MemberExpr>(pCastExpr->getSubExpr());
+        if (!pMemberExpr)
+            continue;
 
-    report(
-        DiagnosticsEngine::Warning,
-        "a destructor with only a single unconditional call to delete on a member, is a sure sign it should be using std::unique_ptr for that field",
-        deleteExpr->getLocStart())
-        << deleteExpr->getSourceRange();
-    report(
-        DiagnosticsEngine::Note,
-        "member is here",
-        pFieldDecl->getLocStart())
-        << pFieldDecl->getSourceRange();
+        // ignore union games
+        const FieldDecl* pFieldDecl = dyn_cast<FieldDecl>(pMemberExpr->getMemberDecl());
+        if (!pFieldDecl)
+            continue;
+        TagDecl const * td = dyn_cast<TagDecl>(pFieldDecl->getDeclContext());
+        if (td->isUnion())
+            continue;
+
+        // ignore calling delete on someone else's field
+        if (pFieldDecl->getParent() != destructorDecl->getParent() )
+            continue;
+
+        if (ignoreLocation(pFieldDecl))
+            continue;
+        // to ignore things like the CPPUNIT macros
+        StringRef aFileName = compiler.getSourceManager().getFilename(compiler.getSourceManager().getSpellingLoc(pFieldDecl->getLocStart()));
+        if (loplugin::hasPathnamePrefix(aFileName, WORKDIR))
+            continue;
+        // passes and stores pointers to member fields
+        if (loplugin::hasPathnamePrefix(aFileName, SRCDIR "/sot/source/sdstor/stgdir.hxx"))
+            continue;
+        // something platform-specific
+        if (loplugin::hasPathnamePrefix(aFileName, SRCDIR "/hwpfilter/source/htags.h"))
+            continue;
+        // passes pointers to member fields
+        if (loplugin::hasPathnamePrefix(aFileName, SRCDIR "/sd/inc/sdpptwrp.hxx"))
+            continue;
+        // @TODO intrusive linked-lists here, with some trickiness
+        if (loplugin::hasPathnamePrefix(aFileName, SRCDIR "/sw/source/filter/html/parcss1.hxx"))
+            continue;
+        // @TODO SwDoc has some weird ref-counting going on
+        if (loplugin::hasPathnamePrefix(aFileName, SRCDIR "/sw/inc/shellio.hxx"))
+            continue;
+        // @TODO it's sharing pointers with another class
+        if (loplugin::hasPathnamePrefix(aFileName, SRCDIR "/sc/inc/formulacell.hxx"))
+            continue;
+
+        report(
+            DiagnosticsEngine::Warning,
+            "unconditional call to delete on a member, should be using std::unique_ptr",
+            deleteExpr->getLocStart())
+            << deleteExpr->getSourceRange();
+        report(
+            DiagnosticsEngine::Note,
+            "member is here",
+            pFieldDecl->getLocStart())
+            << pFieldDecl->getSourceRange();
+    }
 }
 
 void UseUniquePtr::CheckForForLoopDelete(const CXXDestructorDecl* destructorDecl, const CompoundStmt* compoundStmt)
 {
-    auto forStmt = dyn_cast<ForStmt>(compoundStmt->body_back());
-    if (!forStmt)
-        return;
-    auto deleteExpr = dyn_cast<CXXDeleteExpr>(forStmt->getBody());
-    if (!deleteExpr)
-        return;
-
-    const MemberExpr* memberExpr = nullptr;
-    const Expr* subExpr = deleteExpr->getArgument();
-    for (;;)
+    for (auto i = compoundStmt->body_begin(); i != compoundStmt->body_end(); ++i)
     {
-        subExpr = subExpr->IgnoreImpCasts();
-        if ((memberExpr = dyn_cast<MemberExpr>(subExpr)))
-            break;
-        else if (auto arraySubscriptExpr = dyn_cast<ArraySubscriptExpr>(subExpr))
-            subExpr = arraySubscriptExpr->getBase();
-        else if (auto cxxOperatorCallExpr = dyn_cast<CXXOperatorCallExpr>(subExpr))
+        auto forStmt = dyn_cast<ForStmt>(*i);
+        if (!forStmt)
+            continue;
+        auto deleteExpr = dyn_cast<CXXDeleteExpr>(forStmt->getBody());
+        if (!deleteExpr)
+            continue;
+
+        const MemberExpr* memberExpr = nullptr;
+        const Expr* subExpr = deleteExpr->getArgument();
+        for (;;)
         {
-            if (cxxOperatorCallExpr->getOperator() == OO_Subscript)
+            subExpr = subExpr->IgnoreImpCasts();
+            if ((memberExpr = dyn_cast<MemberExpr>(subExpr)))
+                break;
+            else if (auto arraySubscriptExpr = dyn_cast<ArraySubscriptExpr>(subExpr))
+                subExpr = arraySubscriptExpr->getBase();
+            else if (auto cxxOperatorCallExpr = dyn_cast<CXXOperatorCallExpr>(subExpr))
             {
-                memberExpr = dyn_cast<MemberExpr>(cxxOperatorCallExpr->getArg(0));
+                if (cxxOperatorCallExpr->getOperator() == OO_Subscript)
+                {
+                    memberExpr = dyn_cast<MemberExpr>(cxxOperatorCallExpr->getArg(0));
+                    break;
+                }
                 break;
             }
-            return;
+            else
+                break;
         }
-        else
-            return;
-    }
+        if (!memberExpr)
+            continue;
 
-    // ignore union games
-    const FieldDecl* fieldDecl = dyn_cast<FieldDecl>(memberExpr->getMemberDecl());
-    if (!fieldDecl)
-        return;
-    TagDecl const * td = dyn_cast<TagDecl>(fieldDecl->getDeclContext());
-    if (td->isUnion())
-        return;
-
-    // ignore calling delete on someone else's field
-    if (fieldDecl->getParent() != destructorDecl->getParent() )
-        return;
-
-    if (ignoreLocation(fieldDecl))
-        return;
-    // to ignore things like the CPPUNIT macros
-    StringRef aFileName = compiler.getSourceManager().getFilename(compiler.getSourceManager().getSpellingLoc(fieldDecl->getLocStart()));
-    if (loplugin::hasPathnamePrefix(aFileName, WORKDIR))
-        return;
+        // ignore union games
+        const FieldDecl* fieldDecl = dyn_cast<FieldDecl>(memberExpr->getMemberDecl());
+        if (!fieldDecl)
+            continue;
+        TagDecl const * td = dyn_cast<TagDecl>(fieldDecl->getDeclContext());
+        if (td->isUnion())
+            continue;
 
-    report(
-        DiagnosticsEngine::Warning,
-        "rather manage with std::some_container<std::unique_ptr<T>>",
-        deleteExpr->getLocStart())
-        << deleteExpr->getSourceRange();
-    report(
-        DiagnosticsEngine::Note,
-        "member is here",
-        fieldDecl->getLocStart())
-        << fieldDecl->getSourceRange();
+        // ignore calling delete on someone else's field
+        if (fieldDecl->getParent() != destructorDecl->getParent() )
+            continue;
+
+        if (ignoreLocation(fieldDecl))
+            continue;
+        // to ignore things like the CPPUNIT macros
+        StringRef aFileName = compiler.getSourceManager().getFilename(compiler.getSourceManager().getSpellingLoc(fieldDecl->getLocStart()));
+        if (loplugin::hasPathnamePrefix(aFileName, WORKDIR))
+            continue;
+
+        report(
+            DiagnosticsEngine::Warning,
+            "rather manage with std::some_container<std::unique_ptr<T>>",
+            deleteExpr->getLocStart())
+            << deleteExpr->getSourceRange();
+        report(
+            DiagnosticsEngine::Note,
+            "member is here",
+            fieldDecl->getLocStart())
+            << fieldDecl->getSourceRange();
+    }
 }
 
 void UseUniquePtr::CheckForRangedLoopDelete(const CXXDestructorDecl* destructorDecl, const CompoundStmt* compoundStmt)
 {
-    auto cxxForRangeStmt = dyn_cast<CXXForRangeStmt>(compoundStmt->body_back());
-    if (!cxxForRangeStmt)
-        return;
-    auto deleteExpr = dyn_cast<CXXDeleteExpr>(cxxForRangeStmt->getBody());
-    if (!deleteExpr)
-        return;
-    auto memberExpr = dyn_cast<MemberExpr>(cxxForRangeStmt->getRangeInit());
-    if (!memberExpr)
-        return;
-    auto fieldDecl = dyn_cast<FieldDecl>(memberExpr->getMemberDecl());
-    if (!fieldDecl)
-        return;
-
-    // ignore union games
-    TagDecl const * td = dyn_cast<TagDecl>(fieldDecl->getDeclContext());
-    if (td->isUnion())
-        return;
-
-    // ignore calling delete on someone else's field
-    if (fieldDecl->getParent() != destructorDecl->getParent() )
-        return;
-
-    if (ignoreLocation(fieldDecl))
-        return;
-
-    // to ignore things like the CPPUNIT macros
-    StringRef aFileName = compiler.getSourceManager().getFilename(compiler.getSourceManager().getSpellingLoc(fieldDecl->getLocStart()));
-    if (loplugin::hasPathnamePrefix(aFileName, WORKDIR))
-        return;
-    // ignore std::map and std::unordered_map, MSVC 2015 has problems with mixing these with std::unique_ptr
-    auto tc = loplugin::TypeCheck(fieldDecl->getType());
-    if (tc.Class("map").StdNamespace() || tc.Class("unordered_map").StdNamespace())
-        return;
+    for (auto i = compoundStmt->body_begin(); i != compoundStmt->body_end(); ++i)
+    {
+        auto cxxForRangeStmt = dyn_cast<CXXForRangeStmt>(*i);
+        if (!cxxForRangeStmt)
+            continue;
+        auto deleteExpr = dyn_cast<CXXDeleteExpr>(cxxForRangeStmt->getBody());
+        if (!deleteExpr)
+            continue;
+        auto memberExpr = dyn_cast<MemberExpr>(cxxForRangeStmt->getRangeInit());
+        if (!memberExpr)
+            continue;
+        auto fieldDecl = dyn_cast<FieldDecl>(memberExpr->getMemberDecl());
+        if (!fieldDecl)
+            continue;
 
-    report(
-        DiagnosticsEngine::Warning,
-        "rather manage with std::some_container<std::unique_ptr<T>>",
-        deleteExpr->getLocStart())
-        << deleteExpr->getSourceRange();
-    report(
-        DiagnosticsEngine::Note,
-        "member is here",
-        fieldDecl->getLocStart())
-        << fieldDecl->getSourceRange();
+        // ignore union games
+        TagDecl const * td = dyn_cast<TagDecl>(fieldDecl->getDeclContext());
+        if (td->isUnion())
+            continue;
+
+        // ignore calling delete on someone else's field
+        if (fieldDecl->getParent() != destructorDecl->getParent() )
+            continue;
+
+        if (ignoreLocation(fieldDecl))
+            continue;
+
+        // to ignore things like the CPPUNIT macros
+        StringRef aFileName = compiler.getSourceManager().getFilename(compiler.getSourceManager().getSpellingLoc(fieldDecl->getLocStart()));
+        if (loplugin::hasPathnamePrefix(aFileName, WORKDIR))
+            continue;
+        // ignore std::map and std::unordered_map, MSVC 2015 has problems with mixing these with std::unique_ptr
+        auto tc = loplugin::TypeCheck(fieldDecl->getType());
+        if (tc.Class("map").StdNamespace() || tc.Class("unordered_map").StdNamespace())
+            continue;
+
+        report(
+            DiagnosticsEngine::Warning,
+            "rather manage with std::some_container<std::unique_ptr<T>>",
+            deleteExpr->getLocStart())
+            << deleteExpr->getSourceRange();
+        report(
+            DiagnosticsEngine::Note,
+            "member is here",
+            fieldDecl->getLocStart())
+            << fieldDecl->getSourceRange();
+    }
 }
 
 bool UseUniquePtr::VisitCompoundStmt(const CompoundStmt* compoundStmt)
@@ -314,66 +295,7 @@ bool UseUniquePtr::VisitCompoundStmt(const CompoundStmt* compoundStmt)
     return true;
 }
 
-void UseUniquePtr::CheckForDeleteOfPOD(const CompoundStmt* compoundStmt)
-{
-    for (auto i = compoundStmt->body_begin();
-              i != compoundStmt->body_end(); ++i)
-    {
-        auto deleteExpr = dyn_cast<CXXDeleteExpr>(*i);
-        if (!deleteExpr)
-            continue;
-
-        const Expr* argExpr = deleteExpr->getArgument();
-        if (auto implicitCastExpr = dyn_cast<ImplicitCastExpr>(deleteExpr->getArgument()))
-            argExpr = implicitCastExpr->getSubExpr();
-
-        auto memberExpr = dyn_cast<MemberExpr>(argExpr);
-        if (!memberExpr)
-            continue;
-        auto fieldDecl = dyn_cast<FieldDecl>(memberExpr->getMemberDecl());
-        if (!fieldDecl)
-            continue;
-        // ignore union games
-        auto * tagDecl = dyn_cast<TagDecl>(fieldDecl->getDeclContext());
-        if (tagDecl->isUnion())
-            continue;
-
-        auto pointerType = dyn_cast<clang::PointerType>(fieldDecl->getType()->getUnqualifiedDesugaredType());
-        QualType elementType = pointerType->getPointeeType();
-        auto tc = loplugin::TypeCheck(elementType);
-        if (!elementType.isPODType(compiler.getASTContext())
-            && !tc.Class("OUString").Namespace("rtl").GlobalNamespace()
-            && !tc.Class("OString").Namespace("rtl").GlobalNamespace())
-            continue;
-
-        StringRef aFileName = compiler.getSourceManager().getFilename(compiler.getSourceManager().getSpellingLoc(fieldDecl->getLocStart()));
-        // TODO ignore this for now, it's just so messy to fix
-        if (loplugin::hasPathnamePrefix(aFileName, SRCDIR "/include/tools/stream.hxx"))
-            continue;
-        // TODO this is very performance sensitive code
-        if (loplugin::hasPathnamePrefix(aFileName, SRCDIR "/include/svl/itemset.hxx"))
-            continue;
-        // WW8TabBandDesc is playing games with copying/assigning itself
-        if (loplugin::hasPathnamePrefix(aFileName, SRCDIR "/sw/source/filter/ww8/ww8par.hxx"))
-            continue;
-
-        report(
-            DiagnosticsEngine::Warning,
-            "managing POD type %0 manually, rather use std::vector / std::array / std::unique_ptr",
-            deleteExpr->getLocStart())
-            << elementType
-            << deleteExpr->getSourceRange();
-        report(
-            DiagnosticsEngine::Note,
-            "member is here",
-            fieldDecl->getLocStart())
-            << fieldDecl->getSourceRange();
-    }
-}
-
-
-
-loplugin::Plugin::Registration< UseUniquePtr > X("useuniqueptr", true);
+loplugin::Plugin::Registration< UseUniquePtr > X("useuniqueptr", false);
 
 }
 
diff --git a/l10ntools/inc/cfgmerge.hxx b/l10ntools/inc/cfgmerge.hxx
index d2ec28475d39..d9ed5b6c356f 100644
--- a/l10ntools/inc/cfgmerge.hxx
+++ b/l10ntools/inc/cfgmerge.hxx
@@ -155,7 +155,7 @@ public:
 class CfgMerge : public CfgParser
 {
 private:
-    MergeDataFile *pMergeDataFile;
+    std::unique_ptr<MergeDataFile> pMergeDataFile;
     std::vector<OString> aLanguages;
     std::unique_ptr<ResData> pResData;
 
diff --git a/l10ntools/inc/lngmerge.hxx b/l10ntools/inc/lngmerge.hxx
index 24d5b6ed211f..35c7868b01a3 100644
--- a/l10ntools/inc/lngmerge.hxx
+++ b/l10ntools/inc/lngmerge.hxx
@@ -28,8 +28,6 @@
 #include "common.hxx"
 #include "export.hxx"
 
-typedef std::vector< OString* > LngLineList;
-
 #define LNG_OK              0x0000
 #define LNG_COULD_NOT_OPEN  0x0001
 
@@ -43,7 +41,7 @@ typedef std::vector< OString* > LngLineList;
 class LngParser
 {
 private:
-    LngLineList *pLines;
+    std::vector<OString> mvLines;
     OString sSource;
     std::vector<OString> aLanguages;
 
diff --git a/l10ntools/inc/xrmmerge.hxx b/l10ntools/inc/xrmmerge.hxx
index 0000265cac35..7d55bcd89ac7 100644
--- a/l10ntools/inc/xrmmerge.hxx
+++ b/l10ntools/inc/xrmmerge.hxx
@@ -104,7 +104,7 @@ public:
 class XRMResMerge : public XRMResParser
 {
 private:
-    MergeDataFile *pMergeDataFile;
+    std::unique_ptr<MergeDataFile> pMergeDataFile;
     OString sFilename;
     std::unique_ptr<ResData> pResData;
     std::ofstream pOutputStream;
diff --git a/l10ntools/source/cfgmerge.cxx b/l10ntools/source/cfgmerge.cxx
index 60e05d1863ad..2eda993d72f7 100644
--- a/l10ntools/source/cfgmerge.cxx
+++ b/l10ntools/source/cfgmerge.cxx
@@ -103,9 +103,6 @@ CfgStackData* CfgStack::Push(const OString &rTag, const OString &rId)
 
 CfgStack::~CfgStack()
 {
-    for ( size_t i = 0, n = maList.size(); i < n; i++ )
-        delete maList[ i ];
-    maList.clear();
 }
 
 OString CfgStack::GetAccessPath( size_t nPos )
@@ -410,8 +407,8 @@ CfgMerge::CfgMerge(
 
     if (!rMergeSource.isEmpty())
     {
-        pMergeDataFile = new MergeDataFile(
-            rMergeSource, global::inputPathname, true );
+        pMergeDataFile.reset(new MergeDataFile(
+            rMergeSource, global::inputPathname, true ));
         if (rLanguage.equalsIgnoreAsciiCase("ALL") )
         {
             aLanguages = pMergeDataFile->GetLanguages();
@@ -425,7 +422,6 @@ CfgMerge::CfgMerge(
 CfgMerge::~CfgMerge()
 {
     pOutputStream.close();
-    delete pMergeDataFile;
 }
 
 void CfgMerge::WorkOnText(OString &, const OString& rLangIndex)
diff --git a/l10ntools/source/lngmerge.cxx b/l10ntools/source/lngmerge.cxx
index b913a81249c8..f5215e4d14b2 100644
--- a/l10ntools/source/lngmerge.cxx
+++ b/l10ntools/source/lngmerge.cxx
@@ -49,10 +49,8 @@ void lcl_RemoveUTF8ByteOrderMarker( OString &rString )
 // class LngParser
 
 LngParser::LngParser(const OString &rLngFile)
-    : pLines( nullptr )
-    , sSource( rLngFile )
+    : sSource( rLngFile )
 {
-    pLines = new LngLineList;
     std::ifstream aStream(sSource.getStr());
     if (aStream.is_open())
     {
@@ -70,19 +68,15 @@ LngParser::LngParser(const OString &rLngFile)
                 bFirstLine = false;
             }
 
-            pLines->push_back( new OString(sLine) );
+            mvLines.push_back( sLine );
             std::getline(aStream, s);
         }
-        pLines->push_back( new OString() );
+        mvLines.push_back( OString() );
     }
 }
 
 LngParser::~LngParser()
 {
-    for ( size_t i = 0, n = pLines->size(); i < n; ++i )
-        delete (*pLines)[ i ];
-    pLines->clear();
-    delete pLines;
 }
 
 bool LngParser::CreatePO( const OString &rPOFile )
@@ -98,12 +92,12 @@ bool LngParser::CreatePO( const OString &rPOFile )
     OStringHashMap Text;
     OString sID;
 
-    while( nPos < pLines->size() ) {
-        sLine = *(*pLines)[ nPos++ ];
-        while( nPos < pLines->size() && !isNextGroup( sGroup , sLine ) ) {
+    while( nPos < mvLines.size() ) {
+        sLine = mvLines[ nPos++ ];
+        while( nPos < mvLines.size() && !isNextGroup( sGroup , sLine ) ) {
             ReadLine( sLine , Text );
             sID = sGroup;
-            sLine = *(*pLines)[ nPos++ ];
+            sLine = mvLines[ nPos++ ];
         }
         if( bStart ) {
             bStart = false;
@@ -168,9 +162,9 @@ bool LngParser::Merge(
     OString sGroup;
 
     // seek to next group
-    while ( nPos < pLines->size() && !bGroup )
+    while ( nPos < mvLines.size() && !bGroup )
     {
-        OString sLine( *(*pLines)[ nPos ] );
+        OString sLine( mvLines[ nPos ] );
         sLine = sLine.trim();
         if ( sLine.startsWith("[") && sLine.endsWith("]") )
         {
@@ -180,7 +174,7 @@ bool LngParser::Merge(
         nPos ++;
     }
 
-    while ( nPos < pLines->size()) {
+    while ( nPos < mvLines.size()) {
         OStringHashMap Text;
         OString sID( sGroup );
         std::size_t nLastLangPos = 0;
@@ -193,9 +187,9 @@ bool LngParser::Merge(
 
         OString sLanguagesDone;
 
-        while ( nPos < pLines->size() && !bGroup )
+        while ( nPos < mvLines.size() && !bGroup )
         {
-            OString sLine( *(*pLines)[ nPos ] );
+            OString sLine( mvLines[ nPos ] );
             sLine = sLine.trim();
             if ( sLine.startsWith("[") && sLine.endsWith("]") )
             {
@@ -221,9 +215,7 @@ bool LngParser::Merge(
                     sSearch += ";";
 
                     if ( sLanguagesDone.indexOf( sSearch ) != -1 ) {
-                        LngLineList::iterator it = pLines->begin();
-                        std::advance( it, nPos );
-                        pLines->erase( it );
+                        mvLines.erase( mvLines.begin() + nPos );
                     }
                     if( pEntrys )
                     {
@@ -235,14 +227,14 @@ bool LngParser::Merge(
                                 continue;
 
                             if ( !sNewText.isEmpty()) {
-                                OString *pLine = (*pLines)[ nPos ];
+                                OString & rLine = mvLines[ nPos ];
 
                                 OString sText1( sLang );
                                 sText1 += " = \"";
                                 // escape quotes, unescape double escaped quotes fdo#56648
                                 sText1 += sNewText.replaceAll("\"","\\\"").replaceAll("\\\\\"","\\\"");
                                 sText1 += "\"";
-                                *pLine = sText1;
+                                rLine = sText1;
                                 Text[ sLang ] = sNewText;
                             }
                         }
@@ -283,12 +275,10 @@ bool LngParser::Merge(
                         nLastLangPos++;
                         nPos++;
 
-                        if ( nLastLangPos < pLines->size() ) {
-                            LngLineList::iterator it = pLines->begin();
-                            std::advance( it, nLastLangPos );
-                            pLines->insert( it, new OString(sLine) );
+                        if ( nLastLangPos < mvLines.size() ) {
+                            mvLines.insert( mvLines.begin() + nLastLangPos, sLine );
                         } else {
-                            pLines->push_back( new OString(sLine) );
+                            mvLines.push_back( sLine );
                         }
                     }
                 }
@@ -296,8 +286,8 @@ bool LngParser::Merge(
         }
     }
 
-    for ( size_t i = 0; i < pLines->size(); ++i )
-        aDestination << *(*pLines)[i] << '\n';
+    for ( size_t i = 0; i < mvLines.size(); ++i )
+        aDestination << mvLines[i] << '\n';
 
     aDestination.close();
     return true;
diff --git a/l10ntools/source/xrmmerge.cxx b/l10ntools/source/xrmmerge.cxx
index b9102d793d39..6c747ad98654 100644
--- a/l10ntools/source/xrmmerge.cxx
+++ b/l10ntools/source/xrmmerge.cxx
@@ -375,8 +375,8 @@ XRMResMerge::XRMResMerge(
 {
     if (!rMergeSource.isEmpty() && sLanguage.equalsIgnoreAsciiCase("ALL"))
     {
-        pMergeDataFile = new MergeDataFile(
-            rMergeSource, sInputFileName, false);
+        pMergeDataFile.reset(new MergeDataFile(
+            rMergeSource, sInputFileName, false));
         aLanguages = pMergeDataFile->GetLanguages();
     }
     else
@@ -393,7 +393,6 @@ XRMResMerge::XRMResMerge(
 XRMResMerge::~XRMResMerge()
 {
     pOutputStream.close();
-    delete pMergeDataFile;
 }
 
 void XRMResMerge::WorkOnDesc(


More information about the Libreoffice-commits mailing list