Bug Hunting Session
Bug 94306 - Replace boost::noncopyable with plain C++11 deleted copy ctors
Summary: Replace boost::noncopyable with plain C++11 deleted copy ctors
Status: RESOLVED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: LibreOffice (show other bugs)
Version:
(earliest affected)
unspecified
Hardware: Other All
: medium normal
Assignee: Umang Jain
URL:
Whiteboard: ToBeReviewed target:5.2.0
Keywords: difficultyBeginner, easyHack, skillCpp, topicCleanup
Depends on:
Blocks:
 
Reported: 2015-09-17 10:21 UTC by Björn Michaelsen
Modified: 2017-02-14 08:57 UTC (History)
4 users (show)

See Also:
Crash report or crash signature:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Björn Michaelsen 2015-09-17 10:21:52 UTC
In C++11 you can use:

class NonCopyable {
public:
  NonCopyable(const NonCopyable&) = delete;
  const NonCopyable& operator=(const NonCopyable&) = delete;
};

to make a class non-copyable, thus there is no need for boost templates anymore.

To find this being used use:

 git grep boost::noncopyable
 git grep boost/noncopyable

and replace uses of boost::noncopyable with plain C++11 deleted copy constructors.
Comment 1 Umang Jain 2015-09-23 13:56:28 UTC
Hi, This is my first bug fix and I am a new contributor with very little experience. I successfully build libreoffice and tried 
git grep boost::noncopyable
 git grep boost/noncopyable

I see that there has to be changes in the MANY files. how to approach that ?
Comment 2 Noel Grandin 2015-09-23 15:24:25 UTC
Umang, you should make changes in at most one module at a time.
Where a module is all of the code under a top-level directory like vcl/ or sd/
Comment 3 Umang Jain 2015-09-23 16:46:16 UTC
ok. So correct me if I am wrong. Have to make changes module by module, then run "make" for that particular module and examine if some error pops up ?
Comment 4 Miklos Vajna 2015-09-23 21:12:25 UTC
Let's say you pick "sd". Do your changes in "sd", then build the changed code with "make sd.build", and iterate it till you're done with your changes and the code builds. Then run "make check" to run all the automated tests. If all of them pass, then submit the patch to gerrit. :-)
Comment 5 Umang Jain 2015-09-26 05:18:56 UTC
There are some structures using boost noncopyable template, how to remove that ? Does it(i.e. structs) even required to be removed or just only the classes.

Like in :

core/undoanim.cxx:struct UndoAnimationPathImpl: private boost::noncopyable
Comment 6 Daniel L Robertson 2015-09-26 05:43:25 UTC
The only real difference between structs and classes is the default access.

class NonCopyable {
public:
  NonCopyable(const NonCopyable&) = delete;
  const NonCopyable& operator=(const NonCopyable&) = delete;
};

is essentially the same as

struct NonCopyable {
  NonCopyable(const NonCopyable&) = delete;
  const NonCopyable& operator=(const NonCopyable&) = delete;
};
Comment 7 Umang Jain 2015-09-26 15:09:32 UTC
ok regarding the sd/ module, I am not able to make changes in: 

sd/source/ui/remotecontrol/IBluetoothSocket.hxx : (line) struct IBluetoothSocket : private boost::noncopyable

If I change with :

class NonCopyable {
public:
  NonCopyable(const NonCopyable&) = delete;
  const NonCopyable& operator=(const NonCopyable&) = delete;
};

It does not builds successfully and throws errors. If I allow boost templates(only for this file), It build successfully. What should I do ?
Comment 8 Daniel L Robertson 2015-09-26 21:18:36 UTC
I just took a look at it, and if I'm correct this is due to the fact that there is no default constructor in IBluetoothSocket. boost::noncopyable defines a default constructor. As a result, you might try something like the following.

struct IBluetoothSocket
{
protected: // I personally would use protected but someone else may disagree
    IBluetoothSocket() = default;
public:
    // everything else
};

