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.

Sign up for GitHub

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

Jump to bottom

Fix compilation on lastest MSVC #7

Merged
merged 1 commit into from
Oct 25, 2017
Merged

Fix compilation on lastest MSVC #7

merged 1 commit into from
Oct 25, 2017

Conversation

BlowaXD
Copy link
Contributor

@BlowaXD BlowaXD commented Sep 27, 2017

std::unary_function has been removed from C++ 17 standard so it does not compile anymore on MSVC

Setup :
Lastest MSVC
C++ 2017

Thanks to Mylerius for the help.

@BlowaXD
Fix compilation on lastest MSVC
4505bc3 
std::unary_function has been removed from C++ 17 standard so it does not compile anymore on MSVC
@BlowaXD BlowaXD mentioned this pull request Sep 27, 2017
Closed
@fcacciola
Copy link
Collaborator

I haven't worked on Boost for over a decade now (this one piece of code is 17 years old :)
And I don't have write access now, so I cannot merge this pull-request.

@brandon-kohn can you do that?

or alternatively:

@Beman can I get write-acesss? and where do I learn about current procedures so I don't mess things up.

TIA

@pdimov
Copy link
Member

pdimov commented Oct 25, 2017

Welcome back, @fcacciola. Enjoy your write access. :-)

@fcacciola
Copy link
Collaborator

Thank you Peter!

@fcacciola fcacciola merged commit 54d571d into boostorg:develop Oct 25, 2017
@japj
Copy link

japj commented Dec 1, 2017

Hello,

Is there any information when this will land in master (for a future release?)

@marcosatti marcosatti mentioned this pull request Jan 11, 2018
Closed
@fcacciola
Copy link
Collaborator

fcacciola commented Feb 27, 2018 via email

@fcacciola
Copy link
Collaborator

Maybe I don't have to do anything myself manually? The fix is already in "boostorg:develop".

@MarcelRaad
Copy link

@fcacciola Thanks! You already merged this into develop some months ago. Now it needs to be merged from develop to master so that it appears in a release.

@pdimov
Copy link
Member

pdimov commented Feb 28, 2018

A deadline for changes to master has passed though so it might be best to ask @danieljames to take a look first.

@danieljames
Copy link
Contributor

The deadline was major changes, this isn't a major change, so it's still possible. But I'm not sure about this pull request - it really shouldn't be relying on anything from functional::detail. It looks like the use of std::unary_traits was removed in ebfded1 so this might not be needed anyway.

@fcacciola
Copy link
Collaborator

fcacciola commented Feb 28, 2018 via email

@danieljames
Copy link
Contributor

OK, then it's not clear why this was failing, it'd be helpful to see an error message or how to replicate the error. The original report said it was due to the removal of std::unary_function from Visual C++, but that can't be the case after it stopped using that. Maybe the error is because the typedefs from std::unary_function are used somewhere? In which case, the consensus was that the best solution is to add the typedefs to the structs rather than relying on inheritance.

@StefanAtev
Copy link

StefanAtev commented Mar 5, 2018 •
edited
Loading

The issue depends on whether C++17 mode is enabled for MSVC; The default for MSVC is /std:c++14, in which case unary_function is available and everything compiles. However, with /std:c++17 or /std:c++latest, MSVC no longer provides unary/binary function. Perhaps the confusion of whether things are removed or not is due to different uses of the /std switch. Right now, users have to choose between using some boost libraries like lexical_cast and special math, or C++17 features. A fix for /std:c++17 mode would be greatly appreciated.

@pdimov
Copy link
Member

pdimov commented Mar 5, 2018

Maybe the error is because the typedefs from std::unary_function are used somewhere?

What makes this even more interesting is that I can see in the diff that the typedefs are already present at least in trivial_converter_impl.

@pdimov
Copy link
Member

pdimov commented Mar 5, 2018

A fix for /std:c++17 mode would be greatly appreciated.

A fix for what specific issue with /std:c++17? What do you compile, what errors do you get? The tests on master already pass with either 14 or 17.

@pdimov
Copy link
Member

pdimov commented Mar 5, 2018

However, the latest change was on top of Brandon's fix, so I take it to mean it didn't really work on VC 2017?

Then we need an example that demonstrates why it didn't really work.

@StefanAtev
Copy link

I must be clear - I am reporting the issue as it appears in 1.65.1; If it is already fixed when 1.66 is out, that would be great. With 1.65.1, simply using ibeta from <boost/math/special_functions/beta.hpp> or just including <boost/lexical_cast.hpp> is enough to break with /std:c++17 (using the latest toolchain supported by boost as of 1.65.1, which is MSVC 14.11). The initial report was not clear whether the /std switch was used, and if so, at what setting. It was also not clear exactly what MSVC version was used - 14.10, 14.11, or 14.12.

@danieljames
Copy link
Contributor

1.66.0 was released in December.

@StefanAtev
Copy link

Sorry - my bad - I was writing from memory, we are using 1.66 and waiting for the April release that will support MSVC 14.12

@pdimov
Copy link
Member

pdimov commented Mar 5, 2018

The tests for 1.66 pass with /std:c++17 for me. lexical_cast too.

commit ebfded1d7d4a2f773e8233c4df6f96131f79d72e (HEAD -> master, tag: boost-1.66.0, origin/master)
Author: Brandon Kohn <blkohn@hotmail.com>
Date:   Tue Aug 22 13:39:34 2017 -0400

    Remove the deprecated uses of std::unary_function (again).

@StefanAtev
Copy link

OK - scratch all of that, I must have coffee. The build I am looking at is using 1.65.1; It did not switch to 1.66 because 1.66 did not add MSVC 14.12 support so it did appear to be an useless upgrade; now that we are turning on /std:c++17, it seems that it does bring in other benefits. I apologize for the confusion - and appreciate the quick response.

@fcacciola
Copy link
Collaborator

fcacciola commented Mar 5, 2018 via email

@danieljames
Copy link
Contributor

That's good for me. Thanks.

@fcacciola fcacciola mentioned this pull request Mar 6, 2018
Merged
@fcacciola
Copy link
Collaborator

fcacciola commented Mar 6, 2018 via email

@danieljames
Copy link
Contributor

The 'Create Pull Request' button isn't enabled until you enter a title. Although I'd just use command line git to do the merge.

As far as I can tell, the changes in develop are:

  • Add a README.md for github.
  • Remove workarounds for Visual C++ 6 and 7.
  • Add a meta/libraries.json file.

None of these are really needed for a release, but you can merge them if you want.

@fcacciola
Copy link
Collaborator

fcacciola commented Mar 7, 2018 via email

@danieljames
Copy link
Contributor

You can. but I'll be stopping automatic updates of the super project's master branch later, so if you haven't accepted it, I'll do it so that it's included in the beta.

@fcacciola
Copy link
Collaborator

fcacciola commented Mar 7, 2018 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
7 participants
@BlowaXD @fcacciola @pdimov @japj @MarcelRaad @danieljames @StefanAtev