Switch to c++17

[breznak]

from modern C++ features support, reducing dependencies.

• enable c++17 in CMake, compile code with it locally
  • fix additional c++17 nuances
• enable in all CI and make sure all compile
  • update build scripts
  • make sure recent version of compiler installs in the CI
  • Linux
  • Windows
  • OSX
• replace suitable boost:: with std:: -- boost now optional with c++17
• integrate c++17 enhancements in codebase (may not need all at once)
  • replace all *T, Real*, etc with std::array (for fixed size), std::vector for variable respectively; no speed impact
  • use RAII; remove pointers, make local variable where possible; elsewhere use smart_pointers
  • use const where possible, cleaner, safer, faster
  • use auto for cleaner code
  • use for-each for(auto item : list)
  • replace loops with <algorithms> where possible; ie fill(), etc

This is a great readup for any c++ programmer:
https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md

[dkeeney]

It occurred to me that there may be a problem with requiring the C++17 compiler.
The choice of C++ compiler may not be arbitrary.
As I understand it, if we want to support Python 2.7 then the library
we build must use C++9 in order to be compatible. Python 3.5 and above
can handle C++17 but Python 2.7 cannot.

C++ compiler	Visual Studio	Visual C++	Python
C++17	2015, 2017	14.0/15.0	3.5, 3.6
C++11	2010	10.0	3.3, 3.4
C++9	2008	9.0	2.6, 2.7, 3.0, 3.1, 3.2

Is this chart correct?
If so, all of the Nupic Python code must convert to Python 3.5 or higher
before we can use it with our library if we use the C++17 compiler.

It may be that this is true only on Windows where the Python distribution
is in binary. With Linux Python is compiled from sources so it only
matters which version of compiler is used.

Does anyone know?

[breznak]

(Preface: I don't even have Windows machine, but from what I found)

• confirmed the c++17 -> py>2.7
• non issue on linux/osx
• for windows: does installing this help? https://www.microsoft.com/en-us/download/details.aspx?id=44266

[chhenning]

@dkeeney Do you have a link that would explain how a c++ version can affect python compatibility? As far as I understand c++ code is just a dll. Thanks!

[dkeeney]

The link you provided installs the C++9 compiler into VS2017....something I would rather not do. We would have to dumb down our code so it could compile.

The problem is that the Python2.7 for windows is a DLL that was compiled with C++9. linking this with anything newer is incompatible according to the Nupic release README.
Also see https://wiki.python.org/moin/WindowsCompilers

So, that is why the big project to upgrade all of Nupic's Python code to Python3.6+ so that we can use a newer compiler on Windows.

[chhenning]

Is this restriction maybe just related to CPython? I think we might mix up something here.

For instance, pybind, which is c++11 project can also target python 2.7.
http://pybind11.readthedocs.io/en/stable/intro.html

[dkeeney]

As I understand it the problem is with the compiler that built the windows version of the Python library. The link you provided indicated it can work with 2.7 but that did not say anything about Windows. But perhaps pybind11 has a way around that problem.
@chhenning can your interface work with both Python 3.6 and 2.7? I seem to remember that you sort of gave up on trying to support both. Was it the library or just the shear amount of work changing the Python code to work on both versions?

[chhenning]

@dkeeney Yes, one of the main features of pybind is to write bindings that work with 2.7 and the 3.x version of python.
Write once run anywhere. ;-)

[breznak]

Python 3.5 and above can handle C++17 but Python 2.7 cannot.

Reiterating on the Win-Py problem. Could we temporarily move the bindings-py part to Py 3.x, then move to c++17, and then proceed with replacement to pybind. How much work would the porting from 2.7->3.5 be?

Other option on the plate is temporarily dropping py support for win, and roll with just c++ until we get to pybind.

[dkeeney]

Well, the problem is that the SWIG code is setup for Python 2.7. I think it would be hard to switch to 3.x without switching to pybind. I don't know any easy way to split this up into smaller parts. Perhaps we could build the pybind part first and make sure it works and then rip out the SWIG. Oh, but pybind requires at least C++11 I think. So that would not work. Maybe the only way to do it is as one big PR. Hack out SWIG, put in pybind (upgrading at least to C++11 as needed to get it to build). Since we already have it working on Windows that way it would not take long to do. But traceability would be hard to maintain. So many changes....almost as bad as my capnproto PR :)

…

