Enforce unsigned type in flags.

Flag enums could be signed types - since the only operations performed
by the flags were bit ops, this was harmless, but the implicit signed/
unsigned conversions are caught in analysis tools like UBSAN that can
also catch real issues.

Added a compile-time enforcement of underlying unsigned type to the
flags class, and specified uint64_t for underlying type of the class
and the various flags that are passed to it.

Added some SFINAE to disable accidental instantiation of non-enum
types - before C++ 20 std::underlying_type on a non-enum is undefined.

diff --git a/src/gromacs/analysisdata/dataframe.h b/src/gromacs/analysisdata/dataframe.h
index 5a5164d291c60cdb5ad981c092a09b05205a7458..c034fd8ed27a030d85e12cc7e7afe5c74f13f566 100644 (file)
--- a/src/gromacs/analysisdata/dataframe.h
+++ b/src/gromacs/analysisdata/dataframe.h
@@ -1,7 +1,7 @@
 /*
  * This file is part of the GROMACS molecular simulation package.
  *
- * Copyright (c) 2012,2013,2014,2017,2019, by the GROMACS development team, led by
+ * Copyright (c) 2012,2013,2014,2017,2019,2020, 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.
@@ -153,7 +153,7 @@ public:
 
 private:
     //! Possible flags for \a flags_.
-    enum Flag
+    enum Flag : uint64_t
     {
         efSet      = 1 << 0, //!< Value has been set.
         efErrorSet = 1 << 1, //!< Error estimate has been set.

diff --git a/src/gromacs/options/optionflags.h b/src/gromacs/options/optionflags.h
index 6eb2ace8db3444c23f3474a660229cb1ab0e5fcc..575853334c8e726fb0a0511574b1e77efd38bd34 100644 (file)
--- a/src/gromacs/options/optionflags.h
+++ b/src/gromacs/options/optionflags.h
@@ -1,7 +1,7 @@
 /*
  * This file is part of the GROMACS molecular simulation package.
  *
- * Copyright (c) 2010,2011,2012,2014,2019, by the GROMACS development team, led by
+ * Copyright (c) 2010,2011,2012,2014,2019,2020, 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.
@@ -62,7 +62,7 @@ namespace gmx
  * The flags related to default values are confusing, consider reorganizing
  * them.
  */
-enum OptionFlag
+enum OptionFlag : uint64_t
 {
     //! %Option has been set.
     efOption_Set = 1 << 0,

diff --git a/src/gromacs/selection/selectionenums.h b/src/gromacs/selection/selectionenums.h
index 3122231119c12a382ecee01b3cea920119dec690..8ae77c809b587444c1fc239f82e269cd00f85f66 100644 (file)
--- a/src/gromacs/selection/selectionenums.h
+++ b/src/gromacs/selection/selectionenums.h
@@ -66,7 +66,7 @@ namespace gmx
  * These flags are not part of the public interface, even though they are in an
  * installed header.  They are needed in the implementation of SelectionOption.
  */
-enum SelectionFlag
+enum SelectionFlag : uint64_t
 {
     efSelection_OnlyStatic = 1 << 0,
     efSelection_OnlyAtoms  = 1 << 1,

diff --git a/src/gromacs/selection/tests/selectioncollection.cpp b/src/gromacs/selection/tests/selectioncollection.cpp
index 94484badc2f8441588e9c12ea57f19691fb684be..b8c02bb836e3e2a62a66096a21690c718b816797 100644 (file)
--- a/src/gromacs/selection/tests/selectioncollection.cpp
+++ b/src/gromacs/selection/tests/selectioncollection.cpp
@@ -2,7 +2,7 @@
  * This file is part of the GROMACS molecular simulation package.
  *
  * Copyright (c) 2010-2018, The GROMACS development team.
- * Copyright (c) 2019, by the GROMACS development team, led by
+ * Copyright (c) 2019,2020, 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.
@@ -174,7 +174,7 @@ void SelectionCollectionInteractiveTest::runTest(int  count,
 class SelectionCollectionDataTest : public SelectionCollectionTest
 {
 public:
-    enum TestFlag
+    enum TestFlag : uint64_t
     {
         efTestEvaluation          = 1 << 0,
         efTestPositionAtoms       = 1 << 1,

diff --git a/src/gromacs/utility/flags.h b/src/gromacs/utility/flags.h
index d2f49344e2ed86c19b764d3bbda8c88a76663c7b..4be79c783cda178a0b3078a5fec14cfc8c1101a5 100644 (file)
--- a/src/gromacs/utility/flags.h
+++ b/src/gromacs/utility/flags.h
@@ -1,7 +1,7 @@
 /*
  * This file is part of the GROMACS molecular simulation package.
  *
- * Copyright (c) 2010,2011,2012,2013,2019, by the GROMACS development team, led by
+ * Copyright (c) 2010,2011,2012,2013,2019,2020, 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,7 +49,8 @@ namespace gmx
 /*! \brief
  * Template class for typesafe handling of combination of flags.
  *
- * \tparam FlagType An enumerated type that holds the possible single flags.
+ * \tparam FlagType An enumerated type that holds the possible single flags. The enum
+ * must be of unsigned underlying type.
  *
  * This class is not used publicly, but is present in an installed header
  * because it is used internally in public template classes.
@@ -66,6 +67,8 @@ template<typename FlagType>
 class FlagsTemplate
 {
 public:
+    static_assert(std::is_enum_v<FlagType> && std::is_unsigned_v<std::underlying_type_t<FlagType>>,
+                  "Flags must be an unsigned enum type.");
     //! Creates a flags object with no flags set.
     FlagsTemplate() : flags_(0) {}
     //! Creates a flags object from a single flag.
@@ -120,7 +123,7 @@ private:
     //! Creates a flags object with the given flags.
     explicit FlagsTemplate(unsigned long flags) : flags_(flags) {}
 
-    unsigned long flags_;
+    uint64_t flags_;
 };
 
 } // namespace gmx