-Wall Compliant (#349)

* several warnings fixed

* fix remaining warnings

* Committing clang-format changes

---------

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

include/pluginplay/cache/module_cache.hpp

@@ -22,7 +22,7 @@
 
 namespace pluginplay::cache {
 namespace detail_ {
-class ModuleCachePIMPL;
+struct ModuleCachePIMPL;
 }
 
 /** @brief Class storing the memoized results for a particular module

include/pluginplay/cache/module_manager_cache.hpp

@@ -20,7 +20,7 @@
 
 namespace pluginplay::cache {
 namespace detail_ {
-class ModuleManagerCachePIMPL;
+struct ModuleManagerCachePIMPL;
 }
 
 class ModuleCache;

include/pluginplay/fields/bounds_checking/print_bounds.hpp

@@ -21,10 +21,10 @@
 namespace pluginplay::bounds_checking {
 
 template<typename T, typename Op>
-class CompareTo;
+struct CompareTo;
 
 template<typename T>
-class InRange;
+struct InRange;
 
 namespace detail_ {
 

include/pluginplay/fields/module_input.ipp

@@ -19,7 +19,7 @@
 namespace pluginplay {
 
 inline ModuleInput& ModuleInput::operator=(const ModuleInput& rhs) {
-    return *this = std::move(ModuleInput(rhs));
+    return *this = ModuleInput(rhs);
 }
 
 template<typename T>
@@ -118,7 +118,7 @@ auto& ModuleInput::change(T&& new_value) {
     // By convention we store strings as std::string, so we have to catch when
     // the user passed a C-string in
     if constexpr(detail_::IsCString<T>::value) {
-        change_(std::move(wrap_value_(std::string(new_value))));
+        change_(wrap_value_(std::string(new_value)));
     } else {
         change_(std::move(wrap_value_(std::forward<T>(new_value))));
     }

include/pluginplay/fields/module_result.ipp

@@ -22,7 +22,7 @@ namespace pluginplay {
 //------------------------------Implementations---------------------------------
 
 inline ModuleResult& ModuleResult::operator=(const ModuleResult& rhs) {
-    return *this = std::move(ModuleResult(rhs));
+    return *this = ModuleResult(rhs);
 }
 
 template<typename T>
@@ -44,7 +44,7 @@ void ModuleResult::change(T&& new_value) {
     if constexpr(is_shared_any)
         change_(new_value);
     else if constexpr(detail_::IsCString<T>::value)
-        change_(std::move(wrap_value_(std::string(new_value))));
+        change_(wrap_value_(std::string(new_value)));
     else
         change_(std::move(wrap_value_(std::forward<T>(new_value))));
 }

include/pluginplay/module.hpp

@@ -28,7 +28,7 @@ class ModuleBase;
 
 namespace detail_ {
 class ModulePIMPL;
-class ModuleManagerPIMPL;
+struct ModuleManagerPIMPL;
 } // namespace detail_
 
 /** @brief The public API of all modules.

include/pluginplay/module_manager.hpp

@@ -24,7 +24,7 @@
 namespace pluginplay {
 
 namespace detail_ {
-class ModuleManagerPIMPL;
+struct ModuleManagerPIMPL;
 }
 
 /** @brief Class responsible for manipulating

include/pluginplay/submodule_request.hpp

@@ -441,7 +441,7 @@ class SubmoduleRequest {
 
 inline SubmoduleRequest& SubmoduleRequest::operator=(
   const SubmoduleRequest& rhs) {
-    return *this = std::move(SubmoduleRequest(rhs));
+    return *this = SubmoduleRequest(rhs);
 }
 
 template<typename T>

src/pluginplay/detail_/module_manager_pimpl.hpp

@@ -167,7 +167,7 @@ struct ModuleManagerPIMPL {
      */
     void copy_module(const type::key& old_key, type::key new_key) {
         assert_unique_key_(new_key);
-        Module mod = std::move(m_modules.at(old_key)->unlocked_copy());
+        Module mod = m_modules.at(old_key)->unlocked_copy();
         auto ptr   = std::make_shared<Module>(std::move(mod));
         ptr->set_name(new_key);
         m_modules.emplace(std::move(new_key), ptr);

src/pluginplay/fields/detail_/module_result_pimpl.hpp

@@ -232,7 +232,7 @@ inline void ModuleResultPIMPL::set_type(type::rtti new_type) {
 
 inline void ModuleResultPIMPL::set_value(shared_any new_value) {
     // This'll throw bad optional_access if we haven't set the type
-    auto t            = type();
+    type();
     bool is_corr_type = (*m_type_check_)(*new_value);
     if(!is_corr_type) {
         std::string msg{"Value is not of type: "};

src/python/export_module_manager.cpp

@@ -24,9 +24,7 @@
 namespace pluginplay {
 
 void export_module_manager(py_module_reference m) {
-    using module_base_ptr = ModuleManager::module_base_ptr;
-    using at_fxn          = Module& (ModuleManager::*)(const type::key&);
-    using add_mod_fxn     = void (ModuleManager::*)(type::key, module_base_ptr);
+    using at_fxn = Module& (ModuleManager::*)(const type::key&);
 
     using py_obj = pybind11::object;
     using python::PythonWrapper;

tests/cxx/doc_snippets/modules.hpp

@@ -26,7 +26,7 @@ DECLARE_MODULE(ClassicalForce);
 template<typename FloatingPointType>
 DECLARE_MODULE(TemplatedCoulombsLaw);
 
-extern template class TemplatedCoulombsLaw<float>;
-extern template class TemplatedCoulombsLaw<double>;
+extern template struct TemplatedCoulombsLaw<float>;
+extern template struct TemplatedCoulombsLaw<double>;
 
 } // namespace pluginplay_examples

tests/cxx/doc_snippets/templated_coulombs_law.cpp

@@ -75,7 +75,7 @@ TEMPLATED_MODULE_RUN(TemplatedCoulombsLaw, FloatingPointType) {
     return prop_type::wrap_results(rv, E);
 }
 
-template class TemplatedCoulombsLaw<float>;
-template class TemplatedCoulombsLaw<double>;
+template struct TemplatedCoulombsLaw<float>;
+template struct TemplatedCoulombsLaw<double>;
 
 } // namespace pluginplay_examples

tests/cxx/doc_snippets/test_tutorial_module_documents.cpp

@@ -49,8 +49,9 @@ TEST_CASE("tutorial_modules_docs") {
         // Generate module documentation at the given path
         pluginplay::printing::document_modules(mm, docs_path);
 
-        REQUIRE(std::distance(std::filesystem::directory_iterator(docs_path),
-                              std::filesystem::directory_iterator{}) ==
-                mm.size() + 1); // +1 for index.rst
+        decltype(mm.size()) distance(
+          std::distance(std::filesystem::directory_iterator(docs_path),
+                        std::filesystem::directory_iterator{}));
+        REQUIRE(distance == mm.size() + 1); // +1 for index.rst
     }
 }

tests/cxx/unit_tests/pluginplay/any/any_field.cpp

@@ -308,7 +308,7 @@ struct ABaseClass {
 struct DerivedClass : ABaseClass {
     double y;
 
-    DerivedClass(double _y, int _x) : y(_y), ABaseClass(_x) {}
+    DerivedClass(double _y, int _x) : ABaseClass(_x), y(_y) {}
 
     bool operator==(const DerivedClass& rhs) const noexcept {
         return y == rhs.y && ABaseClass::operator==(rhs);

tests/cxx/unit_tests/pluginplay/any/detail_/any_field_wrapper_traits.cpp

@@ -27,10 +27,10 @@ TEMPLATE_TEST_CASE("is_any_field_wrapper", "", testing::types2test) {
 }
 
 TEMPLATE_TEST_CASE("disable_if_any_field_wrapper_t", "", testing::types2test) {
-    using T            = TestType;
-    using wrapper_type = AnyFieldWrapper<T>;
+    using T = TestType;
     STATIC_REQUIRE(std::is_same_v<int, disable_if_any_field_wrapper_t<T, int>>);
     // Should fail to compile, uncomment to check
+    // using wrapper_type = AnyFieldWrapper<T>;
     // STATIC_REQUIRE(
     //  std::is_same_v<int, disable_if_any_field_wrapper_t<wrapper_type>>);
 }

tests/cxx/unit_tests/pluginplay/cache/database/value_proxy_mapper.cpp

@@ -132,7 +132,6 @@ TEMPLATE_LIST_TEST_CASE("ValueProxyMapper", "", testing::test_types) {
         // Still in ProxyMapMaker
         REQUIRE(pmapper->count(value0));
         // Check that we called backup on pmapper
-        TestType v{};
         REQUIRE(pproxy_sub_sub_db->count(1.23));
         REQUIRE(pproxy_sub_sub_db->at(1.23).get() == mapped_value0["foo"]);
         // No longer in sub_db

tests/cxx/unit_tests/pluginplay/cache/uuid_mapper.cpp

@@ -34,7 +34,7 @@ TEMPLATE_LIST_TEST_CASE("UUIDMapper", "", testing::test_types) {
 
     auto [pinner, db] = testing::make_nested_native<key_type, uuid_type>();
 
-    key_type key0;
+    auto key0 = key_type{};
     auto key1 = testing::lexical_cast<key_type>("42");
 
     uuid_db_type uuid_db(std::make_unique<decltype(db)>(std::move(db)));
@@ -110,7 +110,6 @@ TEST_CASE("UUIDMapper<AnyField>") {
       testing::make_transposer<any_type, uuid_type>();
 
     auto sub_db = std::make_unique<decltype(sub)>(std::move(sub));
-    auto psub   = sub_db.get();
     uuid_db_type db(std::move(sub_db));
 
     SECTION("Wrap a double by const ref") {

tests/cxx/unit_tests/pluginplay/detail_/module_pimpl.cpp

@@ -386,7 +386,7 @@ TEST_CASE("ModulePIMPL") {
             auto mod = make_module_pimpl_with_cache<RealDeal>();
             auto in  = mod.inputs();
             in.at("Option 1").change(1);
-            auto result = mod.run(in).at("Result 1").value<int>();
+            mod.run(in).at("Result 1").value<int>();
             REQUIRE(mod.is_cached(in));
         }
     }
@@ -395,11 +395,11 @@ TEST_CASE("ModulePIMPL") {
         auto mod = make_module_pimpl_with_cache<RealDeal>();
         auto in  = mod.inputs();
         in.at("Option 1").change(1);
-        auto result1 = mod.run(in).at("Result 1").value<int>();
+        mod.run(in).at("Result 1").value<int>();
         REQUIRE(mod.is_cached(in));
         mod.reset_cache();
         REQUIRE_FALSE(mod.is_cached(in));
-        auto result2 = mod.run(in).at("Result 1").value<int>();
+        mod.run(in).at("Result 1").value<int>();
         REQUIRE(mod.is_cached(in));
     }
 

tests/cxx/unit_tests/pluginplay/examples/lambda_module.cpp

@@ -48,7 +48,7 @@
 // For the purposes of this tutorial we reuse the property types from our
 // property type tutorial and the modules from our module tutorial. The next
 // two lines include the relevant header files
-#include "workflow.hpp"
+#include "load_modules.hpp"
 #include <pluginplay/pluginplay.hpp>
 
 // TUTORIAL_START_SKIP

tests/cxx/unit_tests/pluginplay/examples/load_modules.hpp

@@ -0,0 +1,74 @@
+/*
+ * Copyright 2022 NWChemEx-Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+/* The purpose of this tutorial is to showcase the packaging of module groups.
+ *
+ * For all intents an purposes the ``ModuleManager`` class can be thought of as
+ * the code embodiment of the pluginplay. It is responsible for storing all of
+ * the available modules, assembling the call graph, integrating the modules
+ * into the actual runtime, and checkpointing the progress of the program. To
+ * get the ``ModuleManager`` class one includes ``ModuleManager.hpp``. The
+ * remainder of the following header files are needed for this tutorial.
+ */
+#pragma once
+#include "writing_a_module.hpp"
+#include "writing_a_property_type.hpp"
+#include <pluginplay/module_manager.hpp>
+
+/* Populating the ModuleManager
+ * ----------------------------
+ *
+ * Before the ``ModuleManager`` can manage some modules, it needs modules. The
+ * primary means of providing the ``ModuleManager`` with modules is manually.
+ * While this sounds tedious, and it is, it only needs to be done once. To this
+ * end libraries meant for use with the pluginplay will provide a function
+ * ``load_modules`` which takes a ``ModuleManager`` instance and adds to it
+ * the modules contained within that library. It is also typically during this
+ * function call that default modules are assigned.
+ *
+ * The following function indicates how this would be done assuming that our
+ * ``Area``, ``PrismVolume``, ``Rectangle`` and ``Prism`` classes are part of a
+ * library meant for use with the pluginplay.
+ */
+inline void load_modules(pluginplay::ModuleManager& mm) {
+    /* The function starts off by registering the two modules provided by our
+     * library. Registration involves two things:
+     *
+     * - creation of an instance of the module
+     * - association of that instance with a module key
+     *
+     * By taking an already instantiated object, the pluginplay remains
+     * decoupled from the procedure for initializing the module. It remains
+     * decoupled from the modules' C++ types (here ``Rectangle`` and ``Prism``)
+     * by taking the objects via pointers to the ``ModuleBase`` class it
+     * necessarily inherits from. To avoid gotchas related to handling
+     * polymorphic data types the pluginplay requires that the user provide the
+     * module as a ``shared_ptr``. The provided key can be used to retrieve the
+     * module (*vide infra*). The key must be unique (if it's not an error will
+     * be raised).
+     */
+    mm.add_module("Rectangle", std::make_shared<Rectangle>());
+    mm.add_module("Prism", std::make_shared<Prism>());
+
+    /* Since our modules introduced two new property types we should also set
+     * default modules for each of them. These defaults will be used when
+     * a module needs, say an ``Area``, and the user has not specifically stated
+     * which submodule to use. Setting the defaults for our library is easy
+     * since it only provides one of each...
+     */
+    mm.set_default<Area>("Rectangle");
+    mm.set_default<PrismVolume>("Prism");
+} // end load_modules()

tests/cxx/unit_tests/pluginplay/examples/workflow.hpp

@@ -17,63 +17,17 @@
 /* The purpose of this tutorial is to showcase an end-to-end use of the
  * pluginplay from C++.
  *
- * For all intents an purposes the ``ModuleManager`` class can be thought of as
+ * For all intents and purposes the ``ModuleManager`` class can be thought of as
  * the code embodiment of the pluginplay. It is responsible for storing all of
  * the available modules, assembling the call graph, integrating the modules
  * into the actual runtime, and checkpointing the progress of the program. To
  * get the ``ModuleManager`` class one includes ``ModuleManager.hpp``. The
  * remainder of the following header files are needed for this tutorial.
  */
 #pragma once
-#include "writing_a_module.hpp"
-#include "writing_a_property_type.hpp"
+#include "load_modules.hpp"
 #include <pluginplay/module_manager.hpp>
 
-/* Populating the ModuleManager
- * ----------------------------
- *
- * Before the ``ModuleManager`` can manage some modules, it needs modules. The
- * primary means of providing the ``ModuleManager`` with modules is manually.
- * While this sounds tedious, and it is, it only needs to be done once. To this
- * end libraries meant for use with the pluginplay will provide a function
- * ``load_modules`` which takes a ``ModuleManager`` instance and adds to it
- * the modules contained within that library. It is also typically during this
- * function call that default modules are assigned.
- *
- * The following function indicates how this would be done assuming that our
- * ``Area``, ``PrismVolume``, ``Rectangle`` and ``Prism`` classes are part of a
- * library meant for use with the pluginplay.
- */
-inline void load_modules(pluginplay::ModuleManager& mm) {
-    /* The function starts off by registering the two modules provided by our
-     * library. Registration involves two things:
-     *
-     * - creation of an instance of the module
-     * - association of that instance with a module key
-     *
-     * By taking an already instantiated object, the pluginplay remains
-     * decoupled from the procedure for initializing the module. It remains
-     * decoupled from the modules' C++ types (here ``Rectangle`` and ``Prism``)
-     * by taking the objects via pointers to the ``ModuleBase`` class it
-     * necessarily inherits from. To avoid gotchas related to handling
-     * polymorphic data types the pluginplay requires that the user provide the
-     * module as a ``shared_ptr``. The provided key can be used to retrieve the
-     * module (*vide infra*). The key must be unique (if it's not an error will
-     * be raised).
-     */
-    mm.add_module("Rectangle", std::make_shared<Rectangle>());
-    mm.add_module("Prism", std::make_shared<Prism>());
-
-    /* Since our modules introduced two new property types we should also set
-     * default modules for each of them. These defaults will be used when
-     * a module needs, say an ``Area``, and the user has not specifically stated
-     * which submodule to use. Setting the defaults for our library is easy
-     * since it only provides one of each...
-     */
-    mm.set_default<Area>("Rectangle");
-    mm.set_default<PrismVolume>("Prism");
-} // end load_modules()
-
 /* Running a Computation
  * ---------------------
  *
@@ -115,7 +69,9 @@ static void example_main() {
      * ``Rectangle`` module since that's what the default is set to).
      */
     std::vector<double> dims{1.23, 4.56, 7.89};
-    auto [volume, area] = mm.run_as<PrismVolume>("Prism", dims);
+    auto [area, volume] = mm.run_as<PrismVolume>("Prism", dims);
+    bool correct = ((area == (1.23 * 4.56)) && (volume == (area * 7.89)));
+    if(!correct) throw std::runtime_error("Miscalculated");
 } // end example_main()
 
 /* That's it. That's the end of the workflow tutorial. From the end-user's

tests/cxx/unit_tests/pluginplay/examples/writing_a_module.hpp

@@ -231,7 +231,7 @@ class Rectangle : public pluginplay::ModuleBase {
          * the string summary.
          */
         const auto area      = base * height;
-        const auto perimeter = 2 * (base + height);
+        const auto perimeter = 2 * (base1 + height1);
         const auto summary = name + " has an area of " + std::to_string(area) +
                              " and a perimeter of " + std::to_string(perimeter);