On Fri, Aug 31, 2018 at 5:11 AM breznak ***@***.***> wrote: Python 3.5 and above can handle C++17 but Python 2.7 cannot. Reiterating on the Win-Py problem. Could we temporarily move the bindings-py part to Py 3.x, then move to c++17, and then proceed with replacement to pybind. How much work would the porting from 2.7->3.5 be? Other option on the plate is temporarily dropping py support for win, and roll with just c++ until we get to pybind. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#55 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AFBa_63L3X137JNxJgkuphiapb7PO-wgks5uWSftgaJpZM4V1cVj> .

[chhenning]

@breznak There is no problem with python in Windows. All works as expected. If you use the pybind (see my code) then you should be good.

[breznak]

Oh, but pybind requires at least C++11 I think. So that would not work.

We are c++11 now.

If you use the pybind (see my code) then you should be good.

We are heading there, slowly. This discussion is IF we could get there without temporarily breaking windows builds.

But traceability would be hard to maintain. So many changes....almost as bad as my capnproto PR :)

Yep, even capnp is a huge feat! Let's be modest and take step-by-step approach. 🍷

Given current state of things, I think we can proceed this way:

0. move Windows CI to MSVC (breaks old-PY on Win )
1. switch to C++17

• upgrade all compilers in CIs
• minimal c++17 support (breaks SWIG for all platforms ??) (shoud be a quick feat)
• some c++17 nice things - use std::filesystem etc, remove all dependencies Reduce dependencies #47 (longer task)

2. plan modularization

• PY bindigns using pybind as first use-case

PS: what is the status of (PY) nupic, resp htm-community/nupic.py ? Anybody working with that?

• By providing {lang} bindings, do we "just" port the bindings (as py here now), or add the full-blown repo?
• we should setup the PY-repo to use our bindings, there's lots of tests and use-cases, so to see if we don't break stuff.

[dkeeney]

That sounds like a reasonable approach. My feeling is that we should ignore Windows build entirely until we get it all put together. Keeping one compiler happy is a big enough task. After we get rid of SWIG we can put it back in with much less work. From what I understand, @christian Henning <chhenning@gmail.com> was working on porting the py code to run under Python 3.x. I don't know how far he got before his real job got in the way. But if Python2.7 will work with pybind11 (and it should) then upgrading to Python 3.x could be a separate set of PR's.

…

On Fri, Aug 31, 2018 at 7:04 AM breznak ***@***.***> wrote: Oh, but pybind requires at least C++11 I think. So that would not work. We are c++11 now. If you use the pybind (see my code) then you should be good. We are heading there, slowly. This discussion is IF we could get there without temporarily breaking windows builds. But traceability would be hard to maintain. So many changes....almost as bad as my capnproto PR :) Yep, even capnp is a huge feat! Let's be modest and take step-by-step approach. 🍷 Given current state of things, I think we can proceed this way: 1. move Windows CI to MSVC (breaks old-PY on Win ) 2. switch to C++17 - upgrade all compilers in CIs - minimal c++17 support (breaks SWIG for all platforms ??) (shoud be a quick feat) - some c++17 nice things - use std::filesystem etc, remove all dependencies #47 <#47> (longer task) 1. plan modularization - PY bindigns using pybind as first use-case PS: what is the status of (PY) nupic, resp htm-community/nupic.py ? Anybody working with that? - By providing {lang} bindings, do we "just" port the bindings (as py here now), or add the full-blown repo? - we should setup the PY-repo to use our bindings, there's lots of tests and use-cases, so to see if we don't break stuff. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#55 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AFBa_wGb2z4j8OmS21gPYtci7OnrHmTbks5uWUJggaJpZM4V1cVj> .

[dkeeney]

I am starting to look at what might be coming next after removing capnproto and it looks like switching to the C++17 library is the next step.

From my viewpoint this PR would do the following:

1. change compiler settings to C++17
2. Delete APR dependencies
3. Delete PCRE dependency
4. Delete boost dependency
5. We are compiling with both YamlLib and YamlCppLib. Remove YamlLib.
6. Add smart pointers to Regions and Links only.
7. Make everything work again.

The objective is to do the minimum needed to prepare for pybind11. I am not going to try to replace loop indexes with auto. I am not going to replace arrays with vectors or loops with algorithms. Those are all good things that make the code cleaner but it still runs the way it is.
In other words, if removing the dependencies doesn't break it then don't try to fix it.

