Print C++17 types (any, optional and variant) and absl::any

[kuzkry]

Rework of rejected #2438 due to the plan on taking hard dependency on Abseil. I'm resubmitting because recent commit 6f5fd0d seems to suggest abandonment of Abseil incorporation like it was planned half a year ago.

[o-micron]

@kuzkry Ok, after digging a little in the code base here is the summary of what I understood so far ..

• Bazel builds are just fine, only cmake on [msvc 15 2017 Win64, msvc 14 2015 Win64] are failing to build so far and the error is at project gmock-matchers_test in file gmock-matchers.h:434
• we replaced #if GTEST_HAS_ABSL by #if GTEST_INTERNAL_HAS_ANY since we now also defined that and internal::StringView is used as ::std::string_view in case of no ABSL ..
• I kept jumping around the different templates and I don't know at this point where the missing part is, what I don't understand is that it compiles on all platforms and even on different msvc versions so I don't know if there is something in these templates that needs to be tweaked or the problem comes from missing cmake params ... since basel builds are just fine .. same code :D
• Does these template errors have something to do with string_view and explicit conversion ?

[kuzkry]

Hey @o-micron.

First things first, #if GTEST_INTERNAL_HAS_ANY has nothing to do with internal::StringView. The latter is guarded by #if GTEST_INTERNAL_HAS_STRING_VIEW ;)

To reply all your other points: I didn't investigate it but you don't need to care about these two failing AppVeyor builds as master is unstable now. I don't know what's the reason but I can tell it's been failing since commit fd53816 so either it's that commit fault or AppVeyor VMs were externally modified.

[o-micron]

Yeah #if GTEST_INTERNAL_HAS_STRING_VIEW not #if GTEST_INTERNAL_HAS_ANY 🤓
So should I just squash the commits and just leave the 2 appveyor builds failing at the moment ?

[kuzkry]

In case of your PR #2736, yes :) Just do your work there and ignore these 2 failing jobs for now. If you wanna fix the failing master, you could create another PR by branching out from master.

[mattcalabrese-google]

Sorry for our delays in getting to this, we are looking at it again now. One quick suggestion upon skimming -- the variant output should likely also stream the index of the alternative that is mentioned (either in addition to or in place of the type info). This is because a variant can have multiple alternatives with the same type, so the type and value alone is not necessarily enough to determine which alternative was actually active.

[zhangxy988]

In general the change LGTM. Thanks!

[zhangxy988]

Sorry for our delays in getting to this, we are looking at it again now. One quick suggestion upon skimming -- the variant output should likely also stream the index of the alternative that is mentioned (either in addition to or in place of the type info). This is because a variant can have multiple alternatives with the same type, so the type and value alone is not necessarily enough to determine which alternative was actually active.

I think this is a good suggestion for improvement but doesn't need to be part of this change. This change aims at supporting users who uses C++17 std types without the dependency on Abseil.

I would even suggest separating the (std|absl)::any support into a separate PR but that is probably going too far.

[kuzkry]

@mattcalabrese-google, thanks, done :)
@zhangxy988, thanks too. I didn't want to implement it all as it doesn't match the existing style, though I agree with you. I can create another small PR and have it all done there if you want.

[mattcalabrese-google]

LGTM. I think the <index> way of including the alternative index may end up being confusing since it makes it look like a template being instantiated with an integral argument, but if we decide that's a problem we can change it in a different PR.

[zhangxy988]

LGTM. I think the <index> way of including the alternative index may end up being confusing since it makes it look like a template being instantiated with an integral argument, but if we decide that's a problem we can change it in a different PR.

I agree "int<0>" is a bit confusing. How about "int(0)" or "int@0"?

[kuzkry]

LGTM. I think the <index> way of including the alternative index may end up being confusing since it makes it look like a template being instantiated with an integral argument, but if we decide that's a problem we can change it in a different PR.

I agree "int<0>" is a bit confusing. How about "int(0)" or "int@0"?

Sure, I've chosen int(0).

[kuzkry]

I've lowered the amount of commits so it's neat and concise now.

[zhangxy988]

Thank you, we have started internal review. Please don’t
push any more changes into this PR as they might be overwritten. (314204435)