NWChemEx / Utilities

Generic, helpful C++ classes
https://nwchemex.github.io/Utilities/
Apache License 2.0
0 stars 1 forks source link

Fix #68 #110

Closed keceli closed 1 year ago

keceli commented 2 years ago

This PR is a workaround to fix #68. It turns out the problem is not just with clang. Same unit tests also fail with GCC 12 since the compiler cannot find the overloads. I have a minimal example here.

The discussion on SO suggests overloading << for stl containers is not ideal due to possible future violations of ODR (one definition rule), but I guess it is ok for our purposes. If not, we may need to make changes in the whole stack to use fmt::print instead of << since fmt does not provide << overloads for stl containers.

ryanmrichard commented 2 years ago

@keceli We were intentionally not putting the operator<< overloads directly in the std namespace to avoid angering the ODR (there's quite a few libraries out there, including TA, that do define overloads in the std namespace for standard types). When we want our overloads we would do something like using namespace utilities::printing to bring the overloads into scope.

You should never put something in the std namespace unless the standard tells you to (e.g., std::hash).

jwaldrop107 commented 1 year ago

The issue this was targeting has been resolved now, so I'm going to go ahead and close this.