[For discussion purpose] - Used unique_ptr instead of auto_ptr when c++11 is used.

[Bertram25]

Hey @Oberon00 :D

(CC @endoalir for info.)

After some fiddling and some research, I manage to obtain a luabind version capable to compile without any warnings when using c++11, and that seems to run Valyria Tear rather fine, according to my latest tests... You can see the changes I made in this PR.

YET, (because nice things never comes free), it seems those changes aren't perfect according to the following issue: https://github.com/HoverRace/HoverRace/issues/260

It seems the following adapted commit is also needed: https://github.com/halmd-org/luaponte/commit/18537877740c3ca540f11a69160706d6e5891e8d because otherwise the following issue may happen: https://github.com/HoverRace/HoverRace/issues/259

However, it seems HoverRace is using the main luabind repository and that other bugs may come into game, though. So I made this PR mainly to know what to do next in order to get a c++11-warning-free-and-functional luabind PR. Do you think my PR is enough, or do you have hints anywhere at how I should handle the issue around boost::shared_ptr vs std::shared_ptr?

Thanks a lot for your repository in any case and for your help, and best regards,

[fhoefling]

Am 24.08.2015, 00:05 Uhr, schrieb Yohann Ferreira
[email protected]:

Hey @Oberon00 :D

(CC @endoalir for info.)

After some fiddling and some research, I manage to obtain a luabind
version capable to compile without any warnings when using c++11, and
that seems to run Valyria Tear rather fine, according to my latest
tests... You can see the changes I made in this PR.

YET, (because nice things never comes free), it seems those changes
aren't perfect according to the following issue:
https://github.com/HoverRace/HoverRace/issues/260

It seems the following adapted commit is also needed:
https://github.com/halmd-org/luaponte/commit/18537877740c3ca540f11a69160706d6e5891e8d
because otherwise the following issue may happen:
https://github.com/HoverRace/HoverRace/issues/259

In HALMD, we've been using luabind (=luaponte) with C++11 since long
without any issues. Our version is perhaps a bit out-dated since I didn't
have time to follow the recent developments in luabind, but it brings all
the functionality we need (as of luabind 0.9.1) and works with the most
recent Boost releases.

I'd love to revert from luaponte to luabind and to use a library which is
supported by a (small) community. Still I'm not sure which luabind branch
to start with, build-improvements seemed to be the most useful one for me
but it hasn't been continued. I think the most urgent need is a kind of
release schedule and finally a new release of luabind (following the old
0.9.1). Featurewise, the various master branches (from different people)
are in a good shape as far as I can tell. However, just adding commits to
"master" is not enough, it does not tell how stable the current tip of the
branch is. And stability is one of the most important criteria to make a
large project depend on a certain library.

BTW, there is at least one failing test concerning "out_value" vs.
"pure_out_value", see commit a1199b5.

Regards,

Felix

[Bertram25]

Hey @fhoefling :)

In HALMD, we've been using luabind (=luaponte) with C++11 since long without any issues. Our version is perhaps a bit out-dated since I didn't have time to follow the recent developments in luabind, but it brings all the functionality we need (as of luabind 0.9.1) and works with the most recent Boost releases.

It's good to know. The only thing to consider though is that it seems required to keep the former C++ norm supported (or keep std::auto_ptr and don't use nullptr) optionally. Maybe @Oberon00 will be able to tell more about this.

I'd love to revert from luaponte to luabind and to use a library which is supported by a (small) community. Still I'm not sure which luabind branch to start with, build-improvements seemed to be the most useful one for me but it hasn't been continued. I think the most urgent need is a kind of release schedule and finally a new release of luabind (following the old 0.9.1).

Here you're speaking about the mother luabind, which many are considering a dead upstream. The small community you're speaking about is IMHO either around @rpavlik or @Oberon00 's forks. So, I wonder: Do you have access to luabind to start and make it live again, for instance? Also, see below.

Featurewise, the various master branches (from different people) are in a good shape as far as I can tell. However, just adding commits to "master" is not enough, it does not tell how stable the current tip of the branch is. And stability is one of the most important criteria to make a large project depend on a certain library.

IMHO, @Oberon00 has done a good job in trying to keep the lib clean and import mostly bug fixes and taking care of not doing anything too experimental. So, would luabind's authors be away for good, I would rather focus on bringing improvements here. (As I'm trying to do with this current PR).

In your opinion, what features would be lacking here before you would consider adoption, for instance? Could we have a common effort on a alive fork and IMO achieve what everyone wants to achieve: A good common new reference release, even from a fork? (After all, it's the way Open Source work sometimes.)

BTW, there is at least one failing test concerning "out_value" vs. "pure_out_value", see commit a1199b5.

k, created issue: https://github.com/Oberon00/luabind/issues/30

[Oberon00]

Since C++17 will probably remove auto_ptr entirely and MSVC 14 (2015) already allows users to disable it, I consider this pull request a quite reasonable idea.

However, your approach of using #ifdef everywhere is suboptimal: Better use a typedef ??? luabind::detail::unique_ptr + a luabind::detail::move function and define that conditionally in one place (probably a new detail header).

Also do not make this configurable in the CMake file (or if an issue arises that forces the offering of a user configurable "override switch" this should be done via a #cmakedefine in build_information.hpp) but instead rely on Boost.Config's BOOST_NO_CXX11_SMART_PTR. Because older versions of Boost that did not have this macro should also be supported, an additional check for LUABIND_NO_RVALUE_REFERENCES would be good too. See config.hpp and std_shared_ptr_converter.hpp for examples.

When that is done, the -Wno-deprecated-declarations flag can be removed from the CMakeLists.txt.

[…] do you have hints anywhere at how I should handle the issue around boost::shared_ptr vs std::shared_ptr?

Do you mean unique_ptr? Or what issue around shared_ptr do you mean?

[Bertram25]

@Oberon00 I'm overwhelmed by other tasks but I'll come back here to upgrade PR eventually. Thanks for your patience! :)

[akien-mga]

Hey guys, what's the status of this PR?

[Oberon00]

I won't merge the pull request until the many #ifdefs are replaced with a typedef & wrapper move function, as I commented above. Also, because it seems that this pull request is just a cosmetic improvement, I don't plan to allocate time for doing so myself though.

[Bertram25]

I won't merge the pull request until the many #ifdefs are replaced with a typedef & wrapper move function, as I commented above

Yeah, I've got some job left on this one. I didn't forget about it, but there is quite some things on my plate atm, sorry. :)

Also, because it seems that this pull request is just a cosmetic improvement, I don't plan to allocate time for doing so myself though.

Here, I disagree, removing deprecation warnings is no cosmetic thing, especially when the c++14 norm will be the standard.

[Oberon00]

Here, I disagree, removing deprecation warnings is no cosmetic thing, especially when the c++14 norm will be the standard.

Good point, I haven't thought about that. luabind is compiled with -Wno-deprecated, but you are right that forcing user code to also disable this (generally useful) warning is a problem.

[Oberon00]

Hi @Bertram25! It seems this PR comes from your master branch? Do you still intend to merge this? If so, please see the comments above. I think you address some important things here, so I would be glad 😃

[kchang718]

Hi guys, we've been looking at these patches and have applied @Bertram25 's changes along with @Oberon00 's comments to support unique_ptr. These changes are on PR #47. @jameslee5656 and I have looked over these changes to our best abilities and they run ok in our codebase.

We've also did some work to address the operator,() issues in scope.hpp for c++11.

Please take a look when time allows.