Hi JS,
On Fri, Jan 23, 2009 at 2:47 PM, Jean-Sébastien Guay
<[email protected]> wrote:
>> My inclination right now is to disable C4251, C4275 and C4244 and keep
>> them in the include/osg/Export header, and then address the rest of
>> the warnings.
>
> I thought the conclusion was to put the warning disable flags as
> compile-time options (in CFLAGS/CXXFLAGS) instead of in osg/Export, since
> then they would apply to user code too?
The 'ideal' solution w.r.t warnings would be to disable no warnings on
the headers and have them compile cleanly without any need for warning
disable, and only disable warnings for the implementations where the
warnings aren't helpful.
Given the nature of these warnings I'm not sure it would be possible
to fix the remaining warnings without lots of changes across many
headers, so the fallback of using headers. We might be able to fix
them without too many intrusive change though, so the door if we can
spot ways of fixing the warnings without modifiying hundreds of files.
I've now fixed the two of the sets of warnings I mentioned in my
previous email, so leaves just the float/double casting, and dll
linkgage related warnings:
#if defined(_MSC_VER) && defined(OSG_DISABLE_MSVC_WARNINGS)
#pragma warning( disable : 4244 )
#pragma warning( disable : 4251 )
#pragma warning( disable : 4275 )
#endif
>> Thoughts, suggestions?
>
> About C4244, I have no preference, if you don't want to add casts then go
> ahead and disable the warning.
At this time I think doing a purge of casts would introduce too many
opportunities for bugs to be introduced this close to a release. If
some one can fix these warnings and show that it's only half dozen or
so files that are changed then I'm open to merging these changes. I
won't embark on such a purge myself though as I have plenty of other
work do, and can't even reproduce the warnings myself so wouldn't be
able to check to see if things are fixed...
> About C4251 and C4275, couldn't this indicate that users would have problems
> when deriving classes from those classes? I think they essentially mean that
> there are missing OSG_EXPORT directives which are not necessary for direct
> OSG usage, but which could matter if the user were to derive classes. But I
> could be wrong.
>
> So I wonder if just adding a few extra OSG_EXPORT directives would remove
> these, especially if the warning comes up whenever ref_ptr is included... Or
> perhaps it could be disabled only for the ref_ptr header using push at the
> top and pop at the bottom (because we don't expect anyone to want to derive
> classes from ref_ptr, and the missing OSG_EXPORT never caused a problem
> before)?
>
> #pragma warning( push )
> #pragma warning( disable : 4251 )
> #pragma warning( disable : 4275 )
>
> // the rest of the ref_ptr header
>
> #pragma warning( pop )
>
> But other cases could be fixed by adding OSG_EXPORT where needed...
You (and others) are welcome to have a bash at trying to fix these
warnings, but it comes with the same qualification with the casting
task - right now we can't afford to change lots of OSG headers as each
change does introduce another chance for a typo bug to be accidentally
introduced.
Robert.
_______________________________________________
osg-users mailing list
[email protected]
http://lists.openscenegraph.org/listinfo.cgi/osg-users-openscenegraph.org
