Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Consider eliminating FCL's dependence on boost #100

Closed
sherm1 opened this issue Mar 23, 2016 · 10 comments
Closed

Consider eliminating FCL's dependence on boost #100

sherm1 opened this issue Mar 23, 2016 · 10 comments
Milestone

Comments

@sherm1
Copy link
Member

sherm1 commented Mar 23, 2016

For discussion -- boost is a very heavyweight dependency and could be an impediment to FCL adoption. It can also cause problems when FCL is integrated into a larger program that already has a boost dependency, possibly involving a different version of boost (Matlab comes with a version linked in, which caused Drake to have to drop boost).

I'm wondering if the need for boost could be eliminated in favor of C++11 use instead? Are there boost features in use that C++11 can't provide?

@mamoll
Copy link
Member

mamoll commented Mar 23, 2016

I'd be in favor of that. I just started the transition to C++11 in OMPL (https://github.com/ompl/ompl/tree/c++11). After grep-ing through FCL's code, it looks fairly straightforward.

One thing that took me a while to figure out was that C++03 and C++11 code are not ABI compatible (usually things just work, though). The FCL library I had installed was compiled linked against Boost and both were built on C++03 mode. After compiling OMPL in C++11 code I got some odd crashes, possibly related to some constexpr's in Boost. So eliminating Boost as a dependency and forcing C++11 could potentially make it easier to adopt FCL.

@scpeters
Copy link
Contributor

Requiring c++11 is a good way to remove many uses of boost, which I have been spearheading for urdfdom and some other repositories. Here's a list of boost header files included in the FCL master branch:

$ grep -rnIi include.*boost . | sed -e 's@.*:#include@@' | tr -d '\r' | sort | uniq
 <boost/array.hpp>
 <boost/assert.hpp>
 <boost/assign/list_of.hpp>
 <boost/bind.hpp>
 <boost/cstdint.hpp>
 <boost/date_time/posix_time/posix_time.hpp>
 <boost/dynamic_bitset.hpp>
 <boost/filesystem.hpp>
 <boost/foreach.hpp>
 <boost/function.hpp>
 <boost/iterator/zip_iterator.hpp>
 <boost/math/constants/constants.hpp>
 <boost/math/special_functions/erf.hpp>
 <boost/math/special_functions/fpclassify.hpp>
 <boost/noncopyable.hpp>
 <boost/random/lagged_fibonacci.hpp>
 <boost/random/mersenne_twister.hpp>
 <boost/random/normal_distribution.hpp>
 <boost/random/uniform_int.hpp>
 <boost/random/uniform_real.hpp>
 <boost/random/variate_generator.hpp>
 <boost/shared_array.hpp>
 <boost/shared_ptr.hpp>
 <boost/test/unit_test.hpp>
 <boost/thread.hpp>
 <boost/thread/mutex.hpp>
 <boost/timer.hpp>
 <boost/unordered_map.hpp>
 <boost/unordered_set.hpp>
 <boost/weak_ptr.hpp>

One that jumps out at me is boost filesystem, which isn't available in c++11, though it may be included in some later standards.

@mamoll
Copy link
Member

mamoll commented Mar 23, 2016

boost filesystem is only used in the unit tests. The boost unit testing framework would also be a problem. Perhaps limiting boost to unit tests is acceptable (since end users wouldn't have to install boost to use fcl)?

@jslee02
Copy link
Member

jslee02 commented Mar 23, 2016

It seems filesystem will be included in C++17. Also, zip_iterator actually isn't used anywhere.

@isucan
Copy link
Contributor

isucan commented Mar 23, 2016

I am also fully in favor of dropping the boost dependency. I think FCL's
use of is is rather light, so it should be quite doable.
On Mar 22, 2016 6:07 PM, "Jeongseok Lee" notifications@github.com wrote:

It seems filesystem will be included in C++17
https://en.wikipedia.org/wiki/C%2B%2B17. Also, zip_iterator actually
isn't used nowhere.


You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub
#100 (comment)

@scpeters
Copy link
Contributor

@sherm1 we could make a checklist based on that list of header files that I made and then check them off as they are removed

@sherm1
Copy link
Member Author

sherm1 commented Mar 23, 2016

we could make a checklist based on that list of header files that I made and then check them off as they are removed

Great idea, Steve! Then we could do small PRs that knock them out one at a time and check them off until the last one is gone.

@sherm1
Copy link
Member Author

sherm1 commented Mar 23, 2016

@mamoll: One thing that took me a while to figure out was that C++03 and C++11 code are not ABI compatible

Yeah, there is no going back. You have to commit to C++11 and then everyone who links with you does also. But ... this is now a 5 year old standard so it doesn't seem too soon to switch!

@sherm1 sherm1 mentioned this issue Mar 23, 2016
30 tasks
@sherm1
Copy link
Member Author

sherm1 commented Mar 23, 2016

I made issue #102 with Steve's boost checklist in it.

@jslee02
Copy link
Member

jslee02 commented Jul 24, 2016

All the boost dependencies are removed by #103, #104, #105, #108, #140, #146, #147, and #148!

Closing this issue.

@jslee02 jslee02 closed this as completed Jul 24, 2016
@jslee02 jslee02 added this to the FCL 0.6.0 milestone Jul 24, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants