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

Fixes for clang in MSVC mode #24

Merged
merged 1 commit into from Mar 18, 2016

Conversation

Projects
None yet
3 participants
@MarcelRaad
Copy link
Contributor

commented Jan 13, 2016

In MSVC mode, clang doesn't define __GNUC__.
Also, it doesn't define __clang, but defines __clang__.

@jzmaddock

This comment has been minimized.

Copy link
Collaborator

commented Jan 13, 2016

I'm happy to apply that, but just curious, doesn't the msvc-preprocessor path work in this case?

@MarcelRaad

This comment has been minimized.

Copy link
Contributor Author

commented Jan 13, 2016

You mean using the BOOST_MSVC path in has_nothrow_assign.hpp? Yes, that would also work, but only is_volatile.hpp and is_assignable.hpp are required for clang, and using the GCC path is consistent with non-MSVC clang. The code in intrinsics.hpp is independent of MSVC or GCC mode.
Or do you mean if BOOST_MSVC is defined for clang in MSVC mode? No, BOOST_CLANG is defined.

@MarcelRaad MarcelRaad force-pushed the MarcelRaad:fix_clang_msvc branch from bc6cde6 to 251bf79 Jan 21, 2016

Fixes for clang in MSVC mode
In MSVC mode, clang doesn't define __GNUC__.
Also, it doesn't define __clang, but defines __clang__.

@MarcelRaad MarcelRaad force-pushed the MarcelRaad:fix_clang_msvc branch from 251bf79 to 71c6fea Feb 12, 2016

@awulkiew

This comment has been minimized.

Copy link
Member

commented Mar 16, 2016

I created a similar PR without looking at the list of PRs, sorry about that.

FYI, on MSC+Clang _MSC_VER, __clang__ and BOOST_CLANG are defined (of course the last one when Config is included). BOOST_MSVC is not defined. So instead of __clang or __clang__ simply BOOST_CLANG could be checked since Boost.Config is included anyway.

I think that in every place where __GNUC__ is checked, BOOST_CLANG should be checked as well. Maybe Config could define some macro for GNUC-compatible compilers?

TL;DR

If intrinsics.hpp BOOST_CLANG is checked and the macros for Clang are enabled (see the errors below), macros for MSVC are disabled. So let's say has_nothrow_constructor.hpp is compiled. Macro BOOST_HAS_NOTHROW_CONSTRUCTOR() is expanded so the compiler tries to instantiate is_default_constructible<>. Since neither BOOST_MSVC nor __GNUC__ are defined neither has_trivial_constructor.hpp nor is_default_constructible.hpp are included, AFAIU the latter is required. The result is identifier not found.

E.g. when I try to compile some code using Boost.Geometry I get errors for following traits:

has_nothrow_constructor.hpp(26,84): error : no template named 'is_default_constructible'; did you mean 'std::is_default_constructible'?
template <class T> struct has_nothrow_constructor : public integral_constant<bool, BOOST_HAS_NOTHROW_CONSTRUCTOR(T)>{};
intrinsics.hpp(193,80) :  note: expanded from macro 'BOOST_HAS_NOTHROW_CONSTRUCTOR'
#     define BOOST_HAS_NOTHROW_CONSTRUCTOR(T) (__has_nothrow_constructor(T) && is_default_constructible<T>::value)

has_nothrow_assign.hpp(64,7): error : no template named 'is_assignable'; did you mean 'std::is_assignable'?
BOOST_HAS_NOTHROW_ASSIGN(T)
intrinsics.hpp(199,96) :  note: expanded from macro 'BOOST_HAS_NOTHROW_ASSIGN'
#     define BOOST_HAS_NOTHROW_ASSIGN(T) (__has_nothrow_assign(T) && !is_volatile<T>::value && is_assignable<T&, const T&>::value)

has_trivial_assign.hpp(28,7): error : no template named 'is_assignable'; did you mean 'std::is_assignable'?
BOOST_HAS_TRIVIAL_ASSIGN(T)
intrinsics.hpp(187,96) :  note: expanded from macro 'BOOST_HAS_TRIVIAL_ASSIGN'
#     define BOOST_HAS_TRIVIAL_ASSIGN(T) (__has_trivial_assign(T) && !is_volatile<T>::value && is_assignable<T&, const T&>::value)

My PR contains only fixes for 2 first traits because after I fixed has_nothrow_assign.hpp is_assignable.hpp was included and therefore a fix in has_trivial_assign.hpp was not needed. So this PR is more complete.

@jzmaddock

This comment has been minimized.

Copy link
Collaborator

commented Mar 18, 2016

I've managed to get clang more or less working on Windows, so I'll check this out later.

jzmaddock added a commit that referenced this pull request Mar 18, 2016

@jzmaddock jzmaddock merged commit 38cbf9d into boostorg:develop Mar 18, 2016

@jzmaddock

This comment has been minimized.

Copy link
Collaborator

commented Mar 18, 2016

Confirmed working OK, many thanks!

@MarcelRaad MarcelRaad deleted the MarcelRaad:fix_clang_msvc branch Mar 18, 2016

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.