Skip to content
This repository has been archived by the owner on Sep 1, 2023. It is now read-only.

Fix build by adding -Wno-error=terminate CXX flag #1415

Closed
wants to merge 1 commit into from

Conversation

mewmew
Copy link
Member

@mewmew mewmew commented Apr 9, 2018

Fixes #1414.

@mewmew
Copy link
Member Author

mewmew commented Apr 10, 2018

The CLA has been signed now.

@mewmew
Copy link
Member Author

mewmew commented Apr 10, 2018

Is is possible to restart the continuous integration checks somehow?

@rhyolight
Copy link
Member

@mewmew Sorry it took us so long to get to this. All the CI will restart whenever you push more code to the remote branch.

@rhyolight
Copy link
Member

I have manually restarted the CI for CircleCI (OS X) and AppVeyor (Windows). They both seemed to have similar environment problems (seemingly unrelated to NuPIC).

@mewmew
Copy link
Member Author

mewmew commented Apr 14, 2018

I have manually restarted the CI for CircleCI (OS X) and AppVeyor (Windows). They both seemed to have similar environment problems (seemingly unrelated to NuPIC).

I noticed that too, e.g. from https://ci.appveyor.com/project/numenta-ci/nupic-core/build/0.3.0.2359

 The C compiler
    "C:/Python27-x64/Scripts/gcc.exe"
  is not able to compile a simple test program.

@rhyolight
Copy link
Member

Maybe your option will not work with their version of GCC?

@breznak
Copy link
Member

breznak commented Jul 30, 2018

Thanks for spotting the issue!
It's related to non-throwing destructors in c++11, which gcc4.7+ apparently adheres to.
https://stackoverflow.com/questions/42976461/exception-in-destructor-c
The proposed global solution is not optimal, though.

A) Best option would be not to throw (aka NTA_CHECK) in the destructor ~Output() at all, although the comment explicitely mentions that we are aware of the "bad practice" but we need to check that.

B) I'm proposing a fix that only allows the behavior locally.

@rhyolight
Copy link
Member

I think this can be considered fixed by #1432.

@rhyolight rhyolight closed this Aug 6, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants