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

Update CowPtr and Clone to fix testClone2 #1382

Merged
merged 10 commits into from
Feb 4, 2019

Conversation

JacquesOlivierLachaud
Copy link
Member

@JacquesOlivierLachaud JacquesOlivierLachaud commented Jan 16, 2019

PR Description

This PR fixes an issue in testClone2.cpp (and more generally in the Clone / CowPtr and CountedPtr mecanism) so that it compiles correctly both with gcc and clang (and hopefully Visual Studio).
GCC was unable to solve a construction of a CountedPtr member from a Clone parameter in an efficient way.

Related to #1381. Fix #1203.

Checklist

  • Unit-test of your feature with testClone2.cpp
  • Doxygen documentation of the code completed (classes, methods, types, members...)
  • Documentation module page added or updated.
  • New entry in the ChangeLog.md added.
  • No warning raised in Debug cmake mode (otherwise, Travis C.I. will fail).
  • All continuous integration tests pass (Travis & appveyor)

@rolanddenis
Copy link
Member

Also related to #1203

Copy link
Member

@dcoeurjo dcoeurjo left a comment

Choose a reason for hiding this comment

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

Thanks a lot @JacquesOlivierLachaud . Just couple of typos.

ChangeLog.md Outdated Show resolved Hide resolved
src/DGtal/base/Clone.h Outdated Show resolved Hide resolved
tests/base/testClone2.cpp Outdated Show resolved Hide resolved
@JacquesOlivierLachaud
Copy link
Member Author

Travis CI errors seem not to be related to this PR. Was there a recent PR modifying the travis script ?

@rolanddenis
Copy link
Member

I appends sometimes, it seems to be temporary issues with Travis... I've restarted the jobs ;)

@JacquesOlivierLachaud
Copy link
Member Author

Looks ready now :-)

src/DGtal/base/Clone.h Show resolved Hide resolved
src/DGtal/base/CowPtr.h Outdated Show resolved Hide resolved
tests/base/testClone2.cpp Outdated Show resolved Hide resolved
Co-Authored-By: JacquesOlivierLachaud <jacques-olivier.lachaud@univ-savoie.fr>
src/DGtal/base/CowPtr.h Outdated Show resolved Hide resolved
Co-Authored-By: JacquesOlivierLachaud <jacques-olivier.lachaud@univ-savoie.fr>
@dcoeurjo
Copy link
Member

dcoeurjo commented Feb 4, 2019

All good for me too. Thx @JacquesOlivierLachaud and @rolanddenis !

Merging (can you please close the associated Issues ?)

@dcoeurjo dcoeurjo merged commit 33db467 into DGtal-team:master Feb 4, 2019
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.

testClone2 failure with g++ 5.4.0 (Ubuntu 16.04)
3 participants