Add ReadAttrVec, ReadAttrArr, ReadAttrStr, and ReadAttrNum to XML utils.

Moving forward, the compiler objects should have encapsulated data, therefore the old ReadAttr* methods are being deprecated as they require raw pointers to read into.

No external changes other than minor improvements to ReadAttr and ReadVector.

PiperOrigin-RevId: 550880401
Change-Id: I632f40322ccd935f9af9b69e66b056c5cda6face

src/xml/xml_util.cc

@@ -12,12 +12,14 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-#include <cfloat>
+#include <algorithm>
+#include <array>
 #include <cmath>
 #include <cstddef>
 #include <cstdio>
 #include <cstdlib>
 #include <cstring>
+#include <functional>
 #include <iostream>
 #include <limits>
 #include <optional>
@@ -44,7 +46,7 @@ using tinyxml2::XMLElement;
 namespace mju = ::mujoco::util;
 
 template <typename T>
-std::optional<T> ParseInfOrNan(const std::string& s) {
+static std::optional<T> ParseInfOrNan(const std::string& s) {
   const char* str = s.c_str();
   if constexpr (std::is_floating_point_v<T>) {
     T sign = 1;
@@ -270,15 +272,6 @@ static void printspace(std::stringstream& str, int n, const char* space) {
 }
 
 
-// max
-static int _max(int a, int b) {
-  if (a>b) {
-    return a;
-  } else {
-    return b;
-  }
-}
-
 
 // print schema as text
 void mjXSchema::Print(std::stringstream& str, int level) {
@@ -294,11 +287,11 @@ void mjXSchema::Print(std::stringstream& str, int level) {
   }
 
   // attributes
-  int cnt = _max(baselen, 30);
+  int cnt = std::max(baselen, 30);
   for (int i=0; i<(int)attr.size(); i++) {
     if (cnt>60) {
       str << "\n";
-      printspace(str, (cnt = _max(30, baselen)), " ");
+      printspace(str, (cnt = std::max(30, baselen)), " ");
 
     }
 
@@ -508,6 +501,63 @@ XMLElement* mjXSchema::Check(XMLElement* elem, int level) {
 
 //---------------------------------- class mjXUtil implementation ----------------------------------
 
+// helper function to read multiple numerical values from an attribute
+// return false if the entire attribute wasn't read (max was reached)
+// throw error if syntax error while trying to read numerical data
+template<typename T>
+bool mjXUtil::ReadAttrValues(XMLElement* elem, const char* attr,
+                             std::function<void (int, T)> push, int max) {
+  const char* pstr = elem->Attribute(attr);
+  T item;
+
+  if (pstr == nullptr) {
+    return true;
+  }
+
+  // get input stream
+  std::string str = std::string(pstr);
+  std::istringstream strm(str);
+  std::string token;
+
+  // read numbers
+  for (int i = 0; (max < 0 || i < max) && !strm.eof(); ++i) {
+    strm >> token;
+    std::istringstream token_strm(token);
+    token_strm >> item;
+    if (token_strm.fail() || !token_strm.eof()) {
+      // C++ standard libraries do not always parse inf and nan as valid floating point values.
+      std::optional<T> maybe_result = ParseInfOrNan<T>(token);
+      if (maybe_result.has_value()) {
+        item = maybe_result.value();
+      } else {
+        throw mjXError(elem, "problem reading attribute '%s'", attr);
+      }
+    }
+
+    push(i, item);
+    if constexpr (std::is_floating_point_v<T>) {
+      if (std::isnan(item)) {
+        mju_warning("XML contains a 'NaN'. Please check it carefully.");
+      }
+    }
+    // clear any trailing whitespace
+    strm >> std::ws;
+  }
+
+  return strm.eof();
+}
+
+template bool mjXUtil::ReadAttrValues(XMLElement* elem, const char* attr,
+                                      std::function<void (int, double)> push, int max);
+template bool mjXUtil::ReadAttrValues(XMLElement* elem, const char* attr,
+                                      std::function<void (int, float)> push, int max);
+template bool mjXUtil::ReadAttrValues(XMLElement* elem, const char* attr,
+                                      std::function<void (int, int)> push, int max);
+template bool mjXUtil::ReadAttrValues(XMLElement* elem, const char* attr,
+                                      std::function<void (int, mjtByte)> push, int max);
+
+
+
 // compare two vectors
 template<typename T>
 bool mjXUtil::SameVector(const T* vec1, const T* vec2, int n) {
@@ -557,6 +607,76 @@ string mjXUtil::FindValue(const mjMap* map, int mapsz, int value) {
 
 
 
+// if attribute is present, return vector of numerical data
+template<typename T>
+std::optional<std::vector<T>> mjXUtil::ReadAttrVec(XMLElement* elem, const char* attr,
+                                                   bool required) {
+  std::vector<T> vec;
+  ReadAttrValues<T>(elem, attr, [&](int i, T num) { vec.push_back(num); });
+  if (!vec.size()) {
+    if (required) {
+      throw mjXError(elem, "required attribute missing: '%s'", attr);
+    } else {
+      return std::nullopt;
+    }
+  }
+
+  return vec;
+}
+
+template std::optional<std::vector<double>>
+mjXUtil::ReadAttrVec(XMLElement* elem, const char* attr, bool required);
+template std::optional<std::vector<float>>
+mjXUtil::ReadAttrVec(XMLElement* elem, const char* attr, bool required);
+template std::optional<std::vector<int>>
+mjXUtil::ReadAttrVec(XMLElement* elem, const char* attr, bool required);
+template std::optional<std::vector<mjtByte>>
+mjXUtil::ReadAttrVec(XMLElement* elem, const char* attr, bool required);
+
+
+
+// if attribute is present, return attribute as a string
+std::optional<std::string> mjXUtil::ReadAttrStr(XMLElement* elem, const char* attr,
+                                                bool required) {
+  const char* pstr = elem->Attribute(attr);
+
+  // check if attribute exists
+  if (pstr == nullptr) {
+    if (required) {
+      throw mjXError(elem, "required attribute missing: '%s'", attr);
+    } else {
+      return std::nullopt;
+    }
+  }
+
+  return std::string(pstr);
+}
+
+
+
+// if attribute is present, return numerical value of attribute
+template<typename T>
+std::optional<T> mjXUtil::ReadAttrNum(XMLElement* elem, const char* attr,
+                                      bool required) {
+  auto maybe_arr = ReadAttrArr<T, 1>(elem, attr, required);
+  if (!maybe_arr.has_value()) {
+    return std::nullopt;
+  }
+
+  return maybe_arr.value()[0];
+}
+
+template std::optional<double>
+mjXUtil::ReadAttrNum(XMLElement* elem, const char* attr, bool required);
+template std::optional<float>
+mjXUtil::ReadAttrNum(XMLElement* elem, const char* attr, bool required);
+template std::optional<int>
+mjXUtil::ReadAttrNum(XMLElement* elem, const char* attr, bool required);
+template std::optional<mjtByte>
+mjXUtil::ReadAttrNum(XMLElement* elem, const char* attr, bool required);
+
+
+
 // read attribute "attr" of element "elem"
 //  "len" is the number of floats or doubles to be read
 //  the content is returned in "text", the numeric data in "data"
@@ -564,59 +684,23 @@ string mjXUtil::FindValue(const mjMap* map, int mapsz, int value) {
 template<typename T>
 int mjXUtil::ReadAttr(XMLElement* elem, const char* attr, const int len,
                       T* data, string& text, bool required, bool exact) {
-  const char* pstr = elem->Attribute(attr);
+  auto maybe_vec = ReadAttrVec<T>(elem, attr, required);
-
+  if (!maybe_vec.has_value()) {
-  // check if attribute exists
+    return 0;
-  if (!pstr) {
-    if (required) {
-      throw mjXError(elem, "required attribute missing: '%s'", attr);
-    } else {
-      return 0;
-    }
   }
 
-  // convert to string
-  text = string(pstr);
-
-  // get input stream
-  istringstream strm(text);
-  std::string token;
-
-  // read numbers
-  int i = 0;
-  while (!strm.eof() && i < len) {
-    strm >> token;
-    istringstream token_strm(token);
-    token_strm >> data[i];
-    if (token_strm.fail() || !token_strm.eof()) {
-      // C++ standard libraries do not always parse inf and nan as valid floating point values.
-      std::optional<T> maybe_result = ParseInfOrNan<T>(token);
-      if (maybe_result.has_value()) {
-        data[i] = *maybe_result;
-      } else {
-        throw mjXError(elem, "problem reading attribute '%s'", attr);
-      }
-    }
-    if constexpr (std::is_floating_point_v<T>) {
-      if (std::isnan(data[i])) {
-        mju_warning("XML contains a 'NaN'. Please check it carefully.");
-      }
-    }
-    ++i;
-  }
-  strm >> std::ws;
-
   // check if there is not enough data
-  if (exact && i < len) {
+  if (exact && maybe_vec->size() < len) {
     throw mjXError(elem, "attribute '%s' does not have enough data", attr);
   }
 
   // check if there is too much data
-  if (!strm.eof()) {
+  if (maybe_vec->size() > len) {
     throw mjXError(elem, "attribute '%s' has too much data", attr);
   }
 
-  return i;
+  std::copy(maybe_vec->begin(), maybe_vec->end(), data);
+  return maybe_vec->size();
 }
 
 template int mjXUtil::ReadAttr(XMLElement* elem, const char* attr, const int len,
@@ -636,59 +720,36 @@ template int mjXUtil::ReadAttr(XMLElement* elem, const char* attr, const int len
 // read DOUBLE array into C++ vector, return number read
 int mjXUtil::ReadVector(XMLElement* elem, const char* attr,
                         vector<double>& vec, string& text, bool required) {
-  double buffer[1000];
+  auto maybe_vec = ReadAttrVec<double>(elem, attr, required);
-  int n = ReadAttr(elem, attr, 1000, buffer, text, required, false);
+  if (!maybe_vec.has_value()) {
-  if (n>0) {
+    return 0;
-    vec.resize(n);
-    memcpy(vec.data(), buffer, n*sizeof(double));
   }
-  return n;
+
+  vec = std::move(maybe_vec.value());
+  return vec.size();
 }
 
 
 
 // read text field
-bool mjXUtil::ReadAttrTxt(XMLElement* elem, const char* attr, string& text, bool required) {
+bool mjXUtil::ReadAttrTxt(tinyxml2::XMLElement* elem, const char* attr, string& text, bool required) {
-  const char* pstr = elem->Attribute(attr);
+  auto maybe_str = ReadAttrStr(elem, attr, required);
-
+  if (!maybe_str.has_value()) {
-  // check if attribute exists
+    return false;
-  if (!pstr) {
-    if (required) {
-      throw mjXError(elem, "required attribute missing: '%s'", attr);
-    } else {
-      return false;
-    }
   }
 
-  // read text
+  text = maybe_str.value();
-  text = string(pstr);
-
   return true;
 }
 
-
-
 // read single int
 bool mjXUtil::ReadAttrInt(XMLElement* elem, const char* attr, int* data, bool required) {
-  const char* pstr = elem->Attribute(attr);
+  auto maybe_int = ReadAttrNum<int>(elem, attr, required);
-
+  if (!maybe_int.has_value()) {
-  // check if attribute exists
+    return false;
-  if (!pstr) {
-    if (required) {
-      throw mjXError(elem, "required attribute missing: '%s'", attr);
-    } else {
-      return false;
-    }
   }
 
-  // convert to int, check
+  *data = maybe_int.value();
-  int buffer[2] = {0, 0};
-  if (sscanf(pstr, "%d", buffer) != 1) {
-    throw mjXError(elem, "single int expected in attribute %s", attr);
-  }
-
-  // copy data
-  *data = buffer[0];
   return true;
 }
 
@@ -796,15 +857,15 @@ XMLElement* mjXUtil::FindSubElem(XMLElement* elem, string name, bool required) {
 bool mjXUtil::MapValue(XMLElement* elem, const char* attr, int* data,
                        const mjMap* map, int mapSz, bool required) {
   // get attribute text
-  string text;
+  auto maybe_text = ReadAttrStr(elem, attr, required);
-  if (!ReadAttrTxt(elem, attr, text, required)) {
+  if (!maybe_text.has_value()) {
     return false;
   }
 
   // find keyword in map
-  int value = FindKey(map, mapSz, text);
+  int value = FindKey(map, mapSz, maybe_text.value());
-  if (value<0) {
+  if (value < 0) {
-    throw mjXError(elem, "invalid keyword: '%s'", text.c_str());
+    throw mjXError(elem, "invalid keyword: '%s'", maybe_text->c_str());
   }
 
   // copy

src/xml/xml_util.h

@@ -16,7 +16,9 @@
 #ifndef MUJOCO_SRC_XML_XML_UTIL_H_
 #define MUJOCO_SRC_XML_XML_UTIL_H_
 
-// stl
+#include <array>
+#include <functional>
+#include <optional>
 #include <string>
 #include <vector>
 #include <sstream>
@@ -98,21 +100,60 @@ class mjXUtil {
   // find value in map, return key ("": not found)
   static std::string FindValue(const mjMap* map, int mapsz, int value);
 
-  // read any type from attribute, return number read
+  // if attribute is present, return vector of numerical data
+  template<typename T>
+  static std::optional<std::vector<T>> ReadAttrVec(tinyxml2::XMLElement* elem, const char* attr,
+                                                   bool required = false);
+
+  // if attribute is present, return attribute as a string
+  static std::optional<std::string> ReadAttrStr(tinyxml2::XMLElement* elem, const char* attr,
+                                                bool required = false);
+
+  // if attribute is present, return numerical value of attribute
+  template<typename T>
+  static std::optional<T> ReadAttrNum(tinyxml2::XMLElement* elem, const char* attr,
+                                      bool required = false);
+
+  // if attribute is present, return array of numerical data
+  // N should be small as data is allocated on the stack
+  template<typename T, int N>
+  static std::optional<std::array<T, N>> ReadAttrArr(tinyxml2::XMLElement* elem, const char* attr,
+                                                     bool required = false) {
+    std::array<T, N> arr;
+    int n = 0;
+    if (!ReadAttrValues<T>(elem, attr, [&](int i, T num) { arr[i] = num; n++; }, N)) {
+      throw mjXError(elem, "attribute '%s' has too much data", attr);
+    }
+
+    if (!n) {
+      if (required) {
+        throw mjXError(elem, "required attribute missing: '%s'", attr);
+      } else {
+        return std::nullopt;
+      }
+    }
+
+    if (n < N) {
+      throw mjXError(elem, "attribute '%s' does not have enough data", attr);
+    }
+    return arr;
+  }
+
+  // deprecated: use ReadAttrVec or ReadAttrArr
   template<typename T>
   static int ReadAttr(tinyxml2::XMLElement* elem, const char* attr, const int len,
                       T* data, std::string& text,
                       bool required = false, bool exact = true);
 
-  // read DOUBLE array into C++ vector, return number read
+  // deprecated: use ReadAttrVec
   static int ReadVector(tinyxml2::XMLElement* elem, const char* attr,
                         std::vector<double>& vec, std::string& text, bool required = false);
 
-  // read text attribute
+  // deprecated: use ReadAttrStr
   static bool ReadAttrTxt(tinyxml2::XMLElement* elem, const char* attr, std::string& text,
                           bool required = false);
 
-  // read int attribute
+  // deprecated: use ReadAttrNum
   static bool ReadAttrInt(tinyxml2::XMLElement* elem, const char* attr, int* data,
                           bool required = false);
 
@@ -155,6 +196,11 @@ class mjXUtil {
   // write attribute- keyword
   static void WriteAttrKey(tinyxml2::XMLElement* elem, std::string name,
                            const mjMap* map, int mapsz, int data, int def = -12345);
+
+ private:
+  template<typename T>
+  static bool ReadAttrValues(tinyxml2::XMLElement* elem, const char* attr,
+                             std::function<void (int, T)> push, int max = -1);
 };
 
 #endif  // MUJOCO_SRC_XML_XML_UTIL_H_

test/xml/xml_native_reader_test.cc

@@ -317,7 +317,7 @@ TEST_F(XMLReaderTest, InvalidArrayLength) {
   <mujoco>
     <worldbody>
       <body>
-        <geom size="1" axisangle="1 0 0 0 asd"/>
+        <geom size="1" axisangle="1 0 0 0 1"/>
       </body>
     </worldbody>
   </mujoco>