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

Constexpr conversion to MPL types #134

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

Kojoley
Copy link
Contributor

@Kojoley Kojoley commented Nov 30, 2019

Fixes #133

GCC, Clang, MSVC, ICC, all of them are fine with this https://godbolt.org/z/6K6NDm

@Kojoley Kojoley force-pushed the constexpr-conversion-to-mpl-types branch from 43e9ab4 to 7c23e86 Compare January 7, 2020 23:15
test/mpl_interop_test1.cpp Outdated Show resolved Hide resolved
@Kojoley Kojoley force-pushed the constexpr-conversion-to-mpl-types branch from 7c23e86 to 17d9fd9 Compare January 8, 2020 14:17
@Kojoley Kojoley force-pushed the constexpr-conversion-to-mpl-types branch from 17d9fd9 to edf42d5 Compare January 8, 2020 15:37
@jzmaddock
Copy link
Collaborator

As noted in #133 I'm rather inclined to treat this as a non-bug, or at least one that can't be fixed without breaking stuff.

The problem with this fix is it means that code that relies on this conversion, and on mpl::bool_ being at least forward declared after including a type_traits header will be broken (and will need to be modified to include the mpl headers explicitly, as you did for the test case, which quite deliberately did not do that).

I realise they should be doing that already, but if we're forcing folks to change their code, it would be better to mark this interface as deprecated, and suggest they dispatch on integral_constant instead.

@Kojoley
Copy link
Contributor Author

Kojoley commented Jan 8, 2020

Ok, I just will create a new duplicate test that and will not touch this to show that nothing should be broken by this change.

@Kojoley
Copy link
Contributor Author

Kojoley commented Jan 8, 2020

Stop, why we should support the code that uses mpl types and not including mpl? Is not it a bug in their code?

@pdimov
Copy link
Member

pdimov commented Jan 8, 2020

It is, in my opinion, a bug in their code; and the answer to your question is probably "because code in unmaintained libraries will break and we'll need to fix it." :-)

@Kojoley
Copy link
Contributor Author

Kojoley commented Jan 8, 2020

If the only concerns are about Boosts code, I volunteer myself to identify and fix them.

@Kojoley
Copy link
Contributor Author

Kojoley commented Jan 9, 2020

I made a list of libraries with grep -ER "bool_\s*<\s*(true|false)\s*>" libs | cut -d / -f 2 | sort | uniq, and run their tests with the PR applied https://travis-ci.com/Kojoley/type_traits_experiment/builds/143657865 and they are fine, except timeouted multiprecision and not related issues in hof/statechart.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

non-constexpr integral_constant conversions
3 participants