No worries, but typically if you have questions regarding errors it is a good idea to post them  :-)
Comment 9 Siddharth Khabia 2015-09-27 04:43:21 UTC
Like umang this is my first bug too. i have a working build .
in a file in sw :
inc/breakit.hxx :
 should this :
class SW_DLLPUBLIC SwBreakIt : private ::boost::noncopyable
{
    com::sun::star::uno::Reference< com::sun::star::uno::XComponentContext > m_xContext;
    mutable com::sun::star::uno::Reference< com::sun::star::i18n::XBreakIterator > xBreak;

    LanguageTag * m_pLanguageTag;   ///< language tag of the current locale
    com::sun::star::i18n::ForbiddenCharacters * m_pForbidden;

    LanguageType aForbiddenLang; ///< language of the current forbiddenChar struct

    void _GetLocale( const LanguageType aLang );
    void _GetLocale( const LanguageTag& rLanguageTag );
    void _GetForbidden( const LanguageType  aLang );

    void createBreakIterator() const;

    // private (see @ _Create, _Delete).
    explicit SwBreakIt(
        const com::sun::star::uno::Reference< com::sun::star::uno::XComponentContext > & rxContext);
    ~SwBreakIt();

public:
    // private (see @ source/core/bastyp/init.cxx).
    static void _Create(
        const com::sun::star::uno::Reference< com::sun::star::uno::XComponentContext > & rxContext);
    static void _Delete();

public:
    static SwBreakIt * Get();

    com::sun::star::uno::Reference< com::sun::star::i18n::XBreakIterator > GetBreakIter()
    {
        createBreakIterator();
        return xBreak;
    }

    const com::sun::star::lang::Locale& GetLocale( const LanguageType aLang )
    {
        if( !m_pLanguageTag || m_pLanguageTag->getLanguageType() != aLang )
            _GetLocale( aLang );
        return m_pLanguageTag->getLocale();
    }

    const com::sun::star::lang::Locale& GetLocale( const LanguageTag& rLanguageTag )
    {
        // Use LanguageType comparison instead of LanguageTag::operator!=()
        // because here the LanguageTag is already a known LanguageType value
        // assigned, so LanguageTag does not need to convert to BCP47 for
        // comparison.
        if( !m_pLanguageTag || m_pLanguageTag->getLanguageType() != rLanguageTag.getLanguageType() )
            _GetLocale( rLanguageTag );
        return m_pLanguageTag->getLocale();
    }

    const LanguageTag& GetLanguageTag( const LanguageType aLang )
    {
        if( !m_pLanguageTag || m_pLanguageTag->getLanguageType() != aLang )
            _GetLocale( aLang );
        return *m_pLanguageTag;
    }

    const LanguageTag& GetLanguageTag( const LanguageTag& rLanguageTag )
    {
        // Use LanguageType comparison instead of LanguageTag::operator!=()
        // because here the LanguageTag is already a known LanguageType value
        // assigned, so LanguageTag does not need to convert to BCP47 for
        // comparison.
        if( !m_pLanguageTag || m_pLanguageTag->getLanguageType() != rLanguageTag.getLanguageType() )
            _GetLocale( rLanguageTag );
        return *m_pLanguageTag;
    }

    const com::sun::star::i18n::ForbiddenCharacters& GetForbidden( const LanguageType aLang )
    {
        if( !m_pForbidden || aForbiddenLang != aLang )
            _GetForbidden( aLang );
        return *m_pForbidden;
    }

    sal_uInt16 GetRealScriptOfText( const OUString& rText, sal_Int32 nPos ) const;
    SvtScriptType GetAllScriptsOfText( const OUString& rText ) const;

    sal_Int32 getGraphemeCount(const OUString& rStr,
        sal_Int32 nStart, sal_Int32 nEnd) const;
    sal_Int32 getGraphemeCount(const OUString& rStr) const
    {
        return getGraphemeCount(rStr, 0, rStr.getLength());
    }
};

