Add grompp check for usage of "define" field in mdp

Users can misspell the strings specified with -D in the topology. This  now
check that every -D field has a corresponding #ifdef and/or #ifndef and/or
C macro-style text replacement in the topology.

Fixed one unit test with unnecessary define field

refs #1975

Change-Id: Ie6329b234a60dde8efc34fb788e6296f241651ed

diff --git a/docs/release-notes/miscellaneous.rst b/docs/release-notes/miscellaneous.rst
index 27789e47148af8e775833e8239c08ac5d579b6f7..bcff8198f6b660b8ec960e07fe27b4a5825f8c3a 100644 (file)
--- a/docs/release-notes/miscellaneous.rst
+++ b/docs/release-notes/miscellaneous.rst
@@ -1,2 +1,9 @@
 Miscellaneous
 ^^^^^^^^^^^^^
+
+grompp now warns if macros in mdp "define" field are unused in topology
+"""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""
+Macros defined in the mdp (with e.g. -DPOSRES) now cause a warning
+in grompp if they are not encountered while parsing the topology file
+
+:issue:`1975`

diff --git a/src/gromacs/gmxpreprocess/gmxcpp.cpp b/src/gromacs/gmxpreprocess/gmxcpp.cpp
index 2c27664586e0a1d0098ff2a8cbea84bb5438ea0f..a8e621ca342a16c925ce0a0cf2bc4f39e872b68f 100644 (file)
--- a/src/gromacs/gmxpreprocess/gmxcpp.cpp
+++ b/src/gromacs/gmxpreprocess/gmxcpp.cpp
@@ -3,7 +3,7 @@
  *
  * Copyright (c) 1991-2000, University of Groningen, The Netherlands.
  * Copyright (c) 2001-2004, The GROMACS development team.
- * Copyright (c) 2013,2014,2015,2017,2018, by the GROMACS development team, led by
+ * Copyright (c) 2013,2014,2015,2017,2018,2019, by the GROMACS development team, led by
  * Mark Abraham, David van der Spoel, Berk Hess, and Erik Lindahl,
  * and including many others, as listed in the AUTHORS file in the
  * top-level source directory and at http://www.gromacs.org.
@@ -49,6 +49,7 @@
 #include <algorithm>
 #include <memory>
 
+#include <unordered_set>
 #include <sys/types.h>
 
 #include "gromacs/utility/arrayref.h"
@@ -73,16 +74,16 @@ struct gmx_cpp
 {
     std::shared_ptr < std::vector < t_define>>    defines;
     std::shared_ptr < std::vector < std::string>> includes;
-
-    FILE             *fp = nullptr;
-    std::string       path;
-    std::string       cwd;
-    std::string       fn;
-    std::string       line;
-    int               line_nr;
-    std::vector<int>  ifdefs;
-    struct gmx_cpp   *child  = nullptr;
-    struct gmx_cpp   *parent = nullptr;
+    std::unordered_set<std::string> unmatched_defines;
+    FILE                           *fp = nullptr;
+    std::string                     path;
+    std::string                     cwd;
+    std::string                     fn;
+    std::string                     line;
+    int                             line_nr;
+    std::vector<int>                ifdefs;
+    struct gmx_cpp                 *child  = nullptr;
+    struct gmx_cpp                 *parent = nullptr;
 };
 
 static bool is_word_end(char c)
@@ -258,12 +259,14 @@ cpp_open_file(const char                                     *filenm,
                 {
                     std::string buf = cppopts[i] + 2;
                     buf.resize(ptr - cppopts[i] - 2);
-
                     add_define(cpp->defines.get(), buf, ptr + 1);
+                    cpp->unmatched_defines.insert(buf);
+
                 }
                 else
                 {
                     add_define(cpp->defines.get(), cppopts[i] + 2, "");
+                    cpp->unmatched_defines.insert(cppopts[i] + 2);
                 }
             }
             i++;
@@ -379,6 +382,14 @@ process_directive(gmx_cpp_t         *handlep,
             {
                 if (define.name == dval)
                 {
+                    // erase from unmatched_defines in original handle
+                    gmx_cpp_t root = handle;
+                    while (root->parent != nullptr)
+                    {
+                        root = root->parent;
+                    }
+                    root->unmatched_defines.erase(dval);
+
                     found = true;
                     break;
                 }
@@ -627,6 +638,14 @@ int cpp_read_line(gmx_cpp_t *handlep, int n, char buf[])
             }
             if (nn > 0)
             {
+                // Need to erase  unmatched define in original handle
+                gmx_cpp_t root = handle;
+                while (root->parent != nullptr)
+                {
+                    root = root->parent;
+                }
+                root->unmatched_defines.erase(define.name);
+
                 std::string  name;
                 const char  *ptr = buf;
                 const char  *ptr2;
@@ -738,3 +757,20 @@ char *cpp_error(gmx_cpp_t *handlep, int status)
 
     return gmx_strdup(buf);
 }
+
+std::string checkAndWarnForUnusedDefines(const gmx_cpp &handle)
+{
+    std::string warning;
+    if (!handle.unmatched_defines.empty())
+    {
+        warning = "The following macros were defined in the 'define' mdp field with the -D prefix, but "
+            "were not used in the topology:\n";
+        for (auto &str : handle.unmatched_defines)
+        {
+            warning += ("    " + str + "\n");
+        }
+        warning += "If you haven't made a spelling error, either use the macro you defined, "
+            "or don't define the macro";
+    }
+    return warning;
+}

diff --git a/src/gromacs/gmxpreprocess/gmxcpp.h b/src/gromacs/gmxpreprocess/gmxcpp.h
index eca07f155ddb7b4cacddd864f68a0e088b063f53..f85fe06b54287f0df10410a03ee3c3ea1869fa38 100644 (file)
--- a/src/gromacs/gmxpreprocess/gmxcpp.h
+++ b/src/gromacs/gmxpreprocess/gmxcpp.h
@@ -3,7 +3,7 @@
  *
  * Copyright (c) 1991-2000, University of Groningen, The Netherlands.
  * Copyright (c) 2001-2008, The GROMACS development team.
- * Copyright (c) 2012,2014,2015,2018, by the GROMACS development team, led by
+ * Copyright (c) 2012,2014,2015,2018,2019, by the GROMACS development team, led by
  * Mark Abraham, David van der Spoel, Berk Hess, and Erik Lindahl,
  * and including many others, as listed in the AUTHORS file in the
  * top-level source directory and at http://www.gromacs.org.
@@ -86,4 +86,8 @@ void cpp_done(gmx_cpp_t handle);
  */
 char *cpp_error(gmx_cpp_t *handlep, int status);
 
+/* Returns warning message if strings defined in mdp define section (e.g. -DFLEXIBLE)
+ * were not not found when processing the topology */
+std::string checkAndWarnForUnusedDefines(const gmx_cpp &handle);
+
 #endif

diff --git a/src/gromacs/gmxpreprocess/topio.cpp b/src/gromacs/gmxpreprocess/topio.cpp
index 57759a93b31c758e79ae1a2628c273fd3f51fb7e..459f771cd0a21f7ca65c20353cf4c174beee04e9 100644 (file)
--- a/src/gromacs/gmxpreprocess/topio.cpp
+++ b/src/gromacs/gmxpreprocess/topio.cpp
@@ -48,6 +48,7 @@
 
 #include <algorithm>
 
+#include <unordered_set>
 #include <sys/types.h>
 
 #include "gromacs/fileio/gmxfio.h"
@@ -892,6 +893,14 @@ static char **read_topol(const char *infile, const char *outfile,
         }
     }
     while (!done);
+
+    // Check that all strings defined with -D were used when processing topology
+    std::string unusedDefineWarning = checkAndWarnForUnusedDefines(*handle);
+    if (!unusedDefineWarning.empty())
+    {
+        warning(wi, unusedDefineWarning);
+    }
+
     sfree(cpp_opts_return);
 
     if (out)

diff --git a/src/programs/mdrun/tests/swapcoords.cpp b/src/programs/mdrun/tests/swapcoords.cpp
index 195983f14f3d19ade35506aae937aee69d018061..40390a0a04a71c9450548b14ca2613b5b8e16bdc 100644 (file)
--- a/src/programs/mdrun/tests/swapcoords.cpp
+++ b/src/programs/mdrun/tests/swapcoords.cpp
@@ -1,7 +1,7 @@
 /*
  * This file is part of the GROMACS molecular simulation package.
  *
- * Copyright (c) 2013,2014,2015,2016,2018, by the GROMACS development team, led by
+ * Copyright (c) 2013,2014,2015,2016,2018,2019, by the GROMACS development team, led by
  * Mark Abraham, David van der Spoel, Berk Hess, and Erik Lindahl,
  * and including many others, as listed in the AUTHORS file in the
  * top-level source directory and at http://www.gromacs.org.
@@ -80,7 +80,6 @@ TEST_F(CompelTest, SwapCanRun)
     const std::string mdpContents = R"(
         dt                       = 0.005
         nsteps                   = 2
-        define                   = -DPOSRES
         tcoupl                   = Berendsen
         tc-grps                  = System
         tau-t                    = 0.5