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

Some header file declarations in underscore.hpp are inconsistent with definitions in underscore.cpp #12

Open
mohitanand001 opened this issue Oct 1, 2018 · 8 comments
Labels
help wanted Extra attention is needed

Comments

@mohitanand001
Copy link
Owner

No description provided.

@srinathkp
Copy link

Can I work on this?

@mohitanand001
Copy link
Owner Author

Sure @srinath9721

@mohitanand001
Copy link
Owner Author

mohitanand001 commented Oct 17, 2018

@srinath9721 also make sure the order of functions is same in cpp and hpp files. Please ping if any help is needed.

@mohitanand001
Copy link
Owner Author

@srinath9721 are you still working on this. Kindly keep us updated.

@gubatron
Copy link
Collaborator

gubatron commented Oct 30, 2018

I'm working on a somewhat related branch.

I'm introducing formal tests and when I try to use the functions in my test cases, I'm having a lot of issues linking the test executable.

Things like:

Undefined symbols for architecture x86_64:
  "std::__1::vector<int, std::__1::allocator<int> > _::set_union<std::__1::vector<int, std::__1::allocator<int> >, std::__1::vector<int, std::__1::allocator<int> > >(std::__1::vector<int, std::__1::allocator<int> >, std::__1::vector<int, std::__1::allocator<int> >, std::__1::vector<int, std::__1::allocator<int> >)", referenced from:
      ____C_A_T_C_H____T_E_S_T____0() in testing.cpp.o
ld: symbol(s) not found for architecture x86_64

And it all comes from clang warnings like:

/Users/gubatron/workspace.frostwire/underscore_cpp/tests/testing.cpp:11:28: warning: instantiation of function '_::set_union<std::__1::vector<int, std::__1::allocator<int> >, std::__1::vector<int, std::__1::allocator<int> > >' required here, but no definition is available [-Wundefined-func-template]
        std::vector<int> sec = _::set_union(vec, rec, lec);
                                  ^
/Users/gubatron/workspace.frostwire/underscore_cpp/src/underscore.hpp:62:15: note: forward declaration of template entity is here
    Container set_union(Container container1, Container container2, Containers  ... others);
              ^
/Users/gubatron/workspace.frostwire/underscore_cpp/tests/testing.cpp:11:28: note: add an explicit instantiation declaration to suppress this warning if '_::set_union<std::__1::vector<int, std::__1::allocator<int> >, std::__1::vector<int, std::__1::allocator<int> > >' is explicitly instantiated in another translation unit
        std::vector<int> sec = _::set_union(vec, rec, lec);

Then I decided to make some tests with a basic templated function in underscore.hpp and underscore.cpp

// underscore.hpp
template<typename T>
void say_foo(T foo);

and then in the .cpp file

// underscore.cpp
template<typename T>
void say_foo(T foo) {
  std::cout << foo << std::endl;
}

This would cause the same issues when trying to do:

_::say_foo("Hello");
_::say_foo(1337);

The problem exists because the compiler doesn't know of declarations for:

void _::say_foo(std::string foo);
void _::say_foo(int foo);

The easy solution to it all is to get rid of the .cpp code for templated functions and put the code in the .hpp, for the case of this library that'd mean moving all implementations to the .hpp and getting rid of the .cpp file, thus making the project a header only library.

Otherwise it requires doing a bunch of forward declarations for each possible data type case use of each templated function

If we move everything to the .hpp, then this issue is also solved, no inconsistencies would exist if there was a single file.

Edit:
See:
https://isocpp.org/wiki/faq/templates#templates-defn-vs-decl
https://isocpp.org/wiki/faq/templates#separate-template-fn-defn-from-decl

@mohitanand001
Copy link
Owner Author

Even I have faced a similar issue (not sure when). I am not sure, if it would be considered a "good practice" to keep both the implementation and headers in same file. Anyway your call.

@gubatron
Copy link
Collaborator

I'll try to research if there's a way to keep them separated and not have the linkage issues.
For now I'll leave it as a header only file, which for now would make it very simple for people to use, just bring a single .hpp file.

@mohitanand001
Copy link
Owner Author

Cool

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

3 participants