Skip to content
This repository was archived by the owner on Apr 6, 2021. It is now read-only.

Conversation

@ned14
Copy link

@ned14 ned14 commented Oct 7, 2014

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.

Niall Douglas (s [underscore] sourceforge {at} nedprod [dot] com) added 3 commits October 7, 2014 17:32
@ned14
Copy link
Author

ned14 commented Oct 7, 2014

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
Copy link
Author

ned14 commented Oct 7, 2014

No, that's fixed in the pull request

@viboes
Copy link
Collaborator

viboes commented Oct 7, 2014

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
Copy link
Author

ned14 commented Oct 7, 2014

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
Copy link
Collaborator

viboes commented Oct 7, 2014

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
Copy link
Author

ned14 commented Oct 7, 2014

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
Copy link
Collaborator

viboes commented Oct 7, 2014

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
Copy link
Author

ned14 commented Oct 7, 2014

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
Copy link
Collaborator

viboes commented Oct 9, 2014

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
Copy link
Author

ned14 commented Oct 9, 2014

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.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants