Refactor compiler warning handling.

Compiler warnings are now accumulated in a vector of strings within the mjSpec object. New API functions `mjs_numWarnings` and `mjs_getWarning` are added to access these warnings. The compiler's log handler now chains warnings to the global log handler, ensuring they are still displayed immediately. Call sites in `mj_loadXML`, `mj_compile`, and the Python and WASM bindings have been updated to use the new warning API.

PiperOrigin-RevId: 933361650
Change-Id: I47cab98a460c57b0898c0a1a43fce2a5b9648eb1

src/user/user_api.cc

@@ -483,15 +483,37 @@ const double* mjs_getTimer(mjSpec* s) {
   return modelC->timer;
 }
 
-
-
-// check if model has warnings
+// check if model has warnings (but no error)
+// TODO(tassa): delete this function
 int mjs_isWarning(mjSpec* s) {
+  if (!s) {
+    return 0;
+  }
   mjCModel* modelC = static_cast<mjCModel*>(s->element);
-  return modelC->GetError().warning;
+  return modelC->GetError().message[0] == '\0' &&
+         !modelC->GetWarnings().empty();
 }
 
+// get number of warnings
+int mjs_numWarnings(const mjSpec* spec) {
+  if (!spec) {
+    return 0;
+  }
+  const mjCModel* modelC = static_cast<const mjCModel*>(spec->element);
+  return static_cast<int>(modelC->GetWarnings().size());
+}
 