be change d to this :

class SW_DLLPUBLIC SwBreakIt : private
{
    com::sun::star::uno::Reference< com::sun::star::uno::XComponentContext > m_xContext;
    mutable com::sun::star::uno::Reference< com::sun::star::i18n::XBreakIterator > xBreak;

    LanguageTag * m_pLanguageTag;   ///< language tag of the current locale
    com::sun::star::i18n::ForbiddenCharacters * m_pForbidden;

    LanguageType aForbiddenLang; ///< language of the current forbiddenChar struct

    void _GetLocale( const LanguageType aLang );
    void _GetLocale( const LanguageTag& rLanguageTag );
    void _GetForbidden( const LanguageType  aLang );

    void createBreakIterator() const;

    // private (see @ _Create, _Delete).
    explicit SwBreakIt(
        const com::sun::star::uno::Reference< com::sun::star::uno::XComponentContext > & rxContext);
    ~SwBreakIt();

public:


    // private (see @ source/core/bastyp/init.cxx).
    static void _Create(
        const com::sun::star::uno::Reference< com::sun::star::uno::XComponentContext > & rxContext);
    static void _Delete();

public:
    static SwBreakIt * Get();

    com::sun::star::uno::Reference< com::sun::star::i18n::XBreakIterator > GetBreakIter()
    {
        createBreakIterator();
        return xBreak;
    }

    const com::sun::star::lang::Locale& GetLocale( const LanguageType aLang )
    {
        if( !m_pLanguageTag || m_pLanguageTag->getLanguageType() != aLang )
            _GetLocale( aLang );
        return m_pLanguageTag->getLocale();
    }

    const com::sun::star::lang::Locale& GetLocale( const LanguageTag& rLanguageTag )
    {
        // Use LanguageType comparison instead of LanguageTag::operator!=()
        // because here the LanguageTag is already a known LanguageType value
        // assigned, so LanguageTag does not need to convert to BCP47 for
        // comparison.
        if( !m_pLanguageTag || m_pLanguageTag->getLanguageType() != rLanguageTag.getLanguageType() )
            _GetLocale( rLanguageTag );
        return m_pLanguageTag->getLocale();
    }

    const LanguageTag& GetLanguageTag( const LanguageType aLang )
    {
        if( !m_pLanguageTag || m_pLanguageTag->getLanguageType() != aLang )
            _GetLocale( aLang );
        return *m_pLanguageTag;
    }

    const LanguageTag& GetLanguageTag( const LanguageTag& rLanguageTag )
    {
        // Use LanguageType comparison instead of LanguageTag::operator!=()
        // because here the LanguageTag is already a known LanguageType value
        // assigned, so LanguageTag does not need to convert to BCP47 for
        // comparison.
        if( !m_pLanguageTag || m_pLanguageTag->getLanguageType() != rLanguageTag.getLanguageType() )
            _GetLocale( rLanguageTag );
        return *m_pLanguageTag;
    }

    const com::sun::star::i18n::ForbiddenCharacters& GetForbidden( const LanguageType aLang )
    {
        if( !m_pForbidden || aForbiddenLang != aLang )
            _GetForbidden( aLang );
        return *m_pForbidden;
    }

    sal_uInt16 GetRealScriptOfText( const OUString& rText, sal_Int32 nPos ) const;
    SvtScriptType GetAllScriptsOfText( const OUString& rText ) const;

    sal_Int32 getGraphemeCount(const OUString& rStr,
        sal_Int32 nStart, sal_Int32 nEnd) const;
    sal_Int32 getGraphemeCount(const OUString& rStr) const
    {
        return getGraphemeCount(rStr, 0, rStr.getLength());
    }
};
Comment 10 Siddharth Khabia 2015-09-27 04:46:12 UTC
my apology for the earlier comment that was posted by mistake.
i want to ask is that are we supposed to remove boost::noncopyable and add the code provided bu BJORN in the public part of the class ?

