Uploaded image for project: 'Data Management'
  1. Data Management
  2. DM-5394

Investigate boost compiler warnings and update boost to v1.60

    XMLWordPrintable

    Details

    • Story Points:
      2
    • Team:
      Architecture

      Description

      As reported in comments in DM-1304 clang now triggers many warnings with Boost v1.59:

      /Users/rowen/UW/LSST/lsstsw/stack/DarwinX86/boost/1.59.lsst5/include/boost/archive/detail/check.hpp:148:5: warning: unused typedef 'STATIC_WARNING_LINE148' [-Wunused-local-typedef]
          BOOST_STATIC_WARNING(typex::value);
          ^
      /Users/rowen/UW/LSST/lsstsw/stack/DarwinX86/boost/1.59.lsst5/include/boost/serialization/static_warning.hpp:100:33: note: expanded from macro 'BOOST_STATIC_WARNING'
      #define BOOST_STATIC_WARNING(B) BOOST_SERIALIZATION_BSW(B, __LINE__)
                                      ^
      /Users/rowen/UW/LSST/lsstsw/stack/DarwinX86/boost/1.59.lsst5/include/boost/serialization/static_warning.hpp:99:7: note: expanded from macro 'BOOST_SERIALIZATION_BSW'
          > BOOST_JOIN(STATIC_WARNING_LINE, L) BOOST_STATIC_ASSERT_UNUSED_ATTRIBUTE; 
            ^
      /Users/rowen/UW/LSST/lsstsw/stack/DarwinX86/boost/1.59.lsst5/include/boost/config/suffix.hpp:544:28: note: expanded from macro 'BOOST_JOIN'
      #define BOOST_JOIN( X, Y ) BOOST_DO_JOIN( X, Y )
                                 ^
      /Users/rowen/UW/LSST/lsstsw/stack/DarwinX86/boost/1.59.lsst5/include/boost/config/suffix.hpp:545:31: note: expanded from macro 'BOOST_DO_JOIN'
      #define BOOST_DO_JOIN( X, Y ) BOOST_DO_JOIN2(X,Y)
                                    ^
      /Users/rowen/UW/LSST/lsstsw/stack/DarwinX86/boost/1.59.lsst5/include/boost/config/suffix.hpp:546:32: note: expanded from macro 'BOOST_DO_JOIN2'
      #define BOOST_DO_JOIN2( X, Y ) X##Y
                                     ^
      <scratch space>:25:1: note: expanded from here
      STATIC_WARNING_LINE148
      ^
      

      v1.60 is the current version so we should see if these warnings have been fixed in that version.

        Attachments

          Issue Links

            Activity

            Hide
            swinbank John Swinbank added a comment - - edited

            An alternative I've previously suggested is that we include headers from external packages like Boost using -isystem rather than -I. The GCC manual suggests -isystem should be used for "vendor supplied" headers, and GCC will suppress warnings encountered within them.

            The following patch should enable -isystem for any packages which (like boost) use lsst.sconsUtils.ExternalConfiguration in their .cfg file, but not for the code we're writing (which use lsst.sconsUtils.Configuration).

            --- a/python/lsst/sconsUtils/dependencies.py
            +++ b/python/lsst/sconsUtils/dependencies.py
            @@ -73,10 +73,11 @@ def configure(packageName, versionString=None, eupsProduct=None, eupsProductPath
                 # XCPPPATH is a new variable defined by sconsUtils - it's like CPPPATH, but the headers
                 # found there aren't treated as dependencies.  This can make scons a lot faster.
                 state.env['XCPPPATH'] = []
            +    state.env['XCPPPREFIX'] = "-isystem "
                 state.env['_CPPINCFLAGS'] = \
                     "$( ${_concat(INCPREFIX, CPPPATH, INCSUFFIX, __env__, RDirs, TARGET, SOURCE)}"\
            -        " ${_concat(INCPREFIX, XCPPPATH, INCSUFFIX, __env__, RDirs, TARGET, SOURCE)} $)"
            -    state.env['_SWIGINCFLAGS'] = state.env['_CPPINCFLAGS'].replace("CPPPATH", "SWIGPATH")
            +        " ${_concat(XCPPPREFIX, XCPPPATH, INCSUFFIX, __env__, RDirs, TARGET, SOURCE)} $)"
            +    state.env['_SWIGINCFLAGS'] = state.env['_CPPINCFLAGS'].replace("CPPPATH", "SWIGPATH").replace("XCPPPREFIX", "SWIGINCPREFIX")
             
                 if state.env.linkFarmDir:
                     for d in [state.env.linkFarmDir, "#"]:
            

            I should add that when we discussed this previously this suggestion was rejected, and I've no wish to re-open old discussions. However, it's worth mentioning that I've been running with this patch in place on my local system for about a year; it's never caused me any problems, and the freedom from pointless warnings has been a real blessing.

            Show
            swinbank John Swinbank added a comment - - edited An alternative I've previously suggested is that we include headers from external packages like Boost using -isystem rather than -I . The GCC manual suggests -isystem should be used for "vendor supplied" headers, and GCC will suppress warnings encountered within them. The following patch should enable -isystem for any packages which (like boost) use lsst.sconsUtils.ExternalConfiguration in their .cfg file, but not for the code we're writing (which use lsst.sconsUtils.Configuration ). --- a/python/lsst/sconsUtils/dependencies.py +++ b/python/lsst/sconsUtils/dependencies.py @@ -73,10 +73,11 @@ def configure(packageName, versionString=None, eupsProduct=None, eupsProductPath # XCPPPATH is a new variable defined by sconsUtils - it's like CPPPATH, but the headers # found there aren't treated as dependencies. This can make scons a lot faster. state.env['XCPPPATH'] = [] + state.env['XCPPPREFIX'] = "-isystem " state.env['_CPPINCFLAGS'] = \ "$( ${_concat(INCPREFIX, CPPPATH, INCSUFFIX, __env__, RDirs, TARGET, SOURCE)}"\ - " ${_concat(INCPREFIX, XCPPPATH, INCSUFFIX, __env__, RDirs, TARGET, SOURCE)} $)" - state.env['_SWIGINCFLAGS'] = state.env['_CPPINCFLAGS'].replace("CPPPATH", "SWIGPATH") + " ${_concat(XCPPPREFIX, XCPPPATH, INCSUFFIX, __env__, RDirs, TARGET, SOURCE)} $)" + state.env['_SWIGINCFLAGS'] = state.env['_CPPINCFLAGS'].replace("CPPPATH", "SWIGPATH").replace("XCPPPREFIX", "SWIGINCPREFIX")   if state.env.linkFarmDir: for d in [state.env.linkFarmDir, "#"]: I should add that when we discussed this previously this suggestion was rejected, and I've no wish to re-open old discussions. However, it's worth mentioning that I've been running with this patch in place on my local system for about a year; it's never caused me any problems, and the freedom from pointless warnings has been a real blessing.
            Hide
            tjenness Tim Jenness added a comment -

            I just tried v1.60 and it fails to build in ndarray in vectorize.h.

            Show
            tjenness Tim Jenness added a comment - I just tried v1.60 and it fails to build in ndarray in vectorize.h .
            Hide
            tjenness Tim Jenness added a comment -

            Boost 1.60 fixes the boost compiler warnings in daf_persistence. Everything now builds properly with boost 1.60 (modulo the Qserv fix which is in review and will be merged soon). Shall I merge this?

            Show
            tjenness Tim Jenness added a comment - Boost 1.60 fixes the boost compiler warnings in daf_persistence . Everything now builds properly with boost 1.60 (modulo the Qserv fix which is in review and will be merged soon). Shall I merge this?
            Hide
            swinbank John Swinbank added a comment -

            The changes to Boost look fine; go ahead and merge assuming this keeps Jenkins happy.

            Per discussion on HipChat, we should ensure that https://github.com/ndarray/ndarray and https://github.com/lsst/ndarray are in sync. I don't think we need to close DM-2005 in order to mark this ticket as done, but could you PR the changes you've made to ndarray in this ticket against the upstream ndarray repository, please?

            Show
            swinbank John Swinbank added a comment - The changes to Boost look fine; go ahead and merge assuming this keeps Jenkins happy. Per discussion on HipChat, we should ensure that https://github.com/ndarray/ndarray and https://github.com/lsst/ndarray are in sync. I don't think we need to close DM-2005 in order to mark this ticket as done, but could you PR the changes you've made to ndarray in this ticket against the upstream ndarray repository, please?
            Hide
            tjenness Tim Jenness added a comment -

            Boost has been updated and I've cherry-picked some patches from our ndarray repo and made a PR for upstream: https://github.com/ndarray/ndarray/pull/42

            Show
            tjenness Tim Jenness added a comment - Boost has been updated and I've cherry-picked some patches from our ndarray repo and made a PR for upstream: https://github.com/ndarray/ndarray/pull/42

              People

              Assignee:
              tjenness Tim Jenness
              Reporter:
              tjenness Tim Jenness
              Reviewers:
              John Swinbank
              Watchers:
              John Swinbank, Robert Lupton, Russell Owen, Tim Jenness
              Votes:
              0 Vote for this issue
              Watchers:
              4 Start watching this issue

                Dates

                Created:
                Updated:
                Resolved:

                  Jenkins Builds

                  No builds found.