+// get the i-th warning message
+const char* mjs_getWarning(const mjSpec* spec, int index) {
+  if (!spec) {
+    return nullptr;
+  }
+  const mjCModel* modelC = static_cast<const mjCModel*>(spec->element);
+  if (index < 0 || index >= static_cast<int>(modelC->GetWarnings().size())) {
+    return nullptr;
+  }
+  return modelC->GetWarnings()[index].c_str();
+}
 
 // delete model
 void mj_deleteSpec(mjSpec* s) {

src/user/user_mesh.cc

@@ -1546,7 +1546,7 @@ void mjCMesh::Process() {
     for (int i = 0; i < nface(); i++) {
       SetBoundingVolume(i, dvert.data());
     }
-    tree_.CreateBVH();
+    tree_.CreateBVH(model, this);
   }
   mesh_timer_[mjCTIMER_MESH_BVH] += Seconds(Clock::now() - t0).count();
 
@@ -5451,7 +5451,7 @@ void mjCFlex::CreateBVH() {
 
   // create hierarchy
   tree.RemoveInactiveVolumes(nbvh);
-  tree.CreateBVH();
+  tree.CreateBVH(model, this);
 }
 
 

src/user/user_model.cc

@@ -1239,6 +1239,7 @@ void mjCModel::Clear() {
   hasImplicitPluginElem = false;
   compiled = false;
   errInfo = mjCError();
+  ClearCompileWarnings();
   qpos0.clear();
 }
 
@@ -1508,7 +1509,21 @@ const mjCError& mjCModel::GetError() const {
   return errInfo;
 }
 
+// add warning to vector (immediate delivery outside compile)
+void mjCModel::AddWarning(std::string msg, const mjCBase* obj) {
+  if (obj) {
+    msg += "\nElement name '" + obj->name + "', id " + std::to_string(obj->id);
+    if (!obj->info.empty()) {
+      msg += ", " + obj->info;
+    }
+  }
 
+  // outside compile: deliver immediately via normal handler chain
+  if (!compiling_) {
+    mju_warning("%s", msg.c_str());
+  }
+  warnings_.push_back(std::move(msg));
+}
 
 // pointer to world body
 mjCBody* mjCModel::GetWorld() {
@@ -3551,8 +3566,10 @@ void mjCModel::CopyObjects(mjModel* m) {
     if (!pfl->rigid && m->flex_edgeequality[i] == 0 &&
         !pfl->edgestiffness && !pfl->edgedamping && !pfl->damping &&
         pfl->bending.empty()) {
-      mju_warning("flex '%s' is not rigid and has no equality constraints "
-                  "or passive forces", pfl->name.c_str());
+      AddWarning("flex '" + pfl->name +
+                     "' is not rigid and has no equality constraints or "
+                     "passive forces",
+                 pfl);
     }
 
     // copy bvh data (flex aabb computed dynamically in mjData)
@@ -4212,9 +4229,10 @@ template void mjCModel::RestoreState<mjtNum>(
 // resolve keyframe references
 void mjCModel::StoreKeyframes(mjCModel* dest) {
   if (this != dest && !key_pending_.empty()) {
-    mju_warning(
-      "Child model has pending keyframes. They will not be namespaced correctly. "
-      "To prevent this, compile the child model before attaching it again.");
+    dest->AddWarning(
+        "Child model has pending keyframes. They will not be namespaced "
+        "correctly. "
+        "To prevent this, compile the child model before attaching it again.");
   }
 
   // do not change compilation quantities in case the user wants to recompile preserving the state
@@ -4633,10 +4651,17 @@ static void compilerLogHandler(const mjLogMessage* msg) {
     mju::strcpy_arr(errortext, msg->subject);
     std::longjmp(error_jmp_buf, 1);
   } else if (msg->level == mjLOG_WARNING) {
+    // buffer for structured capture (append, not overwrite)
     if (local_warningtext_ptr) {
-      *local_warningtext_ptr = msg->subject;
+      if (!local_warningtext_ptr->empty()) {
+        *local_warningtext_ptr += '\n';
+      }
+      *local_warningtext_ptr += msg->subject;
     } else {
-      mju::strcpy_arr(warningtext, msg->subject);
+      if (warningtext[0]) {
+        mju::strcat_arr(warningtext, "\n");
+      }
+      mju::strcat_arr(warningtext, msg->subject);
     }
   }
 }
@@ -4661,10 +4686,15 @@ mjModel* mjCModel::Compile(const mjVFS* vfs, mjModel** m) {
   mjModel* volatile model = (m && *m) ? *m : nullptr;
   mjData* volatile data = nullptr;
 
-  // save log handler
-  mjfLogHandler save_handler = _mjPRIVATE_setTlsLogHandler(compilerLogHandler);
+  // install compiler log handler (captures warnings silently)
+  mjfLogHandler prev_tls = _mjPRIVATE_setTlsLogHandler(compilerLogHandler);
 
   errInfo = mjCError();
+
+  // set flag so warnings are captured in the spec vector rather than delivered
+  // immediately
+  compiling_ = true;
+  ClearCompileWarnings();
   warningtext[0] = 0;
 
   try {
@@ -4677,7 +4707,7 @@ mjModel* mjCModel::Compile(const mjVFS* vfs, mjModel** m) {
       // also include the last warning that was issued. this is useful for
       // warnings that came out of plugin implementations.
       if (warningtext[0]) {
-        error_msg += "\n";
+        error_msg += '\n';
         error_msg += warningtext;
       }
       throw mjCError(0, "engine error: %s", error_msg.c_str());
@@ -4701,13 +4731,21 @@ mjModel* mjCModel::Compile(const mjVFS* vfs, mjModel** m) {
     }
 
     // restore handler, return 0
-    _mjPRIVATE_setTlsLogHandler(save_handler);
+    _mjPRIVATE_setTlsLogHandler(prev_tls);
+    compiling_ = false;
     return nullptr;
   }
 
-  // restore log handler, mark as compiled, return mjModel
-  _mjPRIVATE_setTlsLogHandler(save_handler);
+  // restore log handler
+  _mjPRIVATE_setTlsLogHandler(prev_tls);
+  compiling_ = false;
   compiled = true;
+
+  // play back compile warnings through the normal handler chain
+  for (int i = num_attach_warnings_; i < warnings_.size(); ++i) {
+    mju_warning("%s", warnings_[i].c_str());
+  }
+
   return model;
 }
 
@@ -5353,7 +5391,22 @@ void mjCModel::TryCompile(mjModel*& m, mjData*& d, const mjVFS* vfs) {
     throw mjCError(0, "could not create mjData");
   }
 
+  // pass compiler warnings into structured warning vector before validation
+  if (warningtext[0]) {
+    std::string warnings(warningtext);
+    std::istringstream stream(warnings);
+    std::string line;
+    while (std::getline(stream, line)) {
+      if (!line.empty()) {
+        AddWarning(line);
+      }
+    }
+  }
+
   // test forward simulation unless asleep_init is true (potentially expensive)
+  // reset warningtext: engine warnings from validation are not compiler
+  // warnings
+  warningtext[0] = 0;
   if (!asleep_init) {
     mj_step(m, d);
   }
@@ -5364,11 +5417,6 @@ void mjCModel::TryCompile(mjModel*& m, mjData*& d, const mjVFS* vfs) {
   m->opt.enableflags = enableflags;
   d = nullptr;
 
-  // pass warning back
-  if (warningtext[0]) {
-    mju::strcpy_arr(errInfo.message, warningtext);
-    errInfo.warning = true;
-  }
 
   // save signature
   m->signature = Signature();

src/user/user_model.h

@@ -249,6 +249,23 @@ class mjCModel : public mjCModel_, private mjSpec {
   bool IsCompiled() const;                                          // is model already compiled
   const mjCError& GetError() const;                                 // get reference of error object
   void SetError(const mjCError& error) { errInfo = error; }         // set value of error object
+  void AddWarning(std::string msg,  // add warning to vector
+                  const mjCBase* obj = nullptr);
+  const std::vector<std::string>& GetWarnings()
+      const {  // get accumulated warnings
+    return warnings_;
+  }
+  void ClearWarnings() {
+    warnings_.clear();
+    num_attach_warnings_ = 0;
+  }  // clear all warnings
+  void ClearCompileWarnings() {
+    warnings_.resize(num_attach_warnings_);
+  }                                  // clear compile warnings
+  void SetAttachWarningBoundary() {  // snapshot attach warning count
+    num_attach_warnings_ = warnings_.size();
+  }
+
   mjCBody* GetWorld();                                              // pointer to world body
   mjCDef* FindDefault(const std::string& name) const;               // find defaults class name
   mjCDef* AddDefault(std::string name, mjCDef* parent = nullptr);   // add defaults class to array
@@ -488,10 +505,15 @@ class mjCModel : public mjCModel_, private mjSpec {
   // expand all keyframes in the model
   void ExpandAllKeyframes();
 
-  mjListKeyMap ids;   // map from object names to ids
-  mjCError errInfo;   // last error info
+  mjListKeyMap ids;  // map from object names to ids
+  mjCError errInfo;  // last error info
+  std::vector<std::string>
+      warnings_;  // chronological list of non-fatal warnings
+  int num_attach_warnings_ =
+      0;  // boundary: [0, n) are attach, [n, size) are compile
+  bool compiling_ = false;              // true during Compile()
   std::vector<mjKeyInfo> key_pending_;  // attached keyframes
-  bool deepcopy_;     // copy objects when attaching
+  bool deepcopy_;                       // copy objects when attaching
   bool attached_ = false;  // true if model is attached to a parent model
   std::unordered_map<const mjsCompiler*, mjSpec*> compiler2spec_;  // map from compiler to spec
   std::vector<mjCBase*> detached_;  // list of detached objects

src/user/user_objects.cc

@@ -38,12 +38,11 @@
 #include <utility>
 #include <vector>
 
-#include "lodepng.h"
-#include "cc/array_safety.h"
-#include "engine/engine_passive.h"
-#include "engine/engine_support.h"
+#include "lodepng.h"  // NOLINT
 #include <mujoco/mjspec.h>
 #include <mujoco/mujoco.h>
+#include "cc/array_safety.h"
+#include "engine/engine_passive.h"
 #include "user/user_api.h"
 #include "user/user_cache.h"
 #include "user/user_model.h"
@@ -197,7 +196,6 @@ mjCError::mjCError(const mjCBase* obj, const char* msg, const char* str, int pos
   char temp[600];
 
   // init
-  warning = false;
   if (obj || msg) {
     mju::sprintf_arr(message, "Error");
   } else {
@@ -396,13 +394,13 @@ mjCBoundingVolumeHierarchy::AddBoundingVolume(const int* id, int contype, int co
 
 
 // create bounding volume hierarchy
-void mjCBoundingVolumeHierarchy::CreateBVH() {
+void mjCBoundingVolumeHierarchy::CreateBVH(mjCModel* model,
+                                           const mjCBase* owner) {
   std::vector<BVElement> elements;
   Make(elements);
-  MakeBVH(elements.begin(), elements.end());
+  MakeBVH(elements.begin(), elements.end(), 0, model, owner);
 }
 
-
 void mjCBoundingVolumeHierarchy::Make(std::vector<BVElement>& elements) {
   // precompute the positions of each element in the hierarchy's axes, and drop
   // visual-only elements.
@@ -424,8 +422,9 @@ void mjCBoundingVolumeHierarchy::Make(std::vector<BVElement>& elements) {
 
 // compute bounding volume hierarchy
 int mjCBoundingVolumeHierarchy::MakeBVH(
-  std::vector<BVElement>::iterator elements_begin,
-  std::vector<BVElement>::iterator elements_end, int lev) {
+    std::vector<BVElement>::iterator elements_begin,
+    std::vector<BVElement>::iterator elements_end, int lev, mjCModel* model,
+    const mjCBase* owner) {
   int nelements = elements_end - elements_begin;
   if (nelements == 0) {
     return -1;
@@ -525,11 +524,13 @@ int mjCBoundingVolumeHierarchy::MakeBVH(
 
   // recursive calls
   if (m > 0) {
-    child_[2*index + 0] = MakeBVH(elements_begin, elements_begin + m, lev + 1);
+    child_[2 * index + 0] =
+        MakeBVH(elements_begin, elements_begin + m, lev + 1, model, owner);
   }
 
   if (m != nelements) {
-    child_[2*index + 1] = MakeBVH(elements_begin + m, elements_end, lev + 1);
+    child_[2 * index + 1] =
+        MakeBVH(elements_begin + m, elements_end, lev + 1, model, owner);
   }
 
   // SHOULD NOT OCCUR
@@ -539,14 +540,12 @@ int mjCBoundingVolumeHierarchy::MakeBVH(
   }
 
   if (lev > mjMAXTREEDEPTH) {
-    mju_warning("max tree depth exceeded in body=%s", name_.c_str());
+    model->AddWarning("max tree depth exceeded", owner);
   }
 
   return index;
 }
 
-
-
 //------------------------- class mjCOctree implementation --------------------------------------------
 
 void mjCOctree::CopyLevel(int* level) const {
@@ -2631,7 +2630,7 @@ void mjCBody::ComputeBVH() {
     tree.AddBoundingVolume(&geom->id, geom->contype, geom->conaffinity,
                            geom->pos, geom->quat, geom->aabb);
   }
-  tree.CreateBVH();
+  tree.CreateBVH(model, this);
 }
 
 
@@ -3871,7 +3870,7 @@ void mjCGeom::SetFluidCoefs(void) {
 
 // compute bounding box
 void mjCGeom::ComputeAABB(void) {
-  double aamm[6]; // axis-aligned bounding box in (min, max) format
+  double aamm[6];  // axis-aligned bounding box in (min, max) format
   switch (type) {
     case mjGEOM_HFIELD:
       aamm[0] = -hfield->size[0];

src/user/user_objects.h

@@ -83,7 +83,6 @@ class [[nodiscard]] mjCError {
            int pos2 = 0);
 
   char message[500];              // error message
-  bool warning;                   // is this a warning instead of error
 };
 
 // alternative specifications of frame orientation
@@ -172,7 +171,7 @@ struct mjCBoundingVolumeHierarchy_ {
 class mjCBoundingVolumeHierarchy : public mjCBoundingVolumeHierarchy_ {
  public:
   // make bounding volume hierarchy
-  void CreateBVH();
+  void CreateBVH(mjCModel* model, const mjCBase* owner);
   void Set(double ipos_element[3], double iquat_element[4]);
   void AllocateBoundingVolumes(int nleaf);
   void RemoveInactiveVolumes(int nmax);
@@ -210,7 +209,8 @@ class mjCBoundingVolumeHierarchy : public mjCBoundingVolumeHierarchy_ {
   };
   void Make(std::vector<BVElement>& elements);
   int MakeBVH(std::vector<BVElement>::iterator elements_begin,
-              std::vector<BVElement>::iterator elements_end, int lev = 0);
+              std::vector<BVElement>::iterator elements_end, int lev,
+              mjCModel* model, const mjCBase* owner);
 };
 
 

src/xml/xml_api.cc

@@ -58,8 +58,16 @@ mjModel* mj_loadXML(const char* filename, const mjVFS* vfs,
   }
 
   // handle compile warning
-  if (mjs_isWarning(spec.get())) {
-    mjCopyError(error, mjs_getError(spec.get()), error_sz);
+  int num_warnings = mjs_numWarnings(spec.get());
+  if (num_warnings > 0) {
+    std::string all_warnings;
+    for (int i = 0; i < num_warnings; ++i) {
+      if (!all_warnings.empty()) {
+        all_warnings += '\n';
+      }
+      all_warnings += mjs_getWarning(spec.get(), i);
+    }
+    mjCopyError(error, all_warnings.c_str(), error_sz);
   } else if (error) {
     error[0] = '\0';
   }