Opened 6 years ago

Closed 6 years ago

Last modified 6 years ago

#3959 closed defect (fixed)

cpl_port.h globally disables Visual C++ warnings

Reported by: mloskot Owned by: warmerdam
Priority: normal Milestone:
Component: ConfigBuild Version: svn-trunk
Severity: normal Keywords:
Cc:

Description

Long time ago, I committed r10310 which is an incomplete solution because this setting affects compilation of any software where cpl_port.h is included. Basically, including cpl_port.h implicitly affects warnings state what is unwelcome if one wants to compile his software with default or higher levels.

The warnings level should be modified locally and privately using

#if defined(_MSC_VER)
#  pragma warning(push)
#  pragma warning(disable:4127)
#endif

... code which causes C4127 warning

#if defined(_MSC_VER)
#  pragma warning(pop)
#endif

Change History (4)

comment:1 Changed 6 years ago by warmerdam

  • Component changed from default to ConfigBuild

I agree that it is inappropriate for cpl_port.h to end up disabling warnings for client applications. At the very least there could be some sort of opt-out #define that clients could declare (ie. #define CPL_DISABLE_MSVC_WARNING_SUPRESSION) that client applications could use though it really ought to work the other way around where some cpl definitions are only used by GDAL code.

I do not agree to that warnings should be supressed case by case in the code. I consider some of the MSVC warnings to be inappropriate for application to the GDAL code base and I don't want to supress them location by location. Perhaps there are compiler switches we could pass on the commandline for these supressions instead of doing them in cpl_port.h. I would be receptive to such changes.

comment:2 Changed 6 years ago by mloskot

I don't necessarily mean case by case as warning by warning and location by location. It can be still solved by wrapping large chunks of code, but not in public headers. If GDAL wants to disable warnings, no problem but no reason to put this setting in public headers. However, if some public headers need it, then more surgical precision is needed indeed using warning(push) and warning(pop) around problematic code.

I can't recommend any switches.

comment:3 Changed 6 years ago by warmerdam

  • Resolution set to fixed
  • Status changed from new to closed

In trunk I have migrated the warning disabling out of cpl_port.h and into the WARNFLAGS macro in nmake.opt (r21678). This should resolve the issue of inclusion of cpl_port.h disabling warnings in calling applications.

I'm also going to make a pass to try and clean warnings at /W4 on MSVC 2008 from the core GDAL source.

comment:4 Changed 6 years ago by mloskot

Thanks Frank.

Note: See TracTickets for help on using tickets.