Bug Hunting Session
Bug 93240 - replace boost::ptr_container with std::container<std::unique_ptr>
Summary: replace boost::ptr_container with std::container<std::unique_ptr>
Status: RESOLVED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: LibreOffice (show other bugs)
Version:
(earliest affected)
5.1.0.0.alpha0+ Master
Hardware: Other All
: medium normal
Assignee: Not Assigned
URL:
Whiteboard: target:5.2.0
Keywords: difficultyBeginner, easyHack, skillCpp, topicCleanup
Depends on:
Blocks:
 
Reported: 2015-08-07 13:54 UTC by Michael Stahl (CIB)
Modified: 2016-10-25 19:09 UTC (History)
3 users (show)

See Also:
Crash report or crash signature:


Attachments
replace boost::ptr_vector with std::vector<std::unique_ptr> (96 bytes, patch)
2015-08-27 00:48 UTC, derrick rocha
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Michael Stahl (CIB) 2015-08-07 13:54:47 UTC
we use a lot of boost::ptr_container currently:

  git grep ptr_container | wc -l
  286

with C++11 it is now possible to use std::unique_ptr with the C++
standard library containers, and this makes most use of
boost::ptr_container obsolete.

the bundled boost headers require a lot of patches to suppress warnings
so it would reduce the maintenance burden here to use standard C++11
containers instead, and it would also make incremental rebuilds faster
whenever boost is modified/patched.

also with GCC 5's libstdc++ somehow loads of auto_ptr deprecation
warnings are being generated (and ignored) via the boost headers, and
that problem would also be substantially reduced by converting these.

for examples on how to do this search the git log for "boost::ptr_"

in some cases it is even easier to replace the ptr_container with a
std container that doesn't use unique_ptr but holds the elements
by value - a lot of these containers were converted from awkward
legacy tools containers that forced manual memory allocations everywhere.
Comment 1 derrick rocha 2015-08-20 01:52:19 UTC
I am quite interested in investigating this bug. Can you please fill me in on where to start? This is my first time doing open source contribution, and I am a senior of Computer Science at New Mexico Highlands University.

Thanks!
Derrick Rocha
Comment 2 Ryan McCoskrie 2015-08-20 05:03:26 UTC
(In reply to derrick rocha from comment #1)
> I am quite interested in investigating this bug. Can you please fill me in
> on where to start? This is my first time doing open source contribution, and
> I am a senior of Computer Science at New Mexico Highlands University.
> 
> Thanks!
> Derrick Rocha

First make sure that you are set up for using gerrit. Check out the LO wiki on that matter.

After that (assuming you run a *nix system) pick a directory you want to work with and run 'grep -rn ptr_container dirname'. From there it is a simple matter of editing declarations and function calls a little.
Comment 3 Matthew Francis 2015-08-20 05:18:40 UTC
(In reply to Ryan McCoskrie from comment #2)
> After that (assuming you run a *nix system) pick a directory you want to
> work with and run 'grep -rn ptr_container dirname'. From there it is a
> simple matter of editing declarations and function calls a little.

Better to use "git grep" from within the checked-out source directory rather than plain grep - it will be faster, and will skip build artifacts and other irrelevant files
Comment 4 Commit Notification 2015-08-20 12:19:28 UTC
Michael Stahl committed a patch related to this issue.
It has been pushed to "master":

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

tdf#93240: replace boost::ptr_vector with std::vector<std::unique_ptr>

It will be available in 5.1.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 5 derrick rocha 2015-08-20 14:16:41 UTC
(In reply to Matthew Francis from comment #3)
> (In reply to Ryan McCoskrie from comment #2)
> > After that (assuming you run a *nix system) pick a directory you want to
> > work with and run 'grep -rn ptr_container dirname'. From there it is a
> > simple matter of editing declarations and function calls a little.
> 
> Better to use "git grep" from within the checked-out source directory rather
> than plain grep - it will be faster, and will skip build artifacts and other
> irrelevant files

Ok. I'm glad to see I was actually on the right track. Ill try to get a patch submitted this afternoon since I have court this morning :(
Comment 6 derrick rocha 2015-08-21 19:09:59 UTC
Another question. How do I build using C++11? I keep getting errors saying ‘unique_ptr’ is not a member of ‘std’. Thanks
Comment 7 Commit Notification 2015-08-21 23:03:12 UTC
Michael Stahl committed a patch related to this issue.
It has been pushed to "master":

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

tdf#93240: replace boost::ptr_map with std::map<std::unique_ptr>

It will be available in 5.1.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 8 derrick rocha 2015-08-22 02:48:15 UTC
Never mind. I figured it out. Thank you.
Comment 9 derrick rocha 2015-08-25 00:47:37 UTC
anybody know why I keep getting a build failure message after I submit a patch to gerrit? It compiled fine for me before i submitted it.
Comment 10 derrick rocha 2015-08-27 00:48:08 UTC
Created attachment 118209 [details]
replace boost::ptr_vector with std::vector<std::unique_ptr>
Comment 11 Commit Notification 2015-09-11 12:48:41 UTC
Matthew Nicholls committed a patch related to this issue.
It has been pushed to "master":

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

tdf#93240: replace boost::ptr_deque with std::deque<std::unique_ptr>

It will be available in 5.1.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 12 Commit Notification 2015-09-29 11:06:40 UTC
Takeshi Abe committed a patch related to this issue.
It has been pushed to "master":

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

starmath: tdf#93240 replace boost::ptr_vector

It will be available in 5.1.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 13 kerem 2015-10-11 15:11:06 UTC
I sent following patch for this bug;

https://gerrit.libreoffice.org/#/c/19303/
Comment 14 Robinson Tryon (qubit) 2015-12-13 11:04:07 UTC Comment hidden (obsolete)
Comment 15 Michael Stahl (CIB) 2016-02-11 14:07:00 UTC
done, on master there are no more boost::ptr_container
Comment 16 Robinson Tryon (qubit) 2016-02-18 16:37:33 UTC Comment hidden (obsolete)