Resolving clang analyzer warnings

Only exception scanner.cpp

Change-Id: Ia3a74d5079b2e5dccac4499de9c4b65fdd3868e1

diff --git a/src/gromacs/fatalerror/gmxassert.h b/src/gromacs/fatalerror/gmxassert.h
index 24a8589d9d27c23bf1d8b55cfb051101d51cd68b..55c3d12da219732b277533e30fc5b5ecfd5bb86b 100644 (file)
--- a/src/gromacs/fatalerror/gmxassert.h
+++ b/src/gromacs/fatalerror/gmxassert.h
@@ -40,6 +40,7 @@
 #define GMX_FATALERROR_GMXASSERT_H
 
 #include <boost/current_function.hpp>
+#include <boost/exception/detail/attribute_noreturn.hpp>
 
 /*! \addtopublicapi
  * \{
@@ -91,6 +92,7 @@ namespace internal
  *
  * \ingroup module_fatalerror
  */
+BOOST_ATTRIBUTE_NORETURN
 void assertHandler(const char *condition, const char *msg,
                    const char *func, const char *file, int line);
 

diff --git a/src/gromacs/options/tests/abstractoptionstorage.cpp b/src/gromacs/options/tests/abstractoptionstorage.cpp
index 71797a7777b15ecf1d8d5443805205f4c667fdb3..ffecbab76f8d0587c314a53fcbf39c32ceed3e58 100644 (file)
--- a/src/gromacs/options/tests/abstractoptionstorage.cpp
+++ b/src/gromacs/options/tests/abstractoptionstorage.cpp
@@ -164,11 +164,11 @@ TEST(AbstractOptionStorageTest, HandlesSetInFinish)
 {
     gmx::Options                options(NULL, NULL);
     std::vector<std::string>    values;
-    MockOptionStorage          *mock;
+    MockOptionStorage          *mock = NULL;
     ASSERT_NO_THROW(options.addOption(
                         MockOption("name").storageObject(&mock).required()
                             .storeVector(&values)));
-
+    ASSERT_TRUE(mock != NULL);
     {
         ::testing::InSequence dummy;
         using ::testing::DoAll;
@@ -237,11 +237,11 @@ TEST(AbstractOptionStorageTest, HandlesValueAddition)
 {
     gmx::Options                options(NULL, NULL);
     std::vector<std::string>    values;
-    MockOptionStorage          *mock;
+    MockOptionStorage          *mock=NULL;
     ASSERT_NO_THROW(options.addOption(
                         MockOption("name").storageObject(&mock)
                             .storeVector(&values).multiValue()));
-
+    ASSERT_TRUE(mock != NULL);
     {
         ::testing::InSequence dummy;
         using ::testing::DoAll;
@@ -279,11 +279,11 @@ TEST(AbstractOptionStorageTest, HandlesTooManyValueAddition)
 {
     gmx::Options                options(NULL, NULL);
     std::vector<std::string>    values;
-    MockOptionStorage          *mock;
+    MockOptionStorage          *mock=NULL;
     ASSERT_NO_THROW(options.addOption(
                         MockOption("name").storageObject(&mock)
                             .storeVector(&values).valueCount(2)));
-
+    ASSERT_TRUE(mock != NULL);
     {
         ::testing::InSequence dummy;
         using ::testing::DoAll;

diff --git a/src/gromacs/selection/compiler.cpp b/src/gromacs/selection/compiler.cpp
index b9acf9449e64fa02c36d8aa139d61555951ec9c3..87ea0b47cb035e78873cb17032a0a9bcb28df19e 100644 (file)
--- a/src/gromacs/selection/compiler.cpp
+++ b/src/gromacs/selection/compiler.cpp
@@ -1179,6 +1179,10 @@ setup_memory_pooling(t_selelem *sel, gmx_sel_mempool_t *mempool)
 static void
 init_item_evaloutput(t_selelem *sel)
 {
+    GMX_ASSERT(!(sel->child == NULL &&
+                 (sel->type == SEL_SUBEXPRREF || sel->type == SEL_SUBEXPR)),
+               "Subexpression elements should always have a child element");
+
     /* Process children. */
     if (sel->type != SEL_SUBEXPRREF)
     {
@@ -1491,6 +1495,8 @@ init_item_minmax_groups(t_selelem *sel)
                  && ((sel->cdata->flags & SEL_CDATA_SIMPLESUBEXPR)
                      || (sel->cdata->flags & SEL_CDATA_FULLEVAL)))
         {
+            GMX_ASSERT(sel->child,
+                       "Subexpression elements should always have a child element");
             sel->cdata->gmin = sel->child->cdata->gmin;
             sel->cdata->gmax = sel->child->cdata->gmax;
         }
@@ -2042,6 +2048,7 @@ analyze_static(gmx_sel_evaluate_t *data, t_selelem *sel, gmx_ana_index_t *g)
 
         case SEL_EXPRESSION:
         case SEL_MODIFIER:
+            GMX_ASSERT(g, "group cannot be null");
             _gmx_sel_evaluate_method_params(data, sel, g);
             init_method(sel, data->top, g->isize);
             if (!(sel->flags & SEL_DYNAMIC))
@@ -2128,6 +2135,7 @@ analyze_static(gmx_sel_evaluate_t *data, t_selelem *sel, gmx_ana_index_t *g)
             }
             else if (sel->u.cgrp.isize == 0)
             {
+                GMX_ASSERT(g, "group cannot be null");
                 gmx_ana_index_reserve(&sel->u.cgrp, g->isize);
                 sel->cdata->evaluate(data, sel, g);
                 if (bDoMinMax)
@@ -2349,6 +2357,10 @@ init_root_item(t_selelem *root, gmx_ana_index_t *gall)
 static void
 postprocess_item_subexpressions(t_selelem *sel)
 {
+    GMX_ASSERT(!(sel->child == NULL &&
+                 (sel->type == SEL_SUBEXPRREF || sel->type == SEL_SUBEXPR)),
+               "Subexpression elements should always have a child element");
+
     /* Process children. */
     if (sel->type != SEL_SUBEXPRREF)
     {
@@ -2378,10 +2390,8 @@ postprocess_item_subexpressions(t_selelem *sel)
         sel->u.cgrp.name = name;
 
         sel->evaluate = &_gmx_sel_evaluate_subexpr_staticeval;
-        if (sel->cdata)
-        {
-            sel->cdata->evaluate = sel->evaluate;
-        }
+        sel->cdata->evaluate = sel->evaluate;
+
         _gmx_selelem_free_values(sel->child);
         sel->child->mempool = NULL;
         _gmx_selvalue_setstore(&sel->child->v, sel->v.u.ptr);

diff --git a/src/gromacs/selection/evaluate.cpp b/src/gromacs/selection/evaluate.cpp
index a0210372df350d04cff50ccd406c63984ce4636d..83567fa30ef3989d616776be55677e1200fe0738 100644 (file)
--- a/src/gromacs/selection/evaluate.cpp
+++ b/src/gromacs/selection/evaluate.cpp
@@ -1014,12 +1014,11 @@ void
 _gmx_sel_evaluate_arithmetic(gmx_sel_evaluate_t *data, t_selelem *sel,
                              gmx_ana_index_t *g)
 {
-    t_selelem  *left, *right;
     int         n, i, i1, i2;
     real        lval, rval=0., val=0.;
 
-    left  = sel->child;
-    right = left->next;
+    t_selelem  *const left  = sel->child;
+    t_selelem  *const right = left->next;
 
     SelelemTemporaryValueAssigner assigner;
     MempoolSelelemReserver reserver;
@@ -1039,10 +1038,14 @@ _gmx_sel_evaluate_arithmetic(gmx_sel_evaluate_t *data, t_selelem *sel,
 
     n = (sel->flags & SEL_SINGLEVAL) ? 1 : g->isize;
     sel->v.nr = n;
+
+    bool bArithNeg = (sel->u.arith.type == ARITH_NEG);
+    GMX_ASSERT(right || bArithNeg,
+               "Right operand cannot be null except for negations");
     for (i = i1 = i2 = 0; i < n; ++i)
     {
         lval = left->v.u.r[i1];
-        if (sel->u.arith.type != ARITH_NEG)
+        if (!bArithNeg)
         {
             rval = right->v.u.r[i2];
         }
@@ -1060,7 +1063,7 @@ _gmx_sel_evaluate_arithmetic(gmx_sel_evaluate_t *data, t_selelem *sel,
         {
             ++i1;
         }
-        if (sel->u.arith.type != ARITH_NEG && !(right->flags & SEL_SINGLEVAL))
+        if (!bArithNeg && !(right->flags & SEL_SINGLEVAL))
         {
             ++i2;
         }

diff --git a/src/gromacs/selection/parsetree.cpp b/src/gromacs/selection/parsetree.cpp
index 927512acce048f1bc82c721bb8b276ce768d1c44..7a3de60ba8af8dbf76edb493f2582b40aefc35a9 100644 (file)
--- a/src/gromacs/selection/parsetree.cpp
+++ b/src/gromacs/selection/parsetree.cpp
@@ -471,6 +471,7 @@ _gmx_selelem_update_flags(t_selelem *sel, yyscan_t scanner)
      * children have been updated. */
     if (sel->type == SEL_ROOT)
     {
+        GMX_ASSERT(sel->child, "Root elements should always have a child");
         sel->flags |= (sel->child->flags & SEL_VALTYPEMASK);
     }
     /* Mark that the flags are set */

diff --git a/src/gromacs/selection/position.cpp b/src/gromacs/selection/position.cpp
index 232de5ab945606f41128e60a264cc4f780ea4595..c24fc9d67661af32fe92b056dc9834796fb6fdae 100644 (file)
--- a/src/gromacs/selection/position.cpp
+++ b/src/gromacs/selection/position.cpp
@@ -211,10 +211,12 @@ gmx_ana_pos_copy(gmx_ana_pos_t *dest, gmx_ana_pos_t *src, bool bFirst)
     memcpy(dest->x, src->x, dest->nr*sizeof(*dest->x));
     if (dest->v)
     {
+        GMX_ASSERT(src->v, "src velocities should be non-null if dest velocities are allocated");
         memcpy(dest->v, src->v, dest->nr*sizeof(*dest->v));
     }
     if (dest->f)
     {
+        GMX_ASSERT(src->f, "src forces should be non-null if dest forces are allocated");
         memcpy(dest->f, src->f, dest->nr*sizeof(*dest->f));
     }
     gmx_ana_indexmap_copy(&dest->m, &src->m, bFirst);

diff --git a/src/gromacs/selection/selmethod.cpp b/src/gromacs/selection/selmethod.cpp
index 48456a859cbabe3fbf6f72733cf2c26ce8c8ab93..11759808d77c5825381b486ae408757fc6913b06 100644 (file)
--- a/src/gromacs/selection/selmethod.cpp
+++ b/src/gromacs/selection/selmethod.cpp
@@ -228,7 +228,6 @@ check_params(FILE *fp, const char *name, int nparams, gmx_ana_selparam_t param[]
     if (nparams > 0 && !param)
     {
         report_error(fp, name, "error: missing parameter data");
-        bOk = false;
         return false;
     }
     if (nparams == 0 && param)

diff --git a/src/programs/g_ana/g_ana.cpp b/src/programs/g_ana/g_ana.cpp
index 140f626f5013b513228543814b4304f470cf40a1..3f41c9d8700579e408ab388018dd88bd36d0ef2a 100644 (file)
--- a/src/programs/g_ana/g_ana.cpp
+++ b/src/programs/g_ana/g_ana.cpp
@@ -58,7 +58,9 @@ main(int argc, char *argv[])
         ++argv;
 
         gmx::TrajectoryAnalysisCommandLineRunner runner(mod.get());
+#ifndef __clang_analyzer__  //Clang BUG: 11722
         bPrintCopyrightOnError = false;
+#endif
         return runner.run(argc, argv);
     }
     catch (const std::exception &ex)