ptal / expected

What did you expect?
113 stars 18 forks source link

Removed Boost as a dependency from Expected, so Expected can now function standalone with a very minimal boost macros header file #71

Open ned14 opened 9 years ago

ned14 commented 9 years ago

In the process tidied up quite a bit of historical cruft. Verified as compiling and passing all unit tests on GCC 4.8 and VS2013.

If you can let me remove Boost.Move support, I can simplify things still further e.g. strip out BOOST_FWD_REF et al.

ned14 commented 9 years ago

In case you're curious, the only file now needed by Expected is this https://github.com/ned14/local-bind-cpp-library/blob/master/include/boost/config.hpp

ned14 commented 9 years ago

No, that's fixed in the pull request

viboes commented 9 years ago

Niall, I recognize that the current use of Boost macros is not very useful as the code works only for C++11/C++14 compilers. However , at least initially, we wanted to port it to C++03 compilers (I don't know if we have the needed energy). What do you think of creating a std::experimental branch that doesn't make use of Boost at all?

have you tested with BOOST_CONFIG_HPP defined?

ned14 commented 9 years ago

I actually have little objection to the macros in https://github.com/ned14/local-bind-cpp-library/blob/master/include/boost/config.hpp - something like BOOST_NOEXCEPT might as well be called BOOST_NOEXCEPT instead of something like EXPECTED_NOEXCEPT. I can't really think of a better name for those macros. And besides, they are easy.

Regarding an 03 port ... well, I doubt it will be me who does it. It would be an enormous undertaking. Lots of little things break all over, and they are tedious to fix. I don't mind leaving in Boost.Move support for now though, my config.hpp emulation works well enough for me to move ahead with no-alloc future-promise.

Regarding testing with BOOST_CONFIG_HPP, yes that is what I tested. I cannot run unit tests with it not defined as the unit tests need Boost. I therefore, strictly speaking, cannot confirm yet it works without BOOST_CONFIG_HPP defined. No-alloc future-promise should say something about that though.

viboes commented 9 years ago

I agree depending only on boost/config.hpp even for C++11/C++14 compilers is acceptable.

Don't worry for the C++03 porting.

I think that you should change BOOST_CONFIG_HPP by BOOST_EXPECTED_USES_BOOST_HPP. In this way you will be able to check with a without defining BOOST_EXPECTED_USES_BOOST_HPP.

You don't answered my question about a std::experimental brach?

ned14 commented 9 years ago

Just to clear, I am referring to https://github.com/ned14/local-bind-cpp-library/blob/master/include/boost/config.hpp not boost/config.hpp. The former is about as minimal a Boost emulation header as could be written.

The #ifdef BOOST_CONFIG_HPP is for detecting if Expected is being compiled with Boost, and it therefore turns on Boost support. Why BOOST_CONFIG_HPP? Well, that is absolutely guaranteed to be defined if Boost was included. I suppose so should BOOST_VERSION, I could use that instead?

Regarding a std::experimental branch .... I don't think we need a whole separate branch for that. A simple #include which either declares:

  1. namespace boost { inline namespace expected { inline namespace v1 {
  2. namespace std { namespace experimental { inline namespace expected { inline namespace v1 {

... depending on whether BOOST_EXPECTED_USE_BOOST_NAMESPACE or BOOST_EXPECTED_USE_STD_NAMESPACE is defined.

How does this sound?

viboes commented 9 years ago

Oh, I missed that you are suggesting to use your https://github.com/ned14/local-bind-cpp-library/blob/master/include/boost/config.hpp. I don't like the idea at all. I prefer to have a Boost agnostic version using macros as EXPECTED_CONSTEXPR, .... so there is no confusion.

The std::experimental branch wouldn't include just the namespace. It wouldn't use Boost at all, while the other branch (master or another) will be used for the porting to C++03 (which will have less functionalities than the C++11 version).

ned14 commented 9 years ago

Sorry, I didn't explain well.

If you compile expected with -Ilocal-bind-cpp-library/include, then it will find the fake boost/config.hpp and Boost support is not enabled. If you compile expected as part of Boost, the real boost/config.hpp is found and Boost support is enabled.

If we did do a std::experimental version, we could skip the fake boost/config.hpp and define the Boost macros internally.

viboes commented 9 years ago

Lets go for removing the Boost.Move support, as we are unable now to compile for C++98 compilers. Could you take care of this?

ned14 commented 9 years ago

Sure. I have come up with a technique which lets external code choose which namespace a library is defined in, and combined with the local namespace bindings technique which lets external code also choose which dependencies a library should use, this opens the door for an Expected implementation capable of living either in boost or std::experimental, and can be dependent on Boost or the C++ STL. All these coexist, so one could have four implementations of the same Expected all at the same time. Anyway, give me a week or two, then I'll present the technique to boost-dev for feedback.