Consider adding `-Wconversion` to the build flags

[fwyzard]

Should we consider adding -Wconversion to the build flags, to catch errors like floating point values being assigned to integers and silently truncated ?

We could first introduce it as -Wconversion, and once the warnings have been fixed switch to -Werror=conversion.

[fwyzard]

assign core

[cmsbuild]

New categories assigned: core

@Dr15Jones,@smuzaffar,@makortel you have been requested to review this Pull request/Issue and eventually sign? Thanks

[cmsbuild]

A new Issue was created by @fwyzard Andrea Bocci.

@Dr15Jones, @perrotta, @dpiparo, @rappoccio, @makortel, @smuzaffar, @qliphy can you please review it and eventually sign/assign? Thanks.

cms-bot commands are listed here

[smuzaffar]

sure we can, let me add this flag and test if for full cmssw via cmsdist PR

[smuzaffar]

tests for cms-sw/cmssw#8027 show over 1.7M warning, 500K are from cmssw code and most of these ( near 423K ) are coming from

  10875 /data/cmsbld/jenkins/workspace/ib-run-pr-tests/CMSSW_12_5_X_2022-08-17-2300/src/DataFormats/GeometrySurface/interface/LocalError.h:40:63: warning: conversion from 'double' to 'float' may change value [-Wfloat-conversion]
  10875 /data/cmsbld/jenkins/workspace/ib-run-pr-tests/CMSSW_12_5_X_2022-08-17-2300/src/DataFormats/GeometrySurface/interface/LocalError.h:40:73: warning: conversion from 'double' to 'float' may change value [-Wfloat-conversion]
  12060 /data/cmsbld/jenkins/workspace/ib-run-pr-tests/CMSSW_12_5_X_2022-08-17-2300/src/FWCore/Utilities/interface/OffsetToBase.h:49:43: warning: conversion from 'size_t' {aka 'long unsigned int'} to 'int' may change value [-Wconversion]
  12873 /data/cmsbld/jenkins/workspace/ib-run-pr-tests/CMSSW_12_5_X_2022-08-17-2300/src/DataFormats/GeometryVector/interface/Phi.h:39:15: warning: conversion from 'double' to 'float' may change value [-Wfloat-conversion]
  12873 /data/cmsbld/jenkins/workspace/ib-run-pr-tests/CMSSW_12_5_X_2022-08-17-2300/src/DataFormats/GeometryVector/interface/Phi.h:41:15: warning: conversion from 'double' to 'float' may change value [-Wfloat-conversion]
  14685 /data/cmsbld/jenkins/workspace/ib-run-pr-tests/CMSSW_12_5_X_2022-08-17-2300/src/FWCore/Concurrency/interface/LimitedTaskQueue.h:107:65: warning: conversion from 'std::vector<edm::SerialTaskQueue>::size_type' {aka 'long unsigned int'} to 'unsigned int' may change value [-Wconversion]
  16602 /data/cmsbld/jenkins/workspace/ib-run-pr-tests/CMSSW_12_5_X_2022-08-17-2300/src/FWCore/Framework/interface/ProductRegistryHelper.h:178:47: warning: conversion from 'std::vector<edm::ProductRegistryHelper::TypeLabelItem>::size_type' {aka 'long unsigned int'} to 'unsigned int' may change value [-Wconversion]
  17283 /data/cmsbld/jenkins/workspace/ib-run-pr-tests/CMSSW_12_5_X_2022-08-17-2300/src/FWCore/ParameterSet/interface/DocFormatHelper.h:37:39: warning: conversion from 'size_t' {aka 'long unsigned int'} to 'int' may change value [-Wconversion]
  18590 /data/cmsbld/jenkins/workspace/ib-run-pr-tests/CMSSW_12_5_X_2022-08-17-2300/src/FWCore/Utilities/interface/SoATuple.h:148:62: warning: conversion from 'long unsigned int' to 'unsigned int' may change value [-Wconversion]
  18591 /data/cmsbld/jenkins/workspace/ib-run-pr-tests/CMSSW_12_5_X_2022-08-17-2300/src/FWCore/Utilities/interface/SoATuple.h:122:33: warning: conversion from 'long unsigned int' to 'unsigned int' may change value [-Wconversion]
  18591 /data/cmsbld/jenkins/workspace/ib-run-pr-tests/CMSSW_12_5_X_2022-08-17-2300/src/FWCore/Utilities/interface/SoATuple.h:147:33: warning: conversion from 'long unsigned int' to 'unsigned int' may change value [-Wconversion]
  18591 /data/cmsbld/jenkins/workspace/ib-run-pr-tests/CMSSW_12_5_X_2022-08-17-2300/src/FWCore/Utilities/interface/SoATuple.h:149:62: warning: conversion from 'long unsigned int' to 'unsigned int' may change value [-Wconversion]
  18591 /data/cmsbld/jenkins/workspace/ib-run-pr-tests/CMSSW_12_5_X_2022-08-17-2300/src/FWCore/Utilities/interface/SoATuple.h:209:33: warning: conversion from 'long unsigned int' to 'unsigned int' may change value [-Wconversion]
  18591 /data/cmsbld/jenkins/workspace/ib-run-pr-tests/CMSSW_12_5_X_2022-08-17-2300/src/FWCore/Utilities/interface/SoATuple.h:219:33: warning: conversion from 'long unsigned int' to 'unsigned int' may change value [-Wconversion]
  18591 /data/cmsbld/jenkins/workspace/ib-run-pr-tests/CMSSW_12_5_X_2022-08-17-2300/src/FWCore/Utilities/interface/SoATuple.h:263:68: warning: conversion from 'long unsigned int' to 'unsigned int' may change value [-Wconversion]
  18591 /data/cmsbld/jenkins/workspace/ib-run-pr-tests/CMSSW_12_5_X_2022-08-17-2300/src/FWCore/Utilities/interface/SoATuple.h:273:67: warning: conversion from 'long unsigned int' to 'unsigned int' may change value [-Wconversion]
  18591 /data/cmsbld/jenkins/workspace/ib-run-pr-tests/CMSSW_12_5_X_2022-08-17-2300/src/FWCore/Utilities/interface/SoATuple.h:274:62: warning: conversion from 'long unsigned int' to 'unsigned int' may change value [-Wconversion]
  18591 /data/cmsbld/jenkins/workspace/ib-run-pr-tests/CMSSW_12_5_X_2022-08-17-2300/src/FWCore/Utilities/interface/SoATuple.h:275:62: warning: conversion from 'long unsigned int' to 'unsigned int' may change value [-Wconversion]
  18591 /data/cmsbld/jenkins/workspace/ib-run-pr-tests/CMSSW_12_5_X_2022-08-17-2300/src/FWCore/Utilities/interface/SoATuple.h:281:33: warning: conversion from 'long unsigned int' to 'unsigned int' may change value [-Wconversion]
  18591 /data/cmsbld/jenkins/workspace/ib-run-pr-tests/CMSSW_12_5_X_2022-08-17-2300/src/FWCore/Utilities/interface/SoATupleHelper.h:34:51: warning: conversion from 'size_t' {aka 'long unsigned int'} to 'unsigned int' may change value [-Wconversion]
  20484 /data/cmsbld/jenkins/workspace/ib-run-pr-tests/CMSSW_12_5_X_2022-08-17-2300/src/DataFormats/Math/interface/deltaPhi.h:19:27: warning: conversion from 'double' to 'float' changes value from '1.5915494309189535e-1' to '1.59154937e-1f' [-Wfloat-conversion]
  20865 /data/cmsbld/jenkins/workspace/ib-run-pr-tests/CMSSW_12_5_X_2022-08-17-2300/src/DataFormats/Math/interface/angle_units.h:16:68: warning: conversion from 'long double' to 'double' may change value [-Wfloat-conversion]
  20865 /data/cmsbld/jenkins/workspace/ib-run-pr-tests/CMSSW_12_5_X_2022-08-17-2300/src/DataFormats/Math/interface/angle_units.h:17:75: warning: conversion from 'long long unsigned int' to 'double' may change value [-Wconversion]
  20865 /data/cmsbld/jenkins/workspace/ib-run-pr-tests/CMSSW_12_5_X_2022-08-17-2300/src/DataFormats/Math/interface/angle_units.h:18:68: warning: conversion from 'long double' to 'double' may change value [-Wfloat-conversion]

