Re: [boost] [mpl] bug: mpl::is_sequence< mpl::int_<1> > fails to compile

From: Tobias Schwinger (tschwinger_at_[hidden])
Date: 2006-05-08 16:53:42

David Abrahams wrote:
> Tobias Schwinger <tschwinger_at_[hidden]> writes:
> 
>>The guessing stuff directly in 'is_sequence' is a workaround for
>>broken compilers. For proper compilers:
> 
>>  is_sequence<T> <=> is_same< begin<T>, void_ >

Was actually supposed to mean 'is_same<begin<T>::type,void_>' or just 
assume a placeholder expression around it...
 
>>. So 'is_sequence' works perfectly well and can also work perfectly
>>reliable given that 'begin' does; 'begin', however, fails to compile
>>if there is a 'tag' member for a non-sequence type instead of
>>returning 'void_' which would be the documented behaviour.
> 
> And begin can be spoofed, too, can't it?  Okay, it's very unlikely.
> 

Depends. If that guessing T::begin shortcut is gone it can't -- unless 
you call implementing an incomplete sequence (specializing 'begin_impl') 
spoofing.

> 
>>The reason is "guessing mess" in 'begin_impl': the primary template
>>attempts to return 'T::begin' (it seems to be a convenience feature so
>>a sequence implementor can simply specify a type member called 'begin'
>>and does not need to specialize 'begin_impl'). 'sequence_tag' is
>>assumed to sort out non-sequences -- but it is impossible without
>>guessing (or adding to the user's responsibilities when registering
>>non-sequence types for tag dispatch).
> 
> Exactly.
> 
>>>>>>I don't see why it is impossible. MPL sequences can always
>>>>>>specialize is_sequence<T>. is_sequence *can* work reliably
>>>>>>100% of the time:
>>>>>>template <class S> struct is_sequence : mpl::false_ {}; //
>>>>>>default
>>>>>>// your specializations here
>>
>>That should not even be needed -- implementing 'begin' should be
>>enough...
> 
> I am not thinking about this problem freshly today; all I remember is
> that it's a morasse with no good answers.  But I could be wrong.  

> If you can supply a patch that works, and you can demonstrate it with
> convincing tests, I'll be happy to recommend that Aleksey commit it.
> 

Well, if that's all you're worried about:

I supplied a patch that works and a testcase in the OP (and the 
assertions from the documentation pass, too). 
I'm currently using Fusion on top of it and I didn't encounter any 
problems. It shouldn't break things more than they're already broken, 
still it only mutes the symptoms of the underlying problem.

A proper solution (e.g. as proposed at the end of my previous post) 
means significant changes to the library so that I wouldn't want to 
start coding until you (plural, authors of MPL) agree that the 
proposed changes are desirable.

> 
>>>>>you can probably make it vanishingly unlikely that some random
>>>>>third-party type will pass all the default is_sequence tests, but
>>>>>you can never make it 100% impossible as long as:
>>>>>
>>>>>a. sequences can be arbitrary types, such as void(int,long,char)
>>>>>b. is_sequence<S> reporting correctly is not part of the sequence
>>>>>requirements
> 
>>c. begin works properly
>> 
>> 
>>'begin<non_sequence>::type' being 'void_' (maybe more importantly that
>>it compiles) is already part of the requirements...
> 
> Yes but it *totally* begs the question.  Because, what do you have to
> know in order to decide whether begin<x> should return void_?  That's
> right, you have to know whether x is a sequence.
> 

Simple:

You gotta implement 'begin' when implementing a sequence. If it's not 
implemented it's not a sequence. The requirement to return 'void_' for 
non-sequences forbids "overloading" that metafunction for non-sequence 
types.

> 
>>>>I believe that the default is_sequence should strictly be
>>>>mpl::false_: no guessing games.
>>>>I think:
>>>>a) if you want to make specific types (e.g. RT(A0,A1,...AN)),
>>>>a sequence, then specialize is_sequence for those.
>>> 
>>>ODR violations ensue.  int(int,long) is a sequence in one TU but not
>>>in another.
>> 
>>Just curious: all these classes should have internal linkage - am I
>>missing something?
> 
> foo<is_sequence<int(int,long)>::value>::bar() calls one function in TU
> #1 and another function in TU #2.  

True. foo<true> and foo<false> are two different classes...

> Clear yet?

Sorry, not quite. Where is the redefinition?

> 
>>So here's my €0,02-plan:
>>-- require 'begin_impl' to be specialized; IOW drop that "type member
>>convenience stuff" (described above) -- it is what makes the dispatch
>>a mess!  
>>-- throw out 'sequence_tag' (it doesn't work, can't work and isn't 
>>needed) 
>>-- keep 'is_sequence' ;-)
> 
> Show me the money. Err, the code.
> 

Please think and comment on it first (plural again, includes Aleksey). 

The patch I posted works for me, right now. Getting the underlying flaw
straightened out is primarily *your* business (I'll probably implement
it iff there is a clear demand).

Regards,

Tobias