Re: [osg-users] pragma warnings disabled in headers
Hi Wojtek, On Tue, Nov 25, 2008 at 11:39 AM, Wojciech Lewandowski [EMAIL PROTECTED] wrote: But the other side is that proper fixing would probably require modifying hundreds of OSG headers. I doubt that there is anyone brave enough to accept this time consuming task and I doubt Robert will authorize this move. Proper fixing for quite a few warnings requires fixing the compiler... I'm for gradually tightening up on the sensible warnings that are produced. I'm against a wholesale warnings purge. You really need good engineers to do warnings fixes, as it's very easy to be be lured into think there is problem in the code and then fixing and breaking it in the process. Another danger is that by fixing a warning is that the code can become more obfusticated and more difficult to maintain which leads to more bugs in the long term. This is why you need a engineer that really understands C++, what the actual OSG code is attempting to do, and the idiosyncrasies of compilers and to see to the bottom of all warnings in what they really mean in terms of compiled code and its reliability. I know that some groups have a non warnings policy in their projects, but I consider this a bad policy as it can lead to chasing false positives, fixing things that aren't broken and reducing software quality as a consequence. Robert. ___ osg-users mailing list osg-users@lists.openscenegraph.org http://lists.openscenegraph.org/listinfo.cgi/osg-users-openscenegraph.org
Re: [osg-users] pragma warnings disabled in headers
Hi Guys, I generally agree that suppressing these warnings is bad. Truth is if they were not disabled at the early days of OSG most of them would be not present today because authors would fix them in other way. And this poses a danger for the future. As along as we keep them disabled authors will produce more and more of these warnings. But the other side is that proper fixing would probably require modifying hundreds of OSG headers. I doubt that there is anyone brave enough to accept this time consuming task and I doubt Robert will authorize this move. It an impasse in which we have to choose between bad and worse. The good thing though, is that its up to you which one you consider worse ;-) Wojtek Hi Paul, Wojtek's change only restores the OSG to how it's been compiling since the very early days w.r.t disabling the least usefu/most distractingl of the MSVC warnings. You can disable this via CMake, so that the OSG doesn't suppress any warnings. Fixing warnings is desirable, but has to be done with care as doing so can introduce bugs - this has happened to the OSG in the past. Warnings do often bring up ambiguities in the code that the author didn't spot - which is good, but they also produce false positives - highlighting code that is absolutely correct for it's intended usage, but confuses developer into thinking that something is wrong and fixing something that isn't broken, or just obfuscates the real warnings amoungst the sea of false positives. For this reason I'm cautious about diving in and doing a warnings purge. Robert. On Mon, Nov 24, 2008 at 9:37 PM, Paul Martz [EMAIL PROTECTED] wrote: Hi all -- Sorry I'm coming into this a little late. What I'd really like is a clean compile so that if I introduce some suspicious code, I clearly see a new warning message displayed in the output, so that I can investigate it. What I don't want is to have all warnings disabled so that I never see a message about my bogus change. If the warning is displayed, but I miss it because it's buried in hundreds or thousands of other warnings unrelated to my code change, then that's just as bad as having it disabled and not displayed at all. Perhaps the middle ground is to place the #pagma's in the CMake generated include/osg/Config file? This seems like the worst solution, because not only does it disable all warnings in the OSG headers, it also disables it in my own application code that directly or indirectly includes osg/Config. If I change code and introduce a warning, I'll never see the warning message displayed because osg/Config disabled it. The other approach is to do an explicit casts to avoid the warnings in the first place. This is more wordy and while of dubious practical value would at least fix the warnings. Yes, this is probably the best solution, but is a prohibitive task to bite off for a large existing code base. A third solution, as Wojtek mentioned, is to wrap each OSG header file with #pragma push/pop so that warnings can be disabled just in the header, and not in user code that includes the header. This could be done by changing all OSG headers to include CMake-generated prefix and suffix files that contain the push/pops and disables. This is still an undesirable solution because it turns all warnings off in OSG headers, even for future code changes, but is no worse than what we had before and is relatively easy to implement. -Paul ___ osg-users mailing list osg-users@lists.openscenegraph.org http://lists.openscenegraph.org/listinfo.cgi/osg-users-openscenegraph.org ___ osg-users mailing list osg-users@lists.openscenegraph.org http://lists.openscenegraph.org/listinfo.cgi/osg-users-openscenegraph.org ___ osg-users mailing list osg-users@lists.openscenegraph.org http://lists.openscenegraph.org/listinfo.cgi/osg-users-openscenegraph.org
Re: [osg-users] pragma warnings disabled in headers
Hi Robert,
I have restored MSVC disabled warnings in osg/Export. Difference is they are
now disabled only when OSG_DISABLE_MSVC_WARNINGS macro is defined. This
macro is set through CMake options and autogenerated in osg/Config. Simon
suggested that it would be cool if we had more control over selected
warnings. I tried to learn how to make selection of individual warning
numbers possible, but had to gave up as my cmake skills were not sufficient.
The only way I saw this possible would be adding one define for each MSVC
warning number. But many definitions seemed too be to much clutter for
osg/Config file so I rejected thar idea. For this it would be cool if
autogenerated Config entries could more powerful than simple #define/#undef
flags. Maybe Cmake gurus know how to do it.
I have not reverted added Compiler options. I assume that one may want to
have warnings enabled for the application but may not want to see them while
OSG libraries and examples compile.
Modified files:
osg/Export - now explicitly includes osg/Config to make sure
OSG_DISABLE_MSVC_WARNINGS is read
osg/Config.in - declares OSG_DISABLE_MSVC_WARNINGS flag to be added to
autogenerated osg/Config
CMakeLists.txt - declares OSG_DISABLE_MSVC_WARNINGS as option with default
ON setting
Cheers,
Wojtek
Config.in
Description: Binary data
IF(WIN32)
CMAKE_MINIMUM_REQUIRED(VERSION 2.4.6 FATAL_ERROR)
ELSE(WIN32)
IF(APPLE)
CMAKE_MINIMUM_REQUIRED(VERSION 2.6.0 FATAL_ERROR)
IF(${CMAKE_MAJOR_VERSION} EQUAL 2 AND ${CMAKE_MINOR_VERSION} EQUAL 4 AND
${CMAKE_PATCH_VERSION} LESS 7)
MESSAGE(Warning: A critical CMake bug exists in 2.4.6 and below. Trying
to build Universal Binaries will result in a compile error that seems unrelated. Either
avoid building Universal Binaries by changing the CMAKE_OSX_ARCHITECTURES field to list
only your architecture, or upgrade to the current CVS version of CMake or a newer stable
version if it exists.)
ENDIF(${CMAKE_MAJOR_VERSION} EQUAL 2 AND ${CMAKE_MINOR_VERSION} EQUAL 4
AND ${CMAKE_PATCH_VERSION} LESS 7)
ELSE(APPLE)
CMAKE_MINIMUM_REQUIRED(VERSION 2.4.0 FATAL_ERROR)
ENDIF(APPLE)
ENDIF(WIN32)
if(COMMAND cmake_policy)
# Works around warnings libraries linked against that don't
# have absolute paths (e.g. -lpthreads)
cmake_policy(SET CMP0003 NEW)
# Works around warnings about escaped quotes in ADD_DEFINITIONS
# statements.
cmake_policy(SET CMP0005 NEW)
# cmake-2.6.1 introduces policy cmp0008 decide how to treat full path
libraries that do not appear to be valid library file names
# quote from cvslog Such libraries worked by accident in the VS IDE and Xcode
generators in CMake 2.4 and below.
if(POLICY CMP0008)
cmake_policy(SET CMP0008 OLD)
endif(POLICY CMP0008)
endif(COMMAND cmake_policy)
PROJECT(OpenSceneGraph)
SET(OPENSCENEGRAPH_MAJOR_VERSION 2)
SET(OPENSCENEGRAPH_MINOR_VERSION 7)
SET(OPENSCENEGRAPH_PATCH_VERSION 6)
SET(OPENSCENEGRAPH_SOVERSION 50)
# set to 0 when not a release candidate, non zero means that any generated
# svn tags will be treated as release candidates of given number
SET(OPENSCENEGRAPH_RELEASE_CANDIDATE 0)
SET(OPENSCENEGRAPH_VERSION
${OPENSCENEGRAPH_MAJOR_VERSION}.${OPENSCENEGRAPH_MINOR_VERSION}.${OPENSCENEGRAPH_PATCH_VERSION})
SET(OSG_PLUGINS osgPlugins-${OPENSCENEGRAPH_VERSION})
SET(OSG_PLUGIN_PREFIX )
IF (CYGWIN)
SET(OSG_PLUGIN_PREFIX cygwin_)
ENDIF(CYGWIN)
IF(MINGW)
SET(OSG_PLUGIN_PREFIX mingw_)
ENDIF(MINGW)
# We want to build SONAMES shared librariess
SET(OPENSCENEGRAPH_SONAMES TRUE)
SET(OPENTHREADS_SONAMES TRUE)
SET(OpenThreads_SOURCE_DIR ${OpenSceneGraph_SOURCE_DIR})
# We have some custom .cmake scripts not in the official distribution.
# Maybe this can be used override existing behavior if needed?
SET(CMAKE_MODULE_PATH
${OpenSceneGraph_SOURCE_DIR}/CMakeModules;${CMAKE_MODULE_PATH})
# Mainly for Windows as a convenience. This will find a directory in parallel
with the
# OSG source that contains 3rd party headers and libraries.
# Use of relative paths in CMake is ill-advised, but don't know of any
alternatives in this case
#SET(CMAKE_INCLUDE_PATH
${OpenSceneGraph_SOURCE_DIR}/../3rdParty/include;${CMAKE_INCLUDE_PATH})
#SET(CMAKE_LIBRARY_PATH
${OpenSceneGraph_SOURCE_DIR}/../3rdParty/lib;${CMAKE_LIBRARY_PATH})
IF(USING_OSG_OP_OT_TRIPLE_SET)
SET(CMAKE_INCLUDE_PATH
${OpenSceneGraph_SOURCE_DIR}/../../3rdParty/include;${CMAKE_INCLUDE_PATH})
SET(CMAKE_LIBRARY_PATH
${OpenSceneGraph_SOURCE_DIR}/../../3rdParty/lib;${CMAKE_LIBRARY_PATH})
ENDIF(USING_OSG_OP_OT_TRIPLE_SET)
# Okay, here's the problem: On some platforms, linking against OpenThreads
# is not enough and explicit linking to the underlying thread library
# is also required (e.g. FreeBSD). But OpenThreads may be built with different
# backends (Pthreads, Sproc, Windows) so we don't know what the underlying
# thread library is because some platforms support multiple backends (e.g.
# IRIX
Re: [osg-users] pragma warnings disabled in headers
I thought a push and pop pragma would work ( I use that in my code) , but that would be in the headers which Robert wants to avoid Gordon __ Gordon Tomlinson Product Manager 3D Email : gtomlinson @ overwatch.textron.com __ (C): (+1) 571-265-2612 (W): (+1) 703-437-7651 Self defence is not a function of learning tricks but is a function of how quickly and intensely one can arouse one's instinct for survival - Master Tambo Tetsura -Original Message- From: [EMAIL PROTECTED] [mailto:[EMAIL PROTECTED] On Behalf Of Paul Martz Sent: Monday, November 24, 2008 4:38 PM To: 'OpenSceneGraph Users' Subject: Re: [osg-users] pragma warnings disabled in headers Hi all -- Sorry I'm coming into this a little late. What I'd really like is a clean compile so that if I introduce some suspicious code, I clearly see a new warning message displayed in the output, so that I can investigate it. What I don't want is to have all warnings disabled so that I never see a message about my bogus change. If the warning is displayed, but I miss it because it's buried in hundreds or thousands of other warnings unrelated to my code change, then that's just as bad as having it disabled and not displayed at all. Perhaps the middle ground is to place the #pagma's in the CMake generated include/osg/Config file? This seems like the worst solution, because not only does it disable all warnings in the OSG headers, it also disables it in my own application code that directly or indirectly includes osg/Config. If I change code and introduce a warning, I'll never see the warning message displayed because osg/Config disabled it. The other approach is to do an explicit casts to avoid the warnings in the first place. This is more wordy and while of dubious practical value would at least fix the warnings. Yes, this is probably the best solution, but is a prohibitive task to bite off for a large existing code base. A third solution, as Wojtek mentioned, is to wrap each OSG header file with #pragma push/pop so that warnings can be disabled just in the header, and not in user code that includes the header. This could be done by changing all OSG headers to include CMake-generated prefix and suffix files that contain the push/pops and disables. This is still an undesirable solution because it turns all warnings off in OSG headers, even for future code changes, but is no worse than what we had before and is relatively easy to implement. -Paul ___ osg-users mailing list osg-users@lists.openscenegraph.org http://lists.openscenegraph.org/listinfo.cgi/osg-users-openscenegraph.or g ___ osg-users mailing list osg-users@lists.openscenegraph.org http://lists.openscenegraph.org/listinfo.cgi/osg-users-openscenegraph.org
Re: [osg-users] pragma warnings disabled in headers
I thought a push and pop pragma would work ( I use that in my code) , but that would be in the headers which Robert wants to avoid I'm usually opposed to push/pop too -- not because they're in the headers, but because they disable displaying warnings for a block of code, and therefore prevent a developer from being alerted when he changes the code and introduces a legitimate issue that should be resolved. However, I'd let push/pop slide in this case, as it restricts disabling the warnings to the OSG headers themselves (as opposed to also disabling them for application code that includes the headers). It's really the only practical solution for large extant projects that display a great number of probably harmless warnings. Push/pop can be hidden with CMake-generated prefix and suffix files, just as the current svn head hides them in osg/Config. -Paul ___ osg-users mailing list osg-users@lists.openscenegraph.org http://lists.openscenegraph.org/listinfo.cgi/osg-users-openscenegraph.org
Re: [osg-users] pragma warnings disabled in headers
2008/11/21 Wojciech Lewandowski [EMAIL PROTECTED]: Hi, Guys Some middle step solution could be CMake define which would activate warning disable in osg\Export. Would this be acceptable ? What do other OSG windows developers think ? Sounds good to me. Though perhaps we should go whole hog and make it so each individual warning can be enabled or disabled. If you want to post a modified CMakeLists.txt here I'll give it a test. btw, had you ever noticed the /FI option? I didn't notice it till I was looking at the warning stuff; could be handy. -- The truth is out there. Usually in header files. ___ osg-users mailing list osg-users@lists.openscenegraph.org http://lists.openscenegraph.org/listinfo.cgi/osg-users-openscenegraph.org
Re: [osg-users] pragma warnings disabled in headers
\OpenSceneGraph\include\osg/Matrixd(601) : warning C4244: 'argument' : conversion from 'const osg::Matrixd::value_type' to 'osg::Vec3f::value_type', possible loss of data 24C:\dev\OSG_SVN\OpenSceneGraph\include\osg/Matrixd(617) : warning C4244: 'argument' : conversion from 'const osg::Matrixd::value_type' to 'osg::Vec4f::value_type', possible loss of data 24C:\dev\OSG_SVN\OpenSceneGraph\include\osg/Matrixd(617) : warning C4244: 'argument' : conversion from 'const osg::Matrixd::value_type' to 'osg::Vec4f::value_type', possible loss of data 24C:\dev\OSG_SVN\OpenSceneGraph\include\osg/Matrixd(617) : warning C4244: 'argument' : conversion from 'const osg::Matrixd::value_type' to 'osg::Vec4f::value_type', possible loss of data 24C:\dev\OSG_SVN\OpenSceneGraph\include\osg/Matrixd(617) : warning C4244: 'argument' : conversion from 'const osg::Matrixd::value_type' to 'osg::Vec4f::value_type', possible loss of data 24C:\dev\OSG_SVN\OpenSceneGraph\include\osg/Matrixd(632) : warning C4244: 'argument' : conversion from 'const osg::Matrixd::value_type' to 'osg::Vec4f::value_type', possible loss of data 24C:\dev\OSG_SVN\OpenSceneGraph\include\osg/Matrixd(632) : warning C4244: 'argument' : conversion from 'const osg::Matrixd::value_type' to 'osg::Vec4f::value_type', possible loss of data 24C:\dev\OSG_SVN\OpenSceneGraph\include\osg/Matrixd(632) : warning C4244: 'argument' : conversion from 'const osg::Matrixd::value_type' to 'osg::Vec4f::value_type', possible loss of data - Original Message - From: Simon Hammett To: OpenSceneGraph Users Sent: Friday, November 07, 2008 5:35 PM Subject: Re: [osg-users] pragma warnings disabled in headers 2008/11/7 Robert Osfield [EMAIL PROTECTED] On Fri, Nov 7, 2008 at 3:42 PM, Simon Hammett [EMAIL PROTECTED] wrote: I agree with Peter, arbitrarily turning off peoples warnings isn't good practice. The OSG generally doesn't disable warnings, warning disabling is only done on VS as it's had a history of producing lots of warnings on correct code. It's a number of years since I've used Windows. I do occassionally up the warning levels on the OSG via's our Cmake's options for this, this can help you spot some useful mistakes, but it also flags lots of warnings that mislead more than they inform so you have to enable them with an eye to what the warnings really mean - I do occassionally see fixes to warnings that while well meaning actually break code. If it's possible to get VS to disable warnings via compiler options rather than in source #pragma then this would be the best solution, it's what we have under gcc. It is possible to disable via the command line. I'll have a look into modifying the cmake stuff, I've got a couple of things to submit as well, so I'll post to submissions once I'm done. -- The truth is out there. Usually in header files. ___ osg-users mailing list osg-users@lists.openscenegraph.org http://lists.openscenegraph.org/listinfo.cgi/osg-users-openscenegraph.org ___ osg-users mailing list osg-users@lists.openscenegraph.org http://lists.openscenegraph.org/listinfo.cgi/osg-users-openscenegraph.org ___ osg-users mailing list osg-users@lists.openscenegraph.org http://lists.openscenegraph.org/listinfo.cgi/osg-users-openscenegraph.org ___ osg-users mailing list osg-users@lists.openscenegraph.org http://lists.openscenegraph.org/listinfo.cgi/osg-users-openscenegraph.org
Re: [osg-users] pragma warnings disabled in headers
2008/11/7 Robert Osfield [EMAIL PROTECTED] Hi Peter, On Fri, Nov 7, 2008 at 12:11 PM, Peter Wraae Marino [EMAIL PROTECTED] wrote: there is no push/pop for the warning in the header file which will make these warnings disabled in my own code too. I would like to catch this warnings in my own code and I shouldn't be forced to enabling them. the only reason I see for having them grouped here in the header is a convience method so osg doesn't have to disable them all places that gives these warnings? is this correct? or is there a better reason? The warnings that are disabled are ones that were deemed to be not useful and a hindrance to spotting actual useful warnings. The placement in the header is to avoid all OSG code have it's own localise pragma - all of which are platform specific. With the OSG I try to keep platform specific hacks self contained rather than distributed out across all code. If you really must have the warnings that perhaps one could add a CMake variable for configuring whether to disable these warnings or not. This would need to be a VS specific variable. Robert. ___ osg-users mailing list osg-users@lists.openscenegraph.org http://lists.openscenegraph.org/listinfo.cgi/osg-users-openscenegraph.org I agree with Peter, arbitrarily turning off peoples warnings isn't good practice. Of course it's the sort of thing people can waste hours arguing about, but some of the those warnings are useful if not important: eg 4996 // 'x': was declared deprecated If somebody is going to make the effort of deprecating stuff, people should know about it. Still there's an easy solution, include the osg headers in your precompiled header and surround them with push/pop... Saves mucking around with osgs config. -- The truth is out there. Usually in header files. ___ osg-users mailing list osg-users@lists.openscenegraph.org http://lists.openscenegraph.org/listinfo.cgi/osg-users-openscenegraph.org
Re: [osg-users] pragma warnings disabled in headers
On Fri, Nov 7, 2008 at 3:42 PM, Simon Hammett [EMAIL PROTECTED] wrote: I agree with Peter, arbitrarily turning off peoples warnings isn't good practice. The OSG generally doesn't disable warnings, warning disabling is only done on VS as it's had a history of producing lots of warnings on correct code. It's a number of years since I've used Windows. I do occassionally up the warning levels on the OSG via's our Cmake's options for this, this can help you spot some useful mistakes, but it also flags lots of warnings that mislead more than they inform so you have to enable them with an eye to what the warnings really mean - I do occassionally see fixes to warnings that while well meaning actually break code. If it's possible to get VS to disable warnings via compiler options rather than in source #pragma then this would be the best solution, it's what we have under gcc. Robert. ___ osg-users mailing list osg-users@lists.openscenegraph.org http://lists.openscenegraph.org/listinfo.cgi/osg-users-openscenegraph.org
Re: [osg-users] pragma warnings disabled in headers
Hi Peter, On Fri, Nov 7, 2008 at 12:11 PM, Peter Wraae Marino [EMAIL PROTECTED] wrote: there is no push/pop for the warning in the header file which will make these warnings disabled in my own code too. I would like to catch this warnings in my own code and I shouldn't be forced to enabling them. the only reason I see for having them grouped here in the header is a convience method so osg doesn't have to disable them all places that gives these warnings? is this correct? or is there a better reason? The warnings that are disabled are ones that were deemed to be not useful and a hindrance to spotting actual useful warnings. The placement in the header is to avoid all OSG code have it's own localise pragma - all of which are platform specific. With the OSG I try to keep platform specific hacks self contained rather than distributed out across all code. If you really must have the warnings that perhaps one could add a CMake variable for configuring whether to disable these warnings or not. This would need to be a VS specific variable. Robert. ___ osg-users mailing list osg-users@lists.openscenegraph.org http://lists.openscenegraph.org/listinfo.cgi/osg-users-openscenegraph.org
Re: [osg-users] pragma warnings disabled in headers
2008/11/7 Robert Osfield [EMAIL PROTECTED] On Fri, Nov 7, 2008 at 3:42 PM, Simon Hammett [EMAIL PROTECTED] wrote: I agree with Peter, arbitrarily turning off peoples warnings isn't good practice. The OSG generally doesn't disable warnings, warning disabling is only done on VS as it's had a history of producing lots of warnings on correct code. It's a number of years since I've used Windows. I do occassionally up the warning levels on the OSG via's our Cmake's options for this, this can help you spot some useful mistakes, but it also flags lots of warnings that mislead more than they inform so you have to enable them with an eye to what the warnings really mean - I do occassionally see fixes to warnings that while well meaning actually break code. If it's possible to get VS to disable warnings via compiler options rather than in source #pragma then this would be the best solution, it's what we have under gcc. It is possible to disable via the command line. I'll have a look into modifying the cmake stuff, I've got a couple of things to submit as well, so I'll post to submissions once I'm done. -- The truth is out there. Usually in header files. ___ osg-users mailing list osg-users@lists.openscenegraph.org http://lists.openscenegraph.org/listinfo.cgi/osg-users-openscenegraph.org
