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

How to re-use existing string-conversion template #1024

Closed
baldurk opened this issue Sep 14, 2017 · 6 comments
Closed

How to re-use existing string-conversion template #1024

baldurk opened this issue Sep 14, 2017 · 6 comments

Comments

@baldurk
Copy link

baldurk commented Sep 14, 2017

Description

I have a function template already in my project that defines a function like:

template<typename T>
std::string ToStr(const T &el);

Which I specialise it for each type in my codebase. There's no default implementation as I want to make sure I implement stringifying for all the types I care about.

I'd like to re-use this for catch stringifying, but currently the only way I see to do that is to specialise StringMaker for each type once more and forward the call to ToStr, which is a bit awkward.

Currently what I'm thinking of doing is just locally modifying catch to replace the final return unprintableString; in EnumStringMaker with a forwarding call to my ToStr, which will achieve what I'm after. I'm wondering though - is there an intended better solution for this? To create a 'catch-all' stringmaker that forwards for all uncaught types. I'm not fussed about whether catch or my function handles basic types like ints/floats/etc so it's fine for me to only forward the unhandled cases.

If there isn't a better existing solution, would you be OK with adding this as an upstream feature-macro that could be overridden? E.g. similar in principle to CATCH_CONFIG_NOSTDOUT - something like CATCH_CONFIG_NOUNPRINTABLE where it expects a function of similar prototype to be defined to print any types that catch doesn't handle built-in and that don't have an existing StringMaker specialisation.

@philsquared
Copy link
Collaborator

You are correct that there's not really a way to inject into the final bail out with unprintableString with your own code.
But it seems like a sensible thing to want to customise. I'll mark this as a feature request and put it on the queue (although I can't promise how soon we'd get to it).
Thanks for bringing this up.

@baldurk
Copy link
Author

baldurk commented Sep 14, 2017

Thanks, as long as I'm not missing something obvious then I'm fine with just customising my local catch.hpp to call into my own impl, and if a future release supports official customisation I'll switch to that.

@baldurk
Copy link
Author

baldurk commented Mar 7, 2018

I gave this a whirl on the 2.2.0 release, it works great thanks!

One thing I realised after actually implementing it though is that actually a lot of the things I want to stringify are enums, which unfortunately never make it down to this fallback but instead get coerced and printed as integers earlier.

I realise that's sort of a separate request so I can open a new issue for it, but it would be very nice to pipe enum stringification through the fallback as well (I already have an overload defined for all my enums).

If it's possible through sufficient C++ magic to seamlessly detect an overload of the custom function and fall back to the underlying type if it doesn't exist that would be superb, but for my purposes I'd be happy with a #define to opt-in to having all stringifying responsibilities on the CATCH_CONFIG_FALLBACK_STRINGIFIER with resulting failed compilation if any enum is not handled.

@horenmar
Copy link
Member

horenmar commented Mar 8, 2018

This actually used to be the case during development, but it seemed like it would forward down a bit too many types. I will think about reverting it back.

If it's possible through sufficient C++ magic to seamlessly detect an overload of the custom function and fall back to the underlying type if it doesn't exist

This is entirely possible, but something you will want to do in your local copy. Doing this via TMP is rather costly and Catch already has trouble with compilation times.

@baldurk
Copy link
Author

baldurk commented Mar 8, 2018

Understood, I don't really have an intuition for what's expensive and what's not. I'm fine with just being an opt-in path to override all types without any TMP. In my case my stringification can handle any type so it's fine to forward down anything at all, but I understand that a default handler for enums is probably preferable behaviour.

For now I'll make some local changes to suit my needs, thanks again for looking into this!

johannesmoene pushed a commit to johannesmoene/Catch that referenced this issue May 10, 2018
horenmar added a commit that referenced this issue May 14, 2018
This should align more closely with the intended semantics, where
types without `StringMaker` specialization or `operator<<` overload
are passed down to the user defined fallback stringifier.

Related to #1024
@horenmar
Copy link
Member

After some deliberation, I decided to change this behaviour for the next release. The fallback stringifier will now be passed everything that does not provide either StringMaker specialization or operator<< overload.

horenmar added a commit that referenced this issue May 14, 2018
martinmoene added a commit to johannesmoene/Catch that referenced this issue Sep 26, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants