search

oxfordcontrol / Clarabel.cpp

Clarabel.cpp: C/C++ interface to the Clarabel Interior-point solver for convex conic optimisation problems.
Apache License 2.0
32 stars 10 forks source link

Remove explicit template instantiations #32

Closed jwnimmer-tri closed 5 months ago

jwnimmer-tri commented 8 months ago

I know of no reason why explicit template instantiations should ever appear in a header file: code that includes the header file will already implicitly instantiate any templates that it uses. (See https://en.cppreference.com/w/cpp/language/class_template.) The effect of the instantiations is only to copy unnecessary object code into every translation unit that calls this library.

In any case, for trivial wrapper structs it's always better to use function inlining anyway; for performance, we don't actually want to be jumping through extern functions when dealing with these wrappers.

goulart-paul commented 5 months ago

This was giving a lot of compilation warnings during unit tests, all along the lines of this: 

In file included from /Users/pgoulart/projects/clarabel/Clarabel.cpp/include/Clarabel:6:
/Users/pgoulart/projects/clarabel/Clarabel.cpp/include/cpp/DefaultSettings.h:300:25: warning: 'clarabel_DefaultSettings_f64_default' has C-linkage specified, but returns incomplete type 'DefaultSettings<double>' which could be incompatible with C [-Wreturn-type-c-linkage]
DefaultSettings<double> clarabel_DefaultSettings_f64_default();

The issue appear to be that the template instantiations make the compiler accept things like this.

Happy to remerge this if we can resolve the warnings.

jwnimmer-tri commented 5 months ago

Thanks for the feedback! I learned something new about C++ today.

When I have time to circle back to this, I'll work on developing a working patch, and open a new PR.