From 48d95620247b55ed629848cb613ed06dc65762c1 Mon Sep 17 00:00:00 2001
From: Daniele <Iximiel@users.noreply.github.com>
Date: Thu, 10 Oct 2024 10:30:58 +0200
Subject: [PATCH] More helpful exception text for Registerbase (#1080)

* tiny changes in RegisterBase

* set up a clearer exception for RegisterBase (in ActionRegister)

* set up a clearer exception for RegisterBase (in CLToolRegister)

* added a few lines of documentation fro the exception

---------

Co-authored-by: Daniele Rapetti <5535617+Iximiel@users.noreply.github.com>
---
 src/core/ActionRegister.cpp | 16 +++++++++++++-
 src/core/CLToolRegister.cpp |  5 ++++-
 src/core/RegisterBase.h     | 43 ++++++++++++++++++++++++++++++++-----
 3 files changed, 57 insertions(+), 7 deletions(-)

diff --git a/src/core/ActionRegister.cpp b/src/core/ActionRegister.cpp
index 9054effa76..616a1cafe8 100644
--- a/src/core/ActionRegister.cpp
+++ b/src/core/ActionRegister.cpp
@@ -20,6 +20,7 @@
    along with plumed.  If not, see <http://www.gnu.org/licenses/>.
 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ */
 #include "ActionRegister.h"
+#include "ModuleMap.h"
 #include "Action.h"
 #include <algorithm>
 
@@ -35,7 +36,7 @@ std::unique_ptr<Action> ActionRegister::create(const ActionOptions&ao) {
   return create(images,ao);
 }
 
-std::unique_ptr<Action> ActionRegister::create(const std::vector<void*> & images,const ActionOptions&ao) {
+std::unique_ptr<Action> ActionRegister::create(const std::vector<void*> & images,const ActionOptions&ao) try {
   if(ao.line.size()<1)return nullptr;
 
   auto content=get(images,ao.line[0]);
@@ -45,6 +46,17 @@ std::unique_ptr<Action> ActionRegister::create(const std::vector<void*> & images
   auto fullPath=getFullPath(images,ao.line[0]);
   nao.setFullPath(fullPath);
   return content.create(nao);
+} catch (PLMD::ExceptionRegisterError &e ) {
+  auto& actionName = e.getMissingKey();
+  e <<"Action \"" << actionName << "\" is not known.";
+  if (getModuleMap().count(actionName)>0) {
+    e << "\nAn Action named \""
+      <<actionName
+      <<"\" is available in module \""
+      << getModuleMap().at(actionName)
+      << "\".\nPlease consider installing PLUMED with that module enabled.";
+  }
+  throw e;
 }
 
 bool ActionRegister::printManual(const std::string& action, const bool& vimout, const bool& spellout) {
@@ -64,6 +76,7 @@ bool ActionRegister::printManual(const std::string& action, const bool& vimout,
 }
 
 bool ActionRegister::printTemplate(const std::string& action, bool include_optional) {
+  //no need to insert the try/catch block: check will ensure that action is known
   if( check(action) ) {
     Keywords keys; keys.thisactname = action;
     get(action).keys(keys);
@@ -85,6 +98,7 @@ ActionRegister::ID ActionRegister::add(std::string key,creator_pointer cp,keywor
 }
 
 bool ActionRegister::getKeywords(const std::string& action, Keywords& keys) {
+  //no need to insert the try/catch block: check will ensure that action is known
   if(check(action)) {
     keys.thisactname = action;
     get(action).keys(keys);
diff --git a/src/core/CLToolRegister.cpp b/src/core/CLToolRegister.cpp
index 3361149776..d9535b1f16 100644
--- a/src/core/CLToolRegister.cpp
+++ b/src/core/CLToolRegister.cpp
@@ -38,11 +38,14 @@ std::unique_ptr<CLTool> CLToolRegister::create(const CLToolOptions&ao) {
   return create(images,ao);
 }
 
-std::unique_ptr<CLTool> CLToolRegister::create(const std::vector<void*> & images,const CLToolOptions&ao) {
+std::unique_ptr<CLTool> CLToolRegister::create(const std::vector<void*> & images,const CLToolOptions&ao) try {
   if(ao.line.size()<1)return nullptr;
   auto & content=get(images,ao.line[0]);
   CLToolOptions nao( ao,content.keys );
   return content.create(nao);
+} catch (PLMD::ExceptionRegisterError &e ) {
+  auto& toolName = e.getMissingKey();
+  throw e <<"CL tool \"" << toolName << "\" is not known.";
 }
 
 CLToolRegister::ID CLToolRegister::add(std::string key,creator_pointer cp,keywords_pointer kp) {
diff --git a/src/core/RegisterBase.h b/src/core/RegisterBase.h
index d08c3e4a4c..8f6e80124e 100644
--- a/src/core/RegisterBase.h
+++ b/src/core/RegisterBase.h
@@ -24,6 +24,7 @@
 
 #include "tools/Exception.h"
 #include <string>
+#include <string_view>
 #include <map>
 #include <memory>
 #include <iostream>
@@ -95,6 +96,34 @@ class Register {
   friend std::ostream & operator<<(std::ostream &log,const Register &reg);
 };
 
+/// Class representing an error in a register
+class ExceptionRegisterError :
+  public Exception {
+  /// The missing key
+  std::string missingKey;
+public:
+  using Exception::Exception;
+  /// Sets the missing key
+  /// \param key The missing key
+  /// \return This exception
+  ///
+  /// ExceptionRegisterError can be used as a builder pattern:
+  /// `throw ExceptionRegisterError().setMissingKey(key);`
+  ///
+  /// the key can be retrieved with ExceptionRegisterError::getMissingKey()
+  ExceptionRegisterError& setMissingKey (std::string_view key) {
+    missingKey=key;
+    return *this;
+  }
+  /// Returns the missing key
+  const std::string& getMissingKey() const {return missingKey;}
+  template<typename T>
+  ExceptionRegisterError& operator<<(const T & x) {
+    *static_cast<Exception*>(this) <<x;
+    return *this;
+  }
+};
+
 /// General register.
 /// This class provide a generic implementation based on the content of the Register
 template<class Content>
@@ -208,7 +237,9 @@ const Content & RegisterBase<Content>::get(const std::vector<void*> & images,con
     auto qualified_key=imageToString(*image) + ":" + key;
     if(m.count(qualified_key)>0) return m.find(qualified_key)->second->content;
   }
-  plumed_assert(m.count(key)>0);
+  if (m.count(key) == 0 ) {
+    throw ExceptionRegisterError().setMissingKey(key);
+  }
   return m.find(key)->second->content;
 }
 
@@ -220,7 +251,9 @@ const std::string & RegisterBase<Content>::getFullPath(const std::vector<void*>
     auto qualified_key=imageToString(*image) + ":" + key;
     if(m.count(qualified_key)>0) return m.find(qualified_key)->second->fullPath;
   }
-  plumed_assert(m.count(key)>0);
+  if (m.count(key) == 0 ) {
+    throw ExceptionRegisterError().setMissingKey(key);
+  }
   return m.find(key)->second->fullPath;
 }
 
@@ -228,7 +261,9 @@ template<class Content>
 const Content & RegisterBase<Content>::get(const std::string & key) const {
   // lock map for reading
   std::shared_lock<std::shared_mutex> lock(mutex);
-  plumed_assert(m.count(key)>0);
+  if (m.count(key) == 0 ) {
+    throw ExceptionRegisterError().setMissingKey(key);
+  }
   return m.find(key)->second->content;
 }
 
@@ -288,5 +323,3 @@ void RegisterBase<Content>::clearStaged() noexcept {
 }
 
 #endif
-
-