Re: [lmi] still MSVC compilation problems

[Greg Chicares]

On 2008-06-22 13:14Z, Vadim Zeitlin wrote:
> On Sun, 22 Jun 2008 03:21:04 +0000 Greg Chicares <address@hidden> wrote:
> 
> GC> On 2008-06-22 02:11Z, Vadim Zeitlin wrote:
> GC> [snip patch]
> GC> > 
> GC> > What do you think about this? It does, of course, use an ugly untyped 
> get()
> GC> > function and correspondingly uses static_cast to the right type when we
> GC> > need to access it later but it doesn't look to be much worse to me than 
> the
> GC> > existing static_cast to holder_type
> GC> 
> GC> I need to look at this when I'm more awake. At first glance, I somewhat
> GC> balk at (void*).
> 
>  Yes, using untyped pointer like this is certainly dangerous and normally
> unacceptable. However the question is here is whether it's really so much
> worse than the current static cast from placeholder (which is basically an
> untyped pointer too) to holder<ClassType,pmd_type>. In both cases we know
> that the cast is safe because of the previous comparison of type() with
> pmd_type and I don't think the existing static_cast<> is really "safer".
> I.e. both are safe when used after this comparison in exact_cast() even if
> both are unsafe otherwise. So I'd like to make the argument that the
> existing code is about the same in terms of safety but just hides the
> potential problem.

I think your analysis is correct technically. However, whatever
can be expressed within C++'s strong type system, IMO should be,
and I'm extremely prejudiced against void* in particular. If no
compiler needed this workaround, I'd never apply it. Therefore,
I walled it off in msvc-only conditionals, and gave the getter
an unpleasant name.

Conditionals do create extra paths through the code, but in
this case I'm not much concerned about maintenance: this code
rarely needs to change, and someday msvc might even conform to
the standard so that we can expunge the conditionalized code.

> Also, placeholder is a private class (and IMO should be
> defined inside some "namespace Private" or "namespace impl") so the unsafe
> "void *get()" shouldn't be used from anywhere else.

I know boost does that frequently, and I even copied their
practice for a while. But I just don't see how this makes
anything really safer: one can so easily write 'impl::get()'.

> GC> >  Finally, I still have unit test failures after this change:
> GC> [...]
> GC> > but they seem to be due to the difference between the expected and real
> GC> > value of std::type::info::name() so I think I should just add tests for
> GC> > MSVC into any_member_test.cpp along the existing tests for g++ version. 
> Do
> GC> > you see anything wrong with this?
> GC> 
> GC> Nothing wrong with that. Testing the value of std::type::info::name()
> GC> is fragile, but we're already doing it for other compilers, and I'd
> GC> rather not spend time thinking about removing that fragility right now.
> 
>  In fact I think it's easy enough to remove this fragility: instead of
> hardcoding the results of std::type_info::name() in the test code we could
> construct the string dynamically using lmi::TypeInfo(). Then there would be
> no need for any compiler [version] tests. If you don't object I'd like to
> do this.

This sounds like an interesting idea. Let me suggest you show a
concrete example of such a change first (if that's easier than
doing the whole thing), just so I can see how it looks. I might
like it, or I might not, so I wouldn't want to ask you to do a
lot of work until I can see what it would look like.