-
Notifications
You must be signed in to change notification settings - Fork 779
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
Get rid of boost::assign #1375
Get rid of boost::assign #1375
Conversation
Hmmm, don't understand why CI keeps failing. Seems to be a Github issue?. |
@kartikarcot FYI |
@dellaert maybe a missing include in |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Aside from some small things, I just want to clarify that in some places the changes are just too tiresome so you left them be, e.g. the instances of ListOf
, and that there's not some other reasoning.
Otherwise, this is exciting that lots of places look much cleaner and more c++'ish, e.g. all the Ordering o{1, 2, 3}
type things :)
You probably already knew these, but sharing in case it helps: I think tbb failure (ubuntu special cases) is because overload needs to be defined with the correct allocator. i.e. only the overload is defined operator std::vector<T>() { return result; } // old
operator std::vector<T, internal::FastDefaultAllocator::type>() { return result; } // new should fix it. Or there may be some additional copying logic needed if there isn't an implicit conversion between vectors of different allocators. For windows, I'm not smart enough to figure out, but at least I'll try to share what I can: I gathered that it's complaining that |
… with initializer-list constructor.
One more TBB issue which I’ll fix. But, @ProfFan , I’m wondering why we install LLVM in a gcc CI run? |
I think it's just because it's in the common part of CI scripts |
Appears that GitHub is having a bad time |
Great move, and great PR! :-) Is |
Thanks !The goal is to get rid of boost in 4.3 altogether. |
Hell, yeah. |
We used boost::assign heavily before c++11. In 4.3 we want to make boost optional (no pun intended) and hence getting rid of this old cruft is good prep.
Massive PR, with help of copilot and some ChatGPT advice. Most difficult was the use of
list_of
in the Symbolic unit tests. I created "chaining" constructor/operators in SymbolicBayesNet and SymbolicFactorGraph to ease the pain, and perhaps this would also be nice in BayesTree, but perfection is the enemy of the good...