Skip to content

Add c++11 flag #2035

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

Closed
wants to merge 3 commits into from
Closed

Conversation

denix56
Copy link
Contributor

@denix56 denix56 commented Oct 21, 2017

The flag should give the great opportunity for contributors to use such c++11 features such as rvalue references, lambdas and lots of other cool things.
Also, it will save compatibility with old compilers (for now).
To make the check for C++11 easy, I make the required cmake version 3.1 (It has CXX_STANDARD property). I don`t think, that anyone uses cmake 2.8 with latest pcl versions.

Also I added HAVE_CXX11 definition to pcl_config.h
#1638

It will be the base for checking the compile features available.

@denix56 denix56 mentioned this pull request Oct 21, 2017
3 tasks
@taketwo
Copy link
Member

taketwo commented Oct 22, 2017

I don't quite get how adding this option saves compatibility with old compilers. As soon as we introduce some code that uses C++11 features, it will be impossible to compile the library without turning PCL_ENABLE_C++11 option on.

Also, I'd avoid bumping minimum required CMake version. Ubuntu 14.04 comes with CMake 2.8 and people are still using it. For example, Travis only recently switched to 14.04. ROS Indigo requires 14.04.

@denix56
Copy link
Contributor Author

denix56 commented Oct 22, 2017

@taketwo , The code. that uses C++11 will be protected with HAVE_CXX11

@taketwo
Copy link
Member

taketwo commented Oct 22, 2017

OK, fair enough. But then we need to maintain two implementations, one for the old standard, and one for the new?

@denix56
Copy link
Contributor Author

denix56 commented Oct 22, 2017

I suppose, that it will be a transitional stage before movement to C++11
I think that it will be pretty nice to give the contributors the ability to write C++11 code starting from pcl 1.9.0.

@SergioRAgostinho
Copy link
Member

I don't think we have enough manpower for maintaining both, therefore my opinion is to defer the transition to c++14 to when this comes enabled by default by the compilers. In gcc it already does and clang jumped into the bandwagon and should implement the same in it's major version 6, which should come out in 6 months to a year. By then I would start doing the full transition.

@moriarty
Copy link
Contributor

I was surprised to see an auto_ptr in the code, and opened a pull request... I should have checked the issues first.

Now I'm even more surprise C++11 isn't a requirement.

@jasjuang
Copy link
Contributor

jasjuang commented Nov 6, 2017

+1 for C++11 support

@BrannonKing
Copy link

+1 for C++14 now and ditching boost altogether.

@SergioRAgostinho
Copy link
Member

Ditching Boost entirely won't happen, but we'll reduce the number of things we use from them.

@giacomodabisias
Copy link
Contributor

C++11 is a must nowadays. I don't understand why there is still a discussion about it. Maintaining two versions is undoable, having pragmas protecting the C++11 code is also not doable since the code base is big and it would become quickly horrible. Just move to C++11 from the next version; PCL will be available without C++11 for the older versions and thats it. Delaying this transition will only make the library less efficient, grow slower and have less people contribute to it.

@moriarty
Copy link
Contributor

moriarty commented Jun 5, 2018

@taketwo & @SergioRAgostinho from the comment above:

I don't think we have enough manpower for maintaining both, therefore my opinion is to defer the transition to c++14 to when this comes enabled by default by the compilers. In gcc it already does and clang jumped into the bandwagon and should implement the same in it's major version 6, which should come out in 6 months to a year. By then I would start doing the full transition.

clang-6.0/bionic,now 1:6.0-1ubuntu2 amd64 [installed,automatic]
  C, C++ and Objective-C compiler
gcc-7/bionic,now 7.3.0-16ubuntu3 amd64 [installed,automatic]
  GNU C compiler

Ubuntu 18.04 LTS has the above compilers available... and they enable C++14 out of the box.
So does that meet the requirements to switch?

@taketwo
Copy link
Member

taketwo commented Jun 6, 2018

For me, yes. I think we should enable C++14 as soon as PCL 1.9.0 is shipped.

@taketwo
Copy link
Member

taketwo commented Nov 8, 2018

Superseded by #2382.

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.

7 participants