Fix name resolution errors

[cheparukhin]

1. Global stream I/O operators were sometimes not found in my code. I didn't have enough patience to detect the exact cause of this and to reproduce the error using a small example, but this is apparently due to some complex ADL rules. I moved the operators to the enum namespace scope as I think it's more appropriate, the lookup is more reliable now. It also allowed to get rid of the hacky "traits" class.
2. Using better_enums as a name for both global and nested namespaces is pretty error-prone as the nested one will be chosen by default, unless ::better_enums is specified explicitly. I concatenated the nested namespace names to produce a unique one and prevent the name clash.

[aantron]

Looks reasonable. Would you be able to provide an example that shows the problem with the stream I/O operators, without too much trouble?

[aantron]

Looking into the Travis build, seems like some bit rot, maybe.

[aantron]

Alright. http://llvm.org/apt/ says:

APT mirror was temporary switched off due to excess load. We are working on bringing it back. Stay tuned!

So don't worry about the Travis builds. They should come back eventually :) It's not a problem – I can test locally on most of those compilers.

[cheparukhin]

I was able to reproduce the error, it happens when:

1. The enum is defined in a namespace (not in a global scope)
2. There is any other operator<< defined in the same namespace

I'm not sure why exactly does it happen, I'm not an expert in these complex ADL rules. But it's definitely more appropriate to define an operator in an argument namespace scope rather than in a global one, which is not always searched apparently.

#define BETTER_ENUMS_STRICT_CONVERSION
#include <enum.h>
#include <iostream>

namespace my_namespace {

struct Foo {};
std::ostream& operator<<(std::ostream& stream, Foo) { return stream; }

BETTER_ENUM(MyBetterEnum, int, FOO, BAR, BAZ)

void printMyBetterEnum() {
    MyBetterEnum mbe = MyBetterEnum::FOO;
    std::cout << mbe << std::endl; // error!
}

}

By the way, this reveals a nasty hidden bug: if you omit the strict conversion macro it will compile, but the standard operator<< taking underlying type will be chosen, producing a non-expected result.

That's why I think that BETTER_ENUMS_STRICT_CONVERSION should be enabled by default. IMO supporting both C++98 and C++11 makes the library a lot more fragile and error-prone. This also applies to operator+, which is very easy to forget and silently receive unexpected results (even with the strict conversion turned on). In this aspect this is a huge step back from C++11 enum class IMO.

I'd rather drop C++98 support to have a safer library with a better API (or separate C++98 and C++11 code paths), but you'll probably disagree :)

[aantron]

Thanks! I'll mess around with the repro case. As mentioned in the other issue, there will probably be a delay on my end, though :/

Regarding C++98, I think what I'll do first is work on C++14. That is the most likely thing to definitively convince me to drop C++98. The second most likely, is that it is now 2016 :p I can always point C++98 users to older versions, and provide bugfix releases when things come up. So, dropping C++98 is not that big a deal. I just want to be really sure of it :)

This library actually started out as C++11-only. The main reason I started supporting C++98 is MSVC. When I made this public, MSVC still had really awful C++11 support. In fact, the last time I checked several months ago, MSVC was still not able to compile Better Enums with all of its C++11 features enabled.

EDIT: the last time was in November, even though issue is from August. Perhaps it's time to check again... :)

[cheparukhin]

@aantron, is this PR LGTM, could you merge it?

[aantron]

Hi @cheparukhin,

Part 1 of the PR LGTM by inspection. I am not sure about part 2. There shouldn't be any better_enums namespaces inside a better_enums namespace. The nested namespaces were called _data_Foo, where Foo is the name of your enum. Either I accidentally nested better_enums::_data_Foo inside better_enums somewhere, resulting in better_enums::better_enums::_data_Foo, or you may have misunderstood the code.

Unfortunately, I can't give the PR the proper, thorough look it deserves, due to being extremely, extremely burned out on open source. So, I've given you commit access to the repo.

I'm sure you've already done these, but: run the tests, try the modified macro in a namespace and outside (the tests also cover that), think about any potential corner cases, and glance in doc/CONTRIBUTING.md for anything I may have forgotten. Then, go ahead and merge. The history looks clean, blame-friendly, and bisectable. If you end up reverting commit 2, please rebase the history so that there is only one commit, instead of adding a third.

If willing, please add a test for this PR, i.e. one or more that would fail without your change.

---

I've been thinking about dropping C++98 support since we discussed it, and I think we can make "strict conversions" the default, as you suggest here, and start moving away from C++98.

The guideline I am using is that the enum.h in master should be immediately compatible with recent versions of GCC, Clang, and MSVC. I don't want MSVC users to be left out by Better Enums, except where this is directly the fault of Microsoft.

Since MSVC has had enum class for a while now, we can switch to that, and drop support for the old-style internal enum completely. After doing that, we can just add a note to the README telling users of older MSVC versions which release of Better Enums they should download.

Switching to enum class by default will probably also make it easier to resolve a few other issues, like those around operator +. I think it will make some things uglier, but there may be ways to work around that.

So, if you want to take that step, you are welcome to do it (or anything that makes Better Enums even better, really :) ). You will have to delete lines from the AppVeyor build matrix for compilers until whatever release supported enum class first (2013? 2015?).

As for full-on C++11-only code in master, I think that will still have to wait thanks to MSVC's constexpr. If you want to know the status of that, do an amend or add a dummy commit in the vc2015-constexpr branch to trigger a build, and see what happens in AppVeyor. You might have to look up the most recent version of MSVC, and check that the AppVeyor image we are using has that.

---

If you do want to change things beyond this PR, and want my opinion on anything, or to get my rationale for design decisions I made, I'm perfectly willing to respond by email. Just trigger it through issues/PR, or send directly :)

[cheparukhin]

@aantron thanks for such a detailed response.

Regarding the second commit, there was a global better_enums namespace and one nested in the user namespace, i.e. user_namespace::better_enums::data_Foo, so when you type better_enums:: inside the user namespace it will probably choose not the namespace you expect, that's why I changed that.

[aantron]

Ah, yes, fair enough. Forgot about that :) Both LGTM then, feel free to merge if you dare :)