Define toString template before using toString calls

[kapouer]

A way to avoid (gcc 4.8.1)

test/vector_tile.cpp:121:5:   required from here
test/internal/catch_tostring.hpp:105:35: error: ‘toString’ was not declared in this scope, and no declarations were found by argument-dependent lookup at the point of instantiation [-fpermissive]
             oss << toString( v[i] );
                                   ^
test/internal/catch_tostring.hpp:204:20: note: ‘std::string Catch::toString(unsigned char)’ declared here, later in the translation unit
 inline std::string toString( unsigned char value ) {
                    ^

[kapouer]

To understand this PR: this error happens when using the non-single-file version of Catch.

[philsquared]

Thanks for raising this - and thanks for the bump - I lost track of this after getting back from holiday.

I'm not sure that moving the whole base template is the right thing to do - since that refers to StringMaker<>, which is not yet defined.

So I've just placed a forward declaration before StringMaker<>. Since it wasn't failing for me to begin with could you please give the latest commit a quick try and see if it resolves the issue for you without your patch?

[kapouer]

Yes, i can confirm @1e2f1d160336be07411775c476bfed3c7673263f resolves the issue.

[philsquared]

Cool. Thanks for letting me know

[PureAbstract]

I think this falls under 'necessary, but not sufficient' - the solution is a little more complex (isn't it always?) sigh

I've been tweaking the vector specialisation of StringMaker (specifically, adding allocator support) - see my vector_to_string branch PureAbstract@0dbcf21
(FWIW, I intend to send a pull request for that once I've finished this essay...)

If you provide an overload for Catch::toString for a user-defined type, it's not seen by StringMaker<vector> (since when that gets instantiated, it can only make use of overloads of toString which are visible when the StringMaker specialisation is defined (rather than instantiated).

I'm not making this very clear here - take a look at https://gist.github.com/PureAbstract/6601192 and it may be more apparent what's going on.

I think I've got a pretty good handle on what needs to happen, but probably won't get the time for a few days.

My current thinking is we need (in this order):

1. A declaration (but NOT definition) of the primary toString<T> template.
2. Declarations of the non-template toString overloads (since they're non-templates, it's the declarations rather than the definitions that are important - so there's no harm in defining them here)
3. Declare the StringMaker primary template, and any specialisations that make use of toString.
4. Finally, DEFINE the primary toString<T> template.

...and to reliably provide stringification, users need to specialise Catch::StringMaker rather than overloading toString

If I may quote the C++ Standard (14.7.3/p7):

When writing a specialisation
Be careful about its location
Else to make it compile
Will be such a trial
As to kindle its self immolation

[PureAbstract]

In commit PureAbstract@d6f23a9 of #198, I've hoiked the definition of rangeToString to after all the overloads of Catch::toString - which helps with the toString( vector ) case - but I'm still thinking we also need the definition of toString<T> to be after the overloads.

[philsquared]

Thanks for all the input, Andy (@PureAbstract).
My heart sinks every time 14.7.3/p7 is quoted!

What makes this stuff so fiddly is the rules are so vague and the compilers seem intent on creating a false sense of security.

I think you are right that StringMaker<> should be the primary entity to override - but that's a recent addition.
Interesting idea to have a forward decl for the base template.

I need to think about this more, too.

[PureAbstract]

Thanks for all the input, Andy

No problem - thanks for Catch :-)

My heart sinks every time 14.7.3/p7 is quoted!

I've been able to recite the limerick from memory for at least a decade now - what's worrying is that I can now also do the same thing with the paragraph number...

What makes this stuff so fiddly is the rules are so vague and the compilers seem intent on creating a false sense of security.

I wouldn't describe the rules as vague - quite the opposite. They're just impenetrable (probably why CWG spend so much time in silent contemplation...). That compiler and library vendors all seem to have read them slightly differently is , well, 'interesting'...

In a perfect world, it'd be nice to have users be able to overload toString for their type (since it's a bit less boiler plate than writing a StringMaker) - but I don't see a way around that with the current mechanism. (Aside from declaring user provided overloads of Catch::toString before before including the catch header, but that may be problematic for templated types. I'm pretty sure everyone (modulo VC6 - aka 'VC98') gets template partial specialisation right these days, but I'm not sure that partial ordering of function templates is in such a robust state.

I also considered fully qualifying the calls to toString in rangeToString - I'm currently somewhat ambivalent about allowing UDL to find it.

I'm probably going to add allocator support for basic_string at some point - but need to have a think about how to do it. (In particular, is it reasonable to do it for strings whose traits_type isn't std::char_traits<CharT>)

I, too, shall ponder some more!

Cheers