if not please guide me , as this is my first bug too !
Comment 11 Siddharth Khabia 2015-09-27 14:17:07 UTC
i find this bug great to start off with so can i also join in with umang and do the sw module to get hang of this ?

once i am done with with sw how do i submit it ?
Comment 12 Siddharth Khabia 2015-09-27 14:19:15 UTC Comment hidden (obsolete)
Comment 13 Stephan Bergmann 2015-09-28 07:06:53 UTC
(In reply to Daniel L Robertson from comment #8)
> I just took a look at it, and if I'm correct this is due to the fact that
> there is no default constructor in IBluetoothSocket. boost::noncopyable
> defines a default constructor. As a result, you might try something like the
> following.

It looks somewhat misguided to derive the IBluetoothSocket "interface" from boost::noncopyable in the first place.  Just drop that, and let any class deriving from IBluetoothSocket decide for itself whether or not it should be copyable.
Comment 14 Stephan Bergmann 2015-09-28 07:08:24 UTC
(In reply to Siddharth Khabia from comment #10)
> my apology for the earlier comment that was posted by mistake.
> i want to ask is that are we supposed to remove boost::noncopyable and add
> the code provided bu BJORN in the public part of the class ?

You can put those deleted members into a private section (as they cannot be called anyway, as they are deleted).
Comment 15 Kohei Yoshida 2015-09-28 12:08:02 UTC
(In reply to Stephan Bergmann from comment #14)
> (In reply to Siddharth Khabia from comment #10)
> > my apology for the earlier comment that was posted by mistake.
> > i want to ask is that are we supposed to remove boost::noncopyable and add
> > the code provided bu BJORN in the public part of the class ?
> 
> You can put those deleted members into a private section (as they cannot be
> called anyway, as they are deleted).

Actually, Scott Meyers recommends those members be put in a public section, for better compiler error messages (c.f. Item 11 in Effective Modern C++).
Comment 16 Umang Jain 2015-09-29 06:13:27 UTC
(In reply to Siddharth Khabia from comment #12)
> i find this bug great to start off with so can i also join in with umang and
> do the sw module to get hang of this ?
> 
> once i am done with with sw how do i submit it ?

Great. Actually there are many changes to make considering all modules. So lets join and get this done. Lets connect via E-mail. I have already done changes in about 9-10 modules. You can start with remaining ones.
Comment 17 Siddharth Khabia 2015-09-29 11:58:16 UTC
I will be glad to do that !
have you completed sw , i changed a few files of that ?
Comment 18 Siddharth Khabia 2015-09-29 12:02:42 UTC
(In reply to Kohei Yoshida from comment #15)
> (In reply to Stephan Bergmann from comment #14)
> > (In reply to Siddharth Khabia from comment #10)
> > > my apology for the earlier comment that was posted by mistake.
> > > i want to ask is that are we supposed to remove boost::noncopyable and add
> > > the code provided bu BJORN in the public part of the class ?
> > 
> > You can put those deleted members into a private section (as they cannot be
> > called anyway, as they are deleted).
> 
> Actually, Scott Meyers recommends those members be put in a public section,
> for better compiler error messages (c.f. Item 11 in Effective Modern C++).

thanks for the help ! :)
Comment 19 Siddharth Khabia 2015-10-04 09:43:59 UTC
i have replace uses of boost::noncopyable in a module
but when i do boost/noncopyable , it shows its use as a header file.
will only commenting out/deleteing those headere files work ?
Comment 20 Robinson Tryon (qubit) 2015-12-10 11:40:53 UTC Comment hidden (obsolete)
Comment 21 Robinson Tryon (qubit) 2016-02-18 14:51:24 UTC Comment hidden (obsolete)
Comment 22 Commit Notification 2016-03-08 16:13:53 UTC
Steven Guo committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=5819268ad709f52417b59421260e86e9c7e90f75

tdf#94306 Replace boost::noncopyable with plain C++11 deleted copy ctors

It will be available in 5.2.0.

The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 23 Commit Notification 2016-04-01 08:57:31 UTC
Jochen Nitschke committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=3c43a4810f505c071bcc99aeda47162a4b7b1681

tdf#94306 Replace boost::noncopyable in sc/source

It will be available in 5.2.0.

The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 24 Commit Notification 2016-04-04 06:56:56 UTC
Jochen Nitschke committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=8c2f2e1dd77cdce9bdf63beff5a79f91adc44630

tdf#94306 replace boost::noncopyable in ..

It will be available in 5.2.0.

The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 25 Commit Notification 2016-04-05 06:59:55 UTC
Steven Guo committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=be3c2ff9233e8d4c5afe9c696cb5a60b24b25efc

tdf#94306 Replace boost::noncopyable with plain C++11 deleted copy ctors

It will be available in 5.2.0.

The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 26 Commit Notification 2016-04-07 07:30:51 UTC
Jochen Nitschke committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=f9b200ce54cd67ddc04747f9676568a86e14d864

tdf#94306 replace boost::noncopyable in canvas

It will be available in 5.2.0.

The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 27 Commit Notification 2016-04-08 09:35:16 UTC
Jochen Nitschke committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=98d7b02f2b69f2f88a03054183933df7f190017d

tdf#94306 replace boost::noncopyable in cppuhelper

It will be available in 5.2.0.

The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 28 Commit Notification 2016-04-08 10:01:15 UTC
Jochen Nitschke committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=f781997ee1dcb61b01b04cc050001e2f46b12dfe

tdf#94306 replace boost::noncopyable in c...

It will be available in 5.2.0.

The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 29 Commit Notification 2016-04-08 17:42:19 UTC
Jochen Nitschke committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=25934decf8bfd94506bccd48ac66be9d7eb4dce2

tdf#94306 replace boost::noncopyable in chart2

It will be available in 5.2.0.

The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 30 Commit Notification 2016-04-08 17:45:05 UTC
Jochen Nitschke committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=922ee9a9da62febfe38a7780b11cf0d7ea0d5685

tdf#94306 replace boost::noncopyable in d...

It will be available in 5.2.0.

The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 31 Commit Notification 2016-04-11 07:18:20 UTC
Jochen Nitschke committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=c210bf4510585b554e0e9a371f27fa27e2874762

tdf#94306 replace boost::noncopyable r.. to sdext

It will be available in 5.2.0.

The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 32 Commit Notification 2016-04-11 07:25:24 UTC
Jochen Nitschke committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=d84ef731d8f5d8c1e896ecda3d03d4bb9129578d

tdf#94306 replace boost::noncopyable ...

It will be available in 5.2.0.

The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 33 Commit Notification 2016-04-13 11:22:10 UTC
Jochen Nitschke committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=97abbec95665b43a9a09e10a0fb31854cdbd5c0d

tdf#94306 replace boost::noncopyable in stoc to xmlsec..

It will be available in 5.2.0.

The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 34 Commit Notification 2016-04-13 11:36:52 UTC
Jochen Nitschke committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=0de868cd0f430efc6256926c2865530818d7b7dd

tdf#94306 replace boost::noncopyable in sfx2 to sot

It will be available in 5.2.0.

The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 35 Commit Notification 2016-04-13 19:49:04 UTC
Jochen Nitschke committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=c664b27505223257f93f05425c52a88172ca199b

tdf#94306 remove unused boost dependencies

It will be available in 5.2.0.

The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 36 Ryan McCoskrie 2016-04-18 01:02:39 UTC
I just grep'd the source tree and the only instances of 'noncopyable' are in annotations. I'm pretty sure that this should be closed.