Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rewrite the deduction of visitor return type #51

Merged
merged 1 commit into from Dec 23, 2018

Conversation

Projects
None yet
2 participants
@ldionne
Copy link
Member

ldionne commented Nov 15, 2018

This avoids using boost::declval inside evaluated contexts, which is invalid and will actually be diagnosed by compilers when the type used inside boost::declval has internal linkage (such as an anonymous namespace).

Rewrite the deduction of visitor return type
This avoids using boost::declval inside evaluated contexts, which is invalid
and will actually be diagnosed by compilers [1] when the type used inside
boost::declval has internal linkage (such as an anonymous namespace).

[1]: https://bugs.llvm.org/show_bug.cgi?id=35842

@ldionne ldionne requested a review from apolukhin Nov 15, 2018

if (helper == boost::mpl::distance<begin_it, It>::type::value) {
return deduce_impl(next_t(), ++helper);
}
typedef decltype(true ? boost::declval< Visitor& >()( boost::declval< value_t& >() )

This comment has been minimized.

@ldionne

ldionne Nov 16, 2018

Author Member

I interpreted the original code as trying to trigger return type deduction to make sure the visitor always returned the same type for each type in the variant. Is this correct?

If so, this patch will change how deduction is done to figure a common type between the return types of the visitor. It should be possible to change that if that is not desirable.

@@ -28,6 +28,11 @@ test-suite variant
[ run recursive_variant_test.cpp ]
[ run variant_reference_test.cpp ]
[ run variant_comparison_test.cpp ]
[ run variant_visit_internal_linkage.cpp : : :
<toolset>darwin:<cxxflags>-std=c++14

This comment has been minimized.

@ldionne

ldionne Nov 16, 2018

Author Member

I'm terrible with Boost.Build. What I want to achieve here is to always run the test with -std=c++14 if that is enabled, otherwise skip the test (because it will fail).

This comment has been minimized.

@apolukhin

apolukhin Nov 16, 2018

Member

It should be something close to <cxxstd>14, but I'm also terrible with Boost.Build. I'll double check in slack channel.

This comment has been minimized.

@apolukhin

apolukhin Nov 16, 2018

Member
local below-c++14 = 03 11 ;
[ run variant_visit_internal_linkage.cpp : : : <cxxstd>$(below-c++14)/<build>no ] ;
@@ -0,0 +1,33 @@
//-----------------------------------------------------------------------------

This comment has been minimized.

@apolukhin

apolukhin Nov 16, 2018

Member

Add your name here and copyright year

@apolukhin apolukhin merged commit 1c4d882 into develop Dec 23, 2018

1 of 3 checks passed

continuous-integration/travis-ci/push The Travis CI build could not complete due to an error
Details
continuous-integration/travis-ci/pr The Travis CI build failed
Details
coverage/coveralls First build on ldionne-visit_deduce at 95.063%
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.