From 290a2689fc252ccc711beffc0293cef11f585586 Mon Sep 17 00:00:00 2001
From: Michael Davidsaver <mdavidsaver@gmail.com>
Date: Wed, 15 Jul 2020 11:12:53 -0700
Subject: [PATCH] add Value::lookup()

---
 documentation/value.rst | 13 +++++++++++
 src/data.cpp            | 51 ++++++++++++++++++++++++++++++++++++-----
 src/pvxs/data.h         | 26 ++++++++++++++++-----
 test/testdata.cpp       |  6 ++++-
 4 files changed, 83 insertions(+), 13 deletions(-)

diff --git a/documentation/value.rst b/documentation/value.rst
index f9d528f..4d5deee 100644
--- a/documentation/value.rst
+++ b/documentation/value.rst
@@ -72,6 +72,17 @@ All operations on an invalid Value should be safe and well defined.
 In this example, the operator[] lookup of a non-existant field returns an invalid Value.
 Attempting to extract an integer from this will then throw a `pvxs::NoField` exception.
 
+Value
+-----
+
+Field Lookup
+^^^^^^^^^^^^
+
+Access to members of structured types is accomplished through `pvxs::Value::operator[]` or `pvxs::Value::lookup`.
+These two methods differ in how errors are communicated.
+operator[] will return an "invalid" or "empty" Value if the expression does not address a member.
+lookup() will throw an exception describing where and how expression evaluation failed.
+
 Iteration
 ^^^^^^^^^
 
@@ -103,6 +114,8 @@ and will always appear as empty.
 
 .. doxygenstruct:: pvxs::NoConvert
 
+.. doxygenstruct:: pvxs::LookupError
+
 Array fields
 ------------
 
diff --git a/src/data.cpp b/src/data.cpp
index e1212da..d618e4e 100644
--- a/src/data.cpp
+++ b/src/data.cpp
@@ -24,6 +24,13 @@ NoConvert::NoConvert()
 
 NoConvert::~NoConvert() {}
 
+LookupError::LookupError(const std::string& msg)
+    :std::runtime_error(msg)
+{}
+
+LookupError::~LookupError() {}
+
+
 std::shared_ptr<const impl::FieldDesc>
 Value::Helper::type(const Value& v)
 {
@@ -636,7 +643,7 @@ bool Value::tryCopyIn(const void *ptr, StoreType type)
     }
 }
 
-void Value::traverse(const std::string &expr, bool modify)
+void Value::traverse(const std::string &expr, bool modify, bool dothrow)
 {
     size_t pos=0;
     bool maybedot = false;
@@ -656,6 +663,8 @@ void Value::traverse(const std::string &expr, bool modify)
                 // at top
                 store.reset();
                 desc = nullptr;
+                if(dothrow)
+                    throw LookupError(SB()<<"Can't traverse to parent of root with '"<<expr<<"'");
                 break;
             }
 
@@ -670,6 +679,8 @@ void Value::traverse(const std::string &expr, bool modify)
                 if(expr[pos]!='.') {
                     store.reset();
                     desc = nullptr;
+                    if(dothrow)
+                        throw LookupError(SB()<<"expected '.' at "<<pos<<" in '"<<expr<<"'");
                     break;
                 }
                 maybedot = false;
@@ -680,7 +691,9 @@ void Value::traverse(const std::string &expr, bool modify)
 
             decltype (desc->mlookup)::const_iterator it;
 
-            if(sep>0 && (it=desc->mlookup.find(expr.substr(pos, sep-pos)))!=desc->mlookup.end()) {
+            const auto& name = expr.substr(pos, sep-pos);
+
+            if(sep>0 && (it=desc->mlookup.find(name))!=desc->mlookup.end()) {
                 // found it
                 auto next = desc+it->second;
                 decltype(store) value(store, store.get()+it->second);
@@ -692,6 +705,8 @@ void Value::traverse(const std::string &expr, bool modify)
                 // no such member
                 store.reset();
                 desc = nullptr;
+                if(dothrow)
+                    throw LookupError(SB()<<"no such member '"<<name<<"' in '"<<expr<<"'");
             }
 
         } else if(desc->code.code==TypeCode::Union || desc->code.code==TypeCode::Any) {
@@ -732,6 +747,8 @@ void Value::traverse(const std::string &expr, bool modify)
                             // traversing const Value, can't select Union
                             store.reset();
                             desc = nullptr;
+                            if(dothrow)
+                                throw LookupError(SB()<<"traversing const Value, can't select Union in '"<<expr<<"'");
                         }
                     }
                 }
@@ -739,6 +756,8 @@ void Value::traverse(const std::string &expr, bool modify)
                 // expected "->"
                 store.reset();
                 desc = nullptr;
