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

Fix missing template parameter for overloaded functions that use PointVector. #1232

Merged
merged 7 commits into from
Jan 13, 2017

Conversation

rolanddenis
Copy link
Member

@rolanddenis rolanddenis commented Jan 9, 2017

🎉 🍻 Happy New Year to all the DGtal team ! 🍻 🎉

PR Description

The hint comes from a compilation test of DGtal using Intel Compiler 17. It raises errors in display-related classes (Display[23]DFactory, Viewer3DFactory, ...) about overloaded functions/methods not found.

A minimal example is:

template < unsigned int N >
struct Container {};

template < int N, typename TContainer = Container<N> >
struct Dummy {};

template < int N >
void f( Dummy<N> const & )
{
}

int main()
{
    Dummy<2> toto;
    f(toto);
    return 0;
}

that fails to compile with error:

test.cpp(15): error: no instance of function template "f" matches the argument list
            argument types are: (Dummy<2, Container<2U>>)
      f(toto);
      ^
test.cpp(8): note: this candidate was rejected because at least one template argument could not be deduced
  void f( Dummy<N> const & )

The problem comes from the different integer type in template parameters (int versus unsigned int) that leads icpc to ignore the default TContainer type when searching for overloaded functions/methods.

In DGtal, it corresponds to the Dimension typedef that is a signed integer versus the unsigned std::size_t that is used to specify the size of a std::array (the default container for PointVector).

It is probably a bug since g++ and clang++ doesn't matter about this issue, but anyway, the TContainer template parameter is actually missing from those overloaded functions/methods in order to accept PointVector with non-standard value container, thus leading to this PR submission.

Intel Compiler 17 now compiles without error, but still some warnings about deprecated use of std::binder2nd (see #1221) and another warning about an invalid usage of auto (probably another bug). Version 16 still fails to compile DGtal (default options) but on a clearly valid C++11 syntax.

Checklist

  • [ ] Unit-test of your feature with Catch.
  • [ ] 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 Author

Where are all Travis jobs ? 😱

@dcoeurjo
Copy link
Member

😄 my bad... While working on MacOs bots, I've disabled linux ones...

@dcoeurjo
Copy link
Member

and happy new year 🎉

@dcoeurjo
Copy link
Member

travis up and running ;) can you please merge the master again?

@rolanddenis
Copy link
Member Author

Done :octocat:

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.

Approved 👍 thanks a lot

@dcoeurjo dcoeurjo merged commit fbeb4b4 into DGtal-team:master Jan 13, 2017
@rolanddenis rolanddenis deleted the fix_intel branch January 14, 2017 11:17
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.

2 participants