I don't see this as being very much work...not like capnproto. Most if it is just letting the compiler and unit tests to tell me what needs to be fixed. No real logic changes or new code. I already have a C++ only version that already has this working to use as a guide. SWIG generated code is C++11 compatible so it should be OK as long as it is not using any feature that was deleted in C++17.

I think that once we have this done its time for the PR to replace SWIG with pybind11 as a separate modularization.

Oh, and I will need to re-clone from scratch as soon as the capnproto change is merged so I am working directly from the htm-community.cpp repository.

[breznak]

and it looks like switching to the C++17 library is the next step.

I think this is the next crucial step!

Delete APR dependencies
Delete PCRE dependency
Delete boost dependency
We are compiling with both YamlLib and YamlCppLib. Remove YamlLib.

Let's make the removal of dependencies the first step and a separate PR. (Except Boost, where c++17 functionality is needed). Even for switch to c++17, let's switch first, and then get rid off boost (?)

I think one more needed prerequisite is #69 , current CI runs in MingW imho, which will cause unnecessary problems with c++17

[dkeeney]

Ok, I agree. Lets get rid of those dependencies except boost before switching to C++17. However that means I will have to use boost for os/Path and os/Directory. I guess that is ok because it will be easy to switch from boost to C++17 when the time comes. I suggest that we hold off on Visual Studio until after you get rid of Swig. The numenta documentation says that it will not work unless you install a very old compiler. I was not able to get the bindings to build under Visual Studio with C++11 although I have not tried since we removed capnproto. And I agree MingW is not really a viable build environment.

…

On Tue, Sep 18, 2018 at 10:33 AM breznak ***@***.***> wrote: and it looks like switching to the C++17 library is the next step. I think this is the next crucial step! Delete APR dependencies Delete PCRE dependency Delete boost dependency We are compiling with both YamlLib and YamlCppLib. Remove YamlLib. Let's make the removal of dependencies the first step and a separate PR. (Except Boost, where c++17 functionality is needed). Even for switch to c++17, let's switch first, and then get rid off boost (?) I think one more needed prerequisite is #69 <#69> , current CI runs in MingW imho, which will cause unnecessary problems with c++17 — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#55 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AFBa_xJuxXtMcDwy_WU6pUbtf7i4rPIXks5ucS54gaJpZM4V1cVj> .

[breznak]

I suggest that we hold off on Visual Studio until after you get rid of Swig. The numenta documentation says that it will not work unless you install a very old compiler. I was not able to get the bindings to build under Visual Studio with C++11 although I have not tried since we removed capnproto. And I agree MingW is not really a viable build environment.

Great memo, thanks for the insight! I'm copying this info to the MingW/MSVC Win issue.

[breznak]

FYI https://github.com/gulrak/filesystem if we wanted to use std::filesystem but prefered to stay on c++11 (not switching to c++17) ?

[dkeeney]

This would be better than including boost in those cases that we need it.
I did notice that they wanted windows users to use wstring( ) for paths rather than UTF8. That might be a problem.

[breznak]

This would be better than including boost in those cases that we need it.

I'd still prefer the native c++17 when it's available (on 20th Sep. Apple releases new iOS with XCode11 out of beta) then all our platforms should be c++17 conformant.

What I'm considering is if we want to keep boost support optionally(!) around? It was a pain to setup (link) properly, and albeit we now only use it for boost::filesystem, boost has vast usages.. (??)

[dkeeney]

keep boost support optionally(!) around?

We can do that. I agree it has usages but it comes at a price .. that of the download time and shear size. But if it is for use by the apps (not our library) then it would probably be better for them to download and install it separately rather than blot our library with a copy of boost that we don't really use.

[breznak]

download and install it separately rather than blot our library with a copy of boost that we don't really use.

true, we might drop it. Just boost is now downloaded only optionally (eg non of the CI actually uses it) and via cmake, so it does not really bloat the repo (anymore)

[breznak]

https://discuss.circleci.com/t/macos-10-15-as-container/32548
XCode11 with c++17 and <filesystem> released, waiting for CircleCI to offer macOS 10.15 images so we can turn this on, (and remove boost)

[dkeeney]

I am inclined to keep the minimum at C++11 for a while. I suspect that there are people that have not or can not upgrade to C++17 yet. Some may not be allowed to upgrade due to company policy.

We just want to be sure if the they do have a compiler that supports that we do not use boost.