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

Debug/Display for numeric types #230

Closed
danakj opened this issue Apr 12, 2023 · 5 comments · Fixed by #244, #256 or #253
Closed

Debug/Display for numeric types #230

danakj opened this issue Apr 12, 2023 · 5 comments · Fixed by #244, #256 or #253
Labels
design Design of the library systems as a whole, such as concepts
Milestone

Comments

@danakj
Copy link
Collaborator

danakj commented Apr 12, 2023

We need to be able to print numerics without casting them to primitives every time.

Some basic options exist:

  1. Just for Gtest, add a PrintTo method for them, in the subspace/test/ code.
  2. Add operator<< for them. This means STREAMS. This is bad for compile time.
  3. Add a ToString() method, and implement PrintTo on top of it.

Modeling Debug/Display traits as concepts instead we can do it differently, but something like (2) or (3) above.

This version will look up write_debug() through ADL:

template <class T>
concept sus::fmt::Debug = requires (const T& t) {
  write_debug(t, sus::fmt::Formatter&);
};

It can be implemented as a friend function:

namespace num {
struct i32 {
    friend void write_debug(i32, fmt::Formatter&);
};

Or a free function:

template <std::same_as<i32> T>
void write_debug(T, fmt::Formatter&);

This version will look for the method on the class. It can't be added types you don't control.

template <class T>
concept Debug = requires(const T& t, fmt::Formatter& fmt) { 
    { t.write_debug(fmt) } -> std::same_as<void>; };

And the method has to clutter the type's public API, even though it should not be called directly.

I think we should go with the first option.

Then we have two more choices:

  1. Add PrintTo() for GTest based on write_debug.
  2. Implement write_display() for these types as well, and provide operator<< in some non-default header for them, which tests include.
  3. Do both, as tests would use the write_debug in place of write_display.

As there's much to be done for how the Formatter would work, the Formatter should not be stabilized along with numeric types, but the use of them through PrintTo() or << depending on the above, is fine.

@danakj danakj added this to the stable-numerics milestone Apr 12, 2023
@danakj danakj added the design Design of the library systems as a whole, such as concepts label Apr 12, 2023
@danakj
Copy link
Collaborator Author

danakj commented May 31, 2023

Adding support for fmtlib here: #244

There is so much to re-implement for display and fmt is high quality and widely used. I think it's the right path, so adding support for formatting of each type.

I need to look further into:

  • display vs debug printing
  • printing types without a formatter as a string of bytes
  • so that Option<unknown> can print Some(...bytes...)
  • add a PrintTo() for anything that is fmtlib formattable.
  • maybe provide a stream impl for anything fmtlib formattable as well in a separate compat header.

@danakj
Copy link
Collaborator Author

danakj commented Jun 3, 2023

There is no debug vs display differentiation in fmtlib.

We could add to parse() to look for a ? formatting character so you could write {:?} and get different behaviour. At the moment there's nothing that I would print differently and you don't get the distinction of a type being Debug but not Display. It would have to be Display in order to maybe also be Debug. In which case it's not worth doing anything with until we have types we want to print differently in a debug mode than in a "display" mode. But then.. those are normally types that are only Debug and in this case they'd have to be Display and just the user would not get nice stuff from them. Unless Debug was a long multiline thing and Display was a short thing? idk I don't see it yet, we can consider using the ? formatting character if we need it on a per-type basis or for all subspace types.

@danakj
Copy link
Collaborator Author

danakj commented Jun 3, 2023

subspace/string/compat_stream.h will allow streaming of any (public) subspace type that is formattable.

@danakj
Copy link
Collaborator Author

danakj commented Jun 3, 2023

For some reason the stream operator is not being picked up and used by GTest's ADL mechanisms on MSVC and it's breaking my brain. The decltype stuff for SFINAE all resolves when I reproduce it myself. Not sure if this is a blocker issue or what.

OK it was caching problems due to one TU seeing the operator<< template and one TU not, and compilers cacheing the first resulting GTest template for that type. Resolved by putting the operator<< template in prelude.h which it can do with just iosfwd.

Since we have operator<< there is no need for PrintTo.

@danakj
Copy link
Collaborator Author

danakj commented Jun 4, 2023

I think #244 is sufficient to close this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
design Design of the library systems as a whole, such as concepts
Projects
None yet
1 participant