louisdx / cxx-prettyprint

A header-only library for C++(0x) that allows automagic pretty-printing of any container.
http://louisdx.github.com/cxx-prettyprint/
Boost Software License 1.0
557 stars 73 forks source link

Fix for prettyprint98.hpp #1

Closed evandrix closed 12 years ago

evandrix commented 12 years ago

... include

louisdx commented 12 years ago

Sorry, this is wayyy to radical a change set for me to consider it for merging, and much worse, your change to the template signature of std::multiset isn't even correct. Sorry, and thank you for your interest!

evandrix commented 12 years ago

The template signature change worked on my machine.

louisdx commented 12 years ago

This is very strange. The standard mandates the template arguments, and they are as I wrote. I'm not sure whether the TR1 versions are allowed to be more relaxed and allow further, defaulted template arguments. I've never seen it happen in practice, but perhaps you have a library implementation that does this.

The correct thing to do would be to add another declaration that takes five template arguments.

(Please don't use double-underscores in names, as they are reserved.)

evandrix commented 12 years ago

Well, this is the error message I receive when trying to compile the demo cpp file:

/usr/include/c++/4.2.1/tr1/unordered_set:51: error: previous declaration
‘template<class _Value, class _Hash, class _Pred, class _Alloc, bool
__cache_hash_code> class std::tr1::unordered_set’ prettyprint98.h:19: error:
used 5 template parameter(s) instead of 4
/usr/include/c++/4.2.1/tr1/unordered_set:100: error: previous declaration
‘template<class _Value, class _Hash, class _Pred, class _Alloc, bool
__cache_hash_code> class std::tr1::unordered_multiset’ prettyprint98.h:20:
error: used 5 template parameter(s) instead of 4 make: **\* [prettyprint98.o]
Error 1
evandrix commented 12 years ago

g++ compiler info:

Target: i686-apple-darwin11
Configured with:
/private/var/tmp/llvmgcc42/llvmgcc42-2336.1~22/src/configure
--disable-checking --enable-werror --prefix=/Developer/usr/llvm-gcc-4.2
--mandir=/share/man --enable-languages=c,objc,c++,obj-c++
--program-prefix=llvm- --program-transform-name=/^[cg][^.-]*$/s/$/-4.2/
--with-slibdir=/usr/lib --build=i686-apple-darwin11
--enable-llvm=/private/var/tmp/llvmgcc42/llvmgcc42-2336.1~22/dst-llvmCore/Developer/usr/local
--program-prefix=i686-apple-darwin11- --host=x86_64-apple-darwin11
--target=i686-apple-darwin11 --with-gxx-include-dir=/usr/include/c++/4.2.1
Thread model: posix gcc version 4.2.1 (Based on Apple Inc. build 5658) (LLVM
build 2336.1.00)
evandrix commented 12 years ago

Original tr/unordered_set definition:

...template<class _Value,
       class _Hash = hash<_Value>,
       class _Pred = std::equal_to<_Value>,
       class _Alloc = std::allocator<_Value>,
       bool __cache_hash_code = false>
    class unordered_set

  template<class _Value,
       class _Hash = hash<_Value>,
       class _Pred = std::equal_to<_Value>,
       class _Alloc = std::allocator<_Value>,
       bool __cache_hash_code = false>
    class unordered_multiset...
evandrix commented 12 years ago

I was investigating this further, and believe this discrepancy is caused by using gcc version <=4.2, it works on my machine when i try gcc >= 4.3.

louisdx commented 12 years ago

I've had a look at the TR1 document, too, and that one also specifies the templates precisely way I have them. Moreover, I couldn't find any clause that allows implementations to add further, defaulted template parameters.

That said, I'm not opposed to a small change that adds another version with further template parameters, but I am certainly not going to merge a 1500+ lines change that replaces all my files.

evandrix commented 12 years ago

I tried just added a variant with 5 template parameters instead of 4, but it still triggered the compilation error that I've quoted of a different redefinition of a previous declaration. Not sure how to go with your suggestion, even though I'd be willing to, cause in the original source code for tr1/unordered_set, as I've quoted above, the last template parameter is actually bool ...=false (so there already is a default parameter value for this 5th template parameter, hence is your header file's definition really necessary? or please advise how best should I fix it for myself, other than changing my gcc version to >=4.3?

I'm saying this because I think the following is the right way to overload a function:

   template<class _Value,
       class _Hash = hash<_Value>,
       class _Pred = std::equal_to<_Value>,
       class _Alloc = std::allocator<_Value>,
       bool __cache_hash_code = false>
    class unordered_set

   template<class _Value,
       class _Hash = hash<_Value>,
       class _Pred = std::equal_to<_Value>,
       class _Alloc = std::allocator<_Value>>
    class unordered_set

but having these 2 as it were doesn't seem to compile for me without giving that error.

louisdx commented 12 years ago

Try adding something like this:

  // Pre-declarations of container types so we don't actually have to include the relevant headers if not needed, speeding up compilation time.
template<typename T, typename TComp, typename TAllocator> class set;
template<typename T, typename TComp, typename TAllocator, bool> class set;
template<typename T, typename TComp, typename TAllocator> class multiset;
template<typename T, typename TComp, typename TAllocator, bool> class multiset;

And then:

template<typename T, typename THash, typename TEqual, typename TAllocator, bool BCache>
struct delimiters< ::std::tr1::unordered_set<T, THash, TEqual, TAllocator, BCache>, char> { static const delimiters_values<char> values; };

template<typename T, typename THash, typename TEqual, typename TAllocator, BCache>
const delimiters_values<char> delimiters< ::std::tr1::unordered_set<T, THash, TEqual, TAllocator, BCache>, char>::values = { "{", ", ", "}" };

(And similarly for all the other cases.)