tabulate icon indicating copy to clipboard operation
tabulate copied to clipboard

you are pulling 'std::variant' into the global namespace (bad idea)

Open peterritter opened this issue 2 years ago • 2 comments

Hi

I have been struggling with some compiler errors involving 'variant'. I have my own 'variant' class in my library and it seemed to be clashing when I pulled in the tabulate header. I had a look and found this in tabulate.hpp:

#if __cplusplus >= 201703L #include <string_view> #include using std::get_if; using std::holds_alternative; using std::string_view; using std::variant; using std::visit;

This is not a good practice to pull names from the standard library into the global namespace! Every library that uses tabulate will now have these names in the global namespace too.

I also found std::optional used this way.

I have manually gone and commented out all such using statements and added std:: prefix everywhere where they were used. It is not a lot of places - so it seems an easy fix!

Best Regards, P.

peterritter avatar Nov 02 '23 21:11 peterritter

Hi,

Thanks for the report. Would you be open to creating a PR? I agree with your comments here.

Regards, Pranav

p-ranav avatar Nov 03 '23 00:11 p-ranav

Hi, I fixed it for myself in the 'single_include/tabulate.hpp' which is what I am using. I don't know what it looks like in the rest of the library. There may be other places that need fixing too in the non-amalgamated files. I also don't know what the interaction is with the 'nonstd' namespace - if that would create issues with that? I really just uncommented these and equivalent lines:

#if __cplusplus >= 201703L #include <string_view> #include //using std::get_if; //using std::holds_alternative; //using std::string_view; //using std::variant; //using std::visit; #else // #include <tabulate/string_view_lite.hpp> // #include <tabulate/variant_lite.hpp> using nonstd::get_if; using nonstd::holds_alternative; using nonstd::string_view; using nonstd::variant; using nonstd::visit; #endif

...and then added the 'std::' prefix where required. Regards, Peter

PS: In practice, I realized that when populating large tables, it is not really practical to then iterate over the table again to set formating. It's really at the time of appending a cell to a row, that I would like to define the formating of the cell, as it is at that point that I know what the value type is. I tried quite hard to try to figure out how to format the cells in a row before appending a table. But i didn't quite manage to figure out how. If you ever get around to it that would be a good way to enhance the library.

peterritter avatar Nov 04 '23 15:11 peterritter