[gmx-developers] How to deal with Clang-tidy/Clang static analyzer warnings in existing headers?

[Ullmann, Thomas]

OK, thanks for your time and please let us know how to go about the FMA documentation.

Have a nice evening,
Thomas.
________________________________________
From: gromacs.org_gmx-developers-bounces at maillist.sys.kth.se <gromacs.org_gmx-developers-bounces at maillist.sys.kth.se> on behalf of Erik Lindahl <erik.lindahl at gmail.com>
Sent: Wednesday, February 27, 2019 6:21:25 PM
To: gmx-developers at gromacs.org
Subject: Re: [gmx-developers] How to deal with Clang-tidy/Clang static analyzer warnings in existing headers?

Hi,


On Wed, Feb 27, 2019 at 5:48 PM Ullmann, Thomas <thomas.ullmann at mpibpc.mpg.de<mailto:thomas.ullmann at mpibpc.mpg.de>> wrote:


1081 /home/tullman/GROMACS/gromacs.release-5-X.my/src/gromacs/simd/impl_x86_avx_256/impl_x86_avx_256_simd_double.h:60:9<http://gromacs.release-5-X.my/src/gromacs/simd/impl_x86_avx_256/impl_x86_avx_256_simd_double.h:60:9>: error: constructor does not initialize these fields: simdInternal_ [cppcoreguidelines-pro-type-member-init,-warnings-as-errors]
1082         SimdDouble() {}
1083         ^


That could be a new version of clang being more picky, or using a different format for the internal storage of the SIMD type.

However, for C++ it's a very good practice not to define variables until you actually need them. If you only need the variable inside a loop, just define it there :-) That way we are guaranteed never to have an uninitialized variable, and also that it won't have any old value we forgot about.  If anything, should we start looking into the warning, one potential solution might just be to prohibit declaring uninitialized SIMD vars :-)

OK, I understand and I do not intend to touch the SIMD module itself. I wrote the SIMD code for the linear algebra on
my matrix classes for use in core gmx, that is in the lambda dynamics module, which (hopefully) will be run very frequently
and would then of course consume valuable resources on supercomputers. The functional mode analysis can just also profit
from this code ...

Sure, but "hopefully" is the key word there ;-) The reason for my comment is that there's no such thing as a free lunch: If the amount of work required to get a normal clean patch to compile clean and have people happy about it is "N", you might very well have to spend "5N" to get the same code committed as SIMD. For example: Every single test you write will also have to consider and test the different SIMD widths, and no changes will be able to go in until the handful of us who are familiar with SIMD (mostly me, Roland, Berk and Mark) have time to go through the code in detail.

Doing it without SIMD now won't prevent you from adding that later - and then everyone will find it much easier to understand, because it's just a SIMD version of well-documented and well-tested code already working :-)




https://gerrit.gromacs.org/#/c/3277/50/docs/reference-manual/analysis/functional-mode-analysis.rst


Will have to get back on that later - difficult to check on the phone :-)


--
Erik Lindahl <erik.lindahl at dbb.su.se<mailto:erik.lindahl at dbb.su.se>>
Professor of Biophysics, Dept. Biochemistry & Biophysics, Stockholm University
Science for Life Laboratory, Box 1031, 17121 Solna, Sweden