[fwyzard]

OK, those are clearly too much to be useful :-(

I wonder if there is a way to warn only about conversions from floating point to integer types, and not from wider to narrower types ?

[fwyzard]

Unfortunately I couldn't find anything: -Wno-narrowing does not suppress the warnings about these kinds of narrowing conversions.

[smuzaffar]

we can use -Wfloat-conversion to report floating point conversion but we still have 65K such unique warnings (700K in total) from cmssw itself [a]

   4233 TrackingTools/TrajectoryParametrization/interface/LocalTrajectoryError.h:82:42: warning: conversion from 'double' to 'float' may change value [-Wfloat-conversion]
   4233 TrackingTools/TrajectoryParametrization/interface/LocalTrajectoryError.h:82:69: warning: conversion from 'double' to 'float' may change value [-Wfloat-conversion]
   4233 TrackingTools/TrajectoryParametrization/interface/LocalTrajectoryError.h:82:96: warning: conversion from 'double' to 'float' may change value [-Wfloat-conversion]
   5436 DataFormats/CaloTowers/interface/CaloTower.h:120:52: warning: conversion from 'double' to 'float' may change value [-Wfloat-conversion]
   5436 DataFormats/CaloTowers/interface/CaloTower.h:196:52: warning: conversion from 'double' to 'float' may change value [-Wfloat-conversion]
   5436 DataFormats/CaloTowers/interface/CaloTower.h:197:52: warning: conversion from 'double' to 'float' may change value [-Wfloat-conversion]
   8028 DataFormats/TrajectoryState/interface/PTrajectoryStateOnDet.h:48:25: warning: conversion from 'double' to 'float' changes value from '-9.9999e+14' to '-9.99989988e+14f' [-Wfloat-conversion]
   9393 DataFormats/TrajectoryState/interface/LocalTrajectoryParameters.h:39:17: warning: conversion from 'double' to 'float' may change value [-Wfloat-conversion]
   9393 DataFormats/TrajectoryState/interface/LocalTrajectoryParameters.h:40:18: warning: conversion from 'double' to 'float' may change value [-Wfloat-conversion]
   9393 DataFormats/TrajectoryState/interface/LocalTrajectoryParameters.h:41:18: warning: conversion from 'double' to 'float' may change value [-Wfloat-conversion]
   9393 DataFormats/TrajectoryState/interface/LocalTrajectoryParameters.h:42:15: warning: conversion from 'double' to 'float' may change value [-Wfloat-conversion]
   9393 DataFormats/TrajectoryState/interface/LocalTrajectoryParameters.h:43:15: warning: conversion from 'double' to 'float' may change value [-Wfloat-conversion]
   9393 DataFormats/TrajectoryState/interface/LocalTrajectoryParameters.h:44:17: warning: conversion from 'float' to 'short int' may change value [-Wfloat-conversion]
  10875 DataFormats/GeometrySurface/interface/LocalError.h:40:63: warning: conversion from 'double' to 'float' may change value [-Wfloat-conversion]
  10875 DataFormats/GeometrySurface/interface/LocalError.h:40:73: warning: conversion from 'double' to 'float' may change value [-Wfloat-conversion]
  12873 DataFormats/GeometryVector/interface/Phi.h:39:15: warning: conversion from 'double' to 'float' may change value [-Wfloat-conversion]
  12873 DataFormats/GeometryVector/interface/Phi.h:41:15: warning: conversion from 'double' to 'float' may change value [-Wfloat-conversion]
  20484 DataFormats/Math/interface/deltaPhi.h:19:27: warning: conversion from 'double' to 'float' changes value from '1.5915494309189535e-1' to '1.59154937e-1f' [-Wfloat-conversion]
  20865 DataFormats/Math/interface/angle_units.h:16:68: warning: conversion from 'long double' to 'double' may change value [-Wfloat-conversion]
  20865 DataFormats/Math/interface/angle_units.h:18:68: warning: conversion from 'long double' to 'double' may change value [-Wfloat-conversion]

[fwyzard]

I was actually looking for a way to do the opposite:

• warn about conversions from float or double to integer types
• do not warn about conversions from double to float
• not sure about conversions from larger integer types to smaller integer types

[smuzaffar]

I am afraid there is no such compiler flag to acheive this. -Wfloat-conversion warns for implicit conversions that reduce the precision of a real value. This includes conversions from real to integer, and from higher precision real to lower precision real values. So it will also warn on double to float conversions

[davidlange6]

It would be interesting to know the order of magnitude number of warnings in cmssw (as this also flags things like doubles -> float)..

On Aug 18, 2022, at 10:15 AM, Andrea Bocci @.***> wrote:

Should we consider adding -Wconversion to the build flags, to catch errors like floating point values being assigned to integers and silently truncated ?

We could first introduce it as -Wconversion, and once the warnings have been fixed switch to -Werror=conversion.

— Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you are subscribed to this thread.