Simplify val_to_str and have it and val_to_log_level use PyObject

src/python/pants/engine/internals/native.py

@@ -55,10 +55,6 @@ def create_exception(self, msg):
         """Given a utf8 message string, create an Exception object."""
         return Exception(msg)
 
-    def val_to_str(self, val):
-        """Given a `obj`, return str(obj)."""
-        return "" if val is None else str(val)
-
     def generator_send(
         self, func, arg
     ) -> Union[PyGeneratorResponseGet, PyGeneratorResponseGetMulti, PyGeneratorResponseBreak]:

src/rust/engine/src/core.rs

@@ -273,13 +273,13 @@ impl AsRef<PyObject> for Value {
 
 impl fmt::Debug for Value {
   fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-    write!(f, "{}", externs::val_to_str(&self))
+    write!(f, "{}", externs::val_to_str(&self.as_ref()))
   }
 }
 
 impl fmt::Display for Value {
   fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-    write!(f, "{}", externs::val_to_str(&self))
+    write!(f, "{}", externs::val_to_str(&self.as_ref()))
   }
 }
 
@@ -377,7 +377,7 @@ impl Failure {
       .extract::<String>(py)
       .unwrap()
     } else {
-      Self::native_traceback(&externs::val_to_str(&val))
+      Self::native_traceback(&externs::val_to_str(val.as_ref()))
     };
     Failure::Throw {
       val,
@@ -398,7 +398,7 @@ impl fmt::Display for Failure {
   fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
     match self {
       Failure::Invalidated => write!(f, "Giving up on retrying due to changed files."),
-      Failure::Throw { val, .. } => write!(f, "{}", externs::val_to_str(val)),
+      Failure::Throw { val, .. } => write!(f, "{}", externs::val_to_str(val.as_ref())),
     }
   }
 }

src/rust/engine/src/externs/engine_aware.rs

@@ -27,7 +27,7 @@ impl EngineAwareInformation for EngineAwareLevel {
   fn retrieve(_types: &Types, value: &Value) -> Option<Level> {
     let new_level_val = externs::call_method(value.as_ref(), "level", &[]).ok()?;
     let new_level_val = externs::check_for_python_none(new_level_val)?;
-    externs::val_to_log_level(&new_level_val.into()).ok()
+    externs::val_to_log_level(&new_level_val).ok()
   }
 }
 
@@ -39,7 +39,7 @@ impl EngineAwareInformation for Message {
   fn retrieve(_types: &Types, value: &Value) -> Option<String> {
     let msg_val = externs::call_method(&value, "message", &[]).ok()?;
     let msg_val = externs::check_for_python_none(msg_val)?;
-    Some(externs::val_to_str(&msg_val.into()))
+    Some(externs::val_to_str(&msg_val))
   }
 }
 
@@ -102,6 +102,6 @@ impl EngineAwareInformation for DebugHint {
     externs::call_method(&value, "debug_hint", &[])
       .ok()
       .and_then(externs::check_for_python_none)
-      .map(|val| externs::val_to_str(&val.into()))
+      .map(|val| externs::val_to_str(&val))
   }
 }

src/rust/engine/src/externs/interface.rs

