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

Requiring CPP11 by default in DGtal #1080

Merged
merged 34 commits into from
Dec 14, 2015
Merged

Conversation

dcoeurjo
Copy link
Member

This PR:

  • remove some #ifdef WITH_C11 switches and update the code accordingly
  • change some deprecated boost::array to std::array
  • various cpp11 related fixes

@dcoeurjo
Copy link
Member Author

I'm still working on it, it just wanted to notify the devs.

@rolanddenis
Copy link
Member

👍

@dcoeurjo
Copy link
Member Author

@rolanddenis I have an issue with catch on travis (see https://gist.github.com/dcoeurjo/888095e6f971e2d25267)... any idea?

@rolanddenis
Copy link
Member

It seems that gcc doesn't support std::underlying_type until version 4.7 (see eg http://stackoverflow.com/questions/7931378/how-do-i-get-the-fundamental-type-of-an-enum)
A fix could be to define CATCH_CONFIG_CPP11_NO_IS_ENUM when a previous gcc version is used.

@dcoeurjo
Copy link
Member Author

@JacquesOlivierLachaud : std::binder1st is deprecated in c++11 (see LocalEstimatorFromSurfelFunctorAdapter.h, line 145) and an error is raised on travis.

Coud you please have a look to the code ? It is unclear for me how to fix it with C++11 stuffs (better solution than redefining a "binder" inner struct)

@rolanddenis
Copy link
Member

A way to solve this is to replace
typedef std::binder1st<Metric> MetricToPoint;
by
typedef std::function< typename Metric::Value ( typename Metric::Point ) > MetricToPoint;
including <functional> header.

In LocalEstimatorFromSurfelFunctorAdapter.ih, line 170, the construction of metricToPoint becomes:
MetricToPoint metricToPoint = std::bind( *myMetric, myEmbedder( *it ), std::placeholders::_1 );
(adding constness to metricToPoint and vfunctor is possible).

It is also possible to directly construct vfunctor from a lambda and thus avoiding declaring those private typedef.

Note: the documentation talks about a CMetric concept, is it not rather CMetricSpace ?

@dcoeurjo
Copy link
Member Author

Thanks @rolanddenis !

@dcoeurjo
Copy link
Member Author

(Yeah.. CMetric -> CMetricSpace, fixed now)

@dcoeurjo
Copy link
Member Author

Allright, this PR is ready for review.

@kerautret
Copy link
Member

I look it and also with DGtalTools

@rolanddenis
Copy link
Member

The "WITH_C11" value is still displayed and "false".
See https:/dcoeurjo/DGtal/blob/cpp11/cmake/CheckDGtalOptionalDependencies.cmake#L39

@dcoeurjo
Copy link
Member Author

you're right. I'll clean this up

MESSAGE(STATUS "Supported c++11 features: [${C11_FEATURES} ]")
ADD_DEFINITIONS("-DWITH_C11 ")
ELSE()
MESSAGE(FATAL_ERROR "Your compiler does not support any c++11 feature. Please specify another C++ compiler of disable this WITH_C11 option.")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

since we can no more use it without c++11 we can update "or disable this WITH_C11 option"

@kerautret
Copy link
Member

looks fine for me, tested on different compilators, and it looks also to fix the DGtalTools "rand" related bug. ;) !
Just correct also the message that appears on non cpp11 compilers.
And also the main documentation we should precise it: (in moduleBuildDGtal.dox) ?
(and also here http://dgtal.org/download/)

@dcoeurjo
Copy link
Member Author

yep, you're right.

@dcoeurjo
Copy link
Member Author

I've also changed the dgtal.org web page.
Ready for a merge ?

@kerautret
Copy link
Member

yes thanks, perfect ! I have also the fix in DGtalTools, merging..

kerautret added a commit that referenced this pull request Dec 14, 2015
Requiring CPP11 by default in DGtal
@kerautret kerautret merged commit 2baa9ef into DGtal-team:master Dec 14, 2015
@dcoeurjo
Copy link
Member Author

Thanks... Please have a look to #1090 for the boost::random cleanup.

@dcoeurjo dcoeurjo deleted the cpp11 branch December 14, 2015 12:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants