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

[Erik Lindahl]

Hi,


On Wed, Feb 27, 2019 at 5:48 PM Ullmann, Thomas <
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:
> 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>
Professor of Biophysics, Dept. Biochemistry & Biophysics, Stockholm
University
Science for Life Laboratory, Box 1031, 17121 Solna, Sweden
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://maillist.sys.kth.se/pipermail/gromacs.org_gmx-developers/attachments/20190227/7cb73822/attachment.html>