@@ -694,10 +694,9 @@ fn nailgun_server_create(
         ];
         match externs::call_function(&runner, &runner_args) {
           Ok(exit_code) => {
-            let exit_code_val: Value = exit_code.into();
             // TODO: We don't currently expose a "project_i32", but it will not be necessary with
             // https://github.com/pantsbuild/pants/pull/9593.
-            nailgun::ExitCode(externs::val_to_str(&exit_code_val).parse().unwrap())
+            nailgun::ExitCode(externs::val_to_str(&exit_code).parse().unwrap())
           }
           Err(e) => {
             error!("Uncaught exception in nailgun handler: {:#?}", e);

src/rust/engine/src/externs/mod.rs

@@ -25,7 +25,7 @@ use crate::interning::Interns;
 
 use cpython::{
   py_class, CompareOp, FromPyObject, ObjectProtocol, PyBool, PyBytes, PyClone, PyDict, PyErr,
-  PyObject, PyResult as CPyResult, PyString, PyTuple, PyType, Python, PythonObject, ToPyObject,
+  PyObject, PyResult as CPyResult, PyTuple, PyType, Python, PythonObject, ToPyObject,
 };
 use lazy_static::lazy_static;
 use parking_lot::RwLock;
@@ -271,10 +271,6 @@ pub fn project_u64(value: &Value, field: &str) -> u64 {
   getattr(value.as_ref(), field).unwrap()
 }
 
-pub fn project_maybe_u64(value: &Value, field: &str) -> Result<u64, String> {
-  getattr(value.as_ref(), field)
-}
-
 pub fn project_f64(value: &Value, field: &str) -> f64 {
   getattr(value.as_ref(), field).unwrap()
 }
@@ -286,31 +282,30 @@ pub fn project_bytes(value: &Value, field: &str) -> Vec<u8> {
 }
 
 pub fn key_to_str(key: &Key) -> String {
-  val_to_str(&val_for(key))
+  val_to_str(&val_for(key).as_ref())
 }
 
 pub fn type_to_str(type_id: TypeId) -> String {
   project_str(&type_for_type_id(type_id).into_object().into(), "__name__")
 }
 
-pub fn val_to_str(val: &Value) -> String {
-  // TODO: to_string(py) returns a Cow<str>, so we could avoid actually cloning in some cases.
-  with_externs(|py, e| {
-    e.call_method(py, "val_to_str", (val as &PyObject,), None)
-      .unwrap()
-      .cast_as::<PyString>(py)
-      .unwrap()
-      .to_string(py)
-      .map(|cow| cow.into_owned())
-      .unwrap()
-  })
+pub fn val_to_str(obj: &PyObject) -> String {
+  let gil = Python::acquire_gil();
+  let py = gil.python();
+
+  if *obj == py.None() {
+    return "".to_string();
+  }
+
+  let pystring = obj.str(py).unwrap();
+  pystring.to_string(py).unwrap().into_owned()
 }
 
-pub fn val_to_log_level(val: &Value) -> Result<log::Level, String> {
-  let res: Result<PythonLogLevel, String> = project_maybe_u64(&val, "_level").and_then(|n: u64| {
+pub fn val_to_log_level(obj: &PyObject) -> Result<log::Level, String> {
+  let res: Result<PythonLogLevel, String> = getattr(obj, "_level").and_then(|n: u64| {
     n.try_into()
       .map_err(|e: num_enum::TryFromPrimitiveError<_>| {
-        format!("Could not parse {:?} as a LogLevel: {}", val, e)
+        format!("Could not parse {:?} as a LogLevel: {}", val_to_str(obj), e)
       })
   });
   res.map(|py_level| py_level.into())

src/rust/engine/src/nodes.rs

@@ -283,7 +283,8 @@ impl MultiPlatformExecuteProcess {
     };
 
     let description = externs::project_str(&value, "description");
-    let level = externs::val_to_log_level(&externs::project_ignoring_type(&value, "level"))?;
+    let level =
+      externs::val_to_log_level(&externs::project_ignoring_type(&value, "level").as_ref())?;
 
     let append_only_caches = externs::project_frozendict(&value, "append_only_caches")
       .into_iter()
@@ -916,7 +917,7 @@ impl Task {
                     "non_member_error_message",
                     &[externs::val_for(&get.input)],
                   ) {
-                    Ok(err_msg) => throw(&externs::val_to_str(&err_msg.into())),
+                    Ok(err_msg) => throw(&externs::val_to_str(&err_msg)),
                     // If the non_member_error_message() call failed for any reason,
                     // fall back to a generic message.
                     Err(_e) => throw(&format!(