+                if(dothrow)
+                    throw LookupError(SB()<<"expected -> in '"<<expr<<"'");
             }
 
         } else if(desc->code.isarray() && desc->code.kind()==Kind::Compound) {
@@ -766,33 +785,53 @@ void Value::traverse(const std::string &expr, bool modify)
                     // wrong element type or out of range
                     store.reset();
                     desc = nullptr;
+                    if(dothrow)
+                        throw std::runtime_error(SB()<<"wrong element type or out of range in '"<<expr<<"'");
                 }
 
             } else {
                 // syntax error
                 store.reset();
                 desc = nullptr;
+                if(dothrow)
+                    throw std::runtime_error(SB()<<"indexing syntax error in '"<<expr<<"'");
             }
 
         } else {
             // syntax error or wrong field type (can't index scalar array)
             store.reset();
             desc = nullptr;
+            if(dothrow)
+                throw std::runtime_error(SB()<<"indexing syntax error or wrong field type (can't index scalar array) in '"<<expr<<"'");
         }
     }
 }
 
-Value Value::operator[](const char *name)
+Value Value::operator[](const std::string& name)
 {
     Value ret(*this);
-    ret.traverse(name, true);
+    ret.traverse(name, true, false);
     return ret;
 }
 
-const Value Value::operator[](const char *name) const
+const Value Value::operator[](const std::string& name) const
 {
     Value ret(*this);
-    ret.traverse(name, false);
+    ret.traverse(name, false, false);
+    return ret;
+}
+
+Value Value::lookup(const std::string& name)
+{
+    Value ret(*this);
+    ret.traverse(name, true, true);
+    return ret;
+}
+
+const Value Value::lookup(const std::string& name) const
+{
+    Value ret(*this);
+    ret.traverse(name, false, true);
     return ret;
 }
 
diff --git a/src/pvxs/data.h b/src/pvxs/data.h
index 4b63034..ff07920 100644
--- a/src/pvxs/data.h
+++ b/src/pvxs/data.h
@@ -445,6 +445,12 @@ struct PVXS_API NoConvert : public std::runtime_error
     virtual ~NoConvert();
 };
 
+struct PVXS_API LookupError : public std::runtime_error
+{
+    explicit LookupError(const std::string& msg);
+    virtual ~LookupError();
+};
+
 /** Generic data container
  *
  * References a single data field, which may be free-standing (eg. "int x = 5;")
@@ -644,7 +650,7 @@ public:
 
     // Struct/Union access
 private:
-    void traverse(const std::string& expr, bool modify);
+    void traverse(const std::string& expr, bool modify, bool dothrow);
 public:
 
     /** Attempt to access a descendant field.
@@ -653,7 +659,7 @@ public:
      * * name of a child field.  eg. "value"
      * * name of a descendant field.  eg "alarm.severity"
      * * element of an array of structures.  eg "dimension[0]"
-     * * name of a union field.  eg. "booleanValue"
+     * * name of a union field.  eg. "->booleanValue"
      *
      * These may be composed.  eg.
      *
@@ -662,10 +668,18 @@ public:
      *
      * @returns A valid() Value if the descendant field exists, otherwise an invalid Value.
      */
-    Value operator[](const char *name);
-    inline Value operator[](const std::string& name) { return (*this)[name.c_str()]; }
-    const Value operator[](const char *name) const;
-    inline const Value operator[](const std::string& name) const { return (*this)[name.c_str()]; }
+    Value operator[](const std::string& name);
+    const Value operator[](const std::string& name) const;
+
+    /** Attempt to access a descendant field, or throw exception.
+     *
+     * Acts like operator[] on success, but throws a (hopefully descriptive)
+     * exception instead of returning an invalid Value.
+     *
+     * @throws LookupError If the lookup can not be satisfied
+     */
+    Value lookup(const std::string& name);
+    const Value lookup(const std::string& name) const;
 
     //! Number of child fields.
     //! only Struct, StructA, Union, UnionA return non-zero
diff --git a/test/testdata.cpp b/test/testdata.cpp
index 082259b..8b67c00 100644
--- a/test/testdata.cpp
+++ b/test/testdata.cpp
@@ -27,6 +27,10 @@ void testTraverse()
 
     testOk1(!top["<"].valid());
 
+    testThrows<std::runtime_error>([&top](){
+        top.lookup("<");
+    });
+
     {
         auto top2 = top["value<"];
         testOk1(top.equalType(top2));
@@ -310,7 +314,7 @@ void testAssignSimilar()
 
 MAIN(testdata)
 {
-    testPlan(92);
+    testPlan(93);
     testSetup();
     testTraverse();
     testAssign();