Re[2]: [lmi] still MSVC compilation problems

[Vadim Zeitlin]

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. 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.

GC> However:
GC> 
GC> > Would this be
GC> > acceptable
GC> 
GC> At a minimum, I'd need to test this with several compilers first.

 I tested this with MSVC and g++ 4.1. I can test it with other versions of
g++ but I really don't think this could break compilation with other
compilers, the code does the same thing as before only in a simpler way.

GC> > or, if not, could we at least use this version for MSVC while
GC> > keeping the old one for the other compilers?
GC> 
GC> I'm certainly willing to do it conditionally for msvc.
GC> 
GC> What macro would be appropriate for an msvc conditional?

 The macro defined by the compiler itself is _MSC_VER. We can also define
some LMI_COMPILER_MSVC in config_msvc.hpp if this can be helpful.

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.

 Thanks,
VZ