Stop differentiating between constructors and types in FFI

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

@@ -127,19 +127,13 @@ def __init__(
         # Create the native Scheduler and Session.
         tasks = self._register_rules(rule_index)
 
-        # TODO: There is no longer a need to differentiate constructors from types, as types are
-        # callable as well with the cpython crate.
         types = PyTypes(
-            construct_directory_digest=Digest,
             directory_digest=Digest,
-            construct_snapshot=Snapshot,
             snapshot=Snapshot,
-            construct_file_content=FileContent,
-            construct_digest_contents=DigestContents,
+            file_content=FileContent,
             digest_contents=DigestContents,
-            construct_process_result=FallibleProcessResultWithPlatform,
-            construct_materialize_directories_results=MaterializeDirectoriesResult,
-            construct_materialize_directory_result=MaterializeDirectoryResult,
+            materialize_directories_results=MaterializeDirectoriesResult,
+            materialize_directory_result=MaterializeDirectoryResult,
             address=Address,
             path_globs=PathGlobs,
             merge_digests=MergeDigests,
@@ -156,11 +150,9 @@ def __init__(
             url_to_fetch=UrlToFetch,
             string=str,
             bytes=bytes,
-            construct_interactive_process_result=InteractiveProcessResult,
             interactive_process=InteractiveProcess,
             interactive_process_result=InteractiveProcessResult,
             snapshot_subset=SnapshotSubset,
-            construct_platform=Platform,
         )
 
         self._scheduler = native.new_scheduler(

src/rust/engine/src/core.rs

@@ -17,8 +17,7 @@ pub type FNV = hash::BuildHasherDefault<FnvHasher>;
 ///
 /// Params represent a TypeId->Key map.
 ///
-/// For efficiency and hashability, they're stored as sorted Keys (with distinct TypeIds), and
-/// wrapped in an `Arc` that allows us to copy-on-write for param contents.
+/// For efficiency and hashability, they're stored as sorted Keys (with distinct TypeIds).
 ///
 #[repr(C)]
 #[derive(Clone, Debug, Default, Eq, Hash, PartialEq)]

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

@@ -395,16 +395,12 @@ py_class!(class PyTypes |py| {
 
   def __new__(
       _cls,
-      construct_directory_digest: PyObject,
       directory_digest: PyType,
-      construct_snapshot: PyObject,
       snapshot: PyType,
-      construct_file_content: PyObject,
-      construct_digest_contents: PyObject,
+      file_content: PyType,
       digest_contents: PyType,
-      construct_process_result: PyObject,
-      construct_materialize_directories_results: PyObject,
-      construct_materialize_directory_result: PyObject,
+      materialize_directories_results: PyType,
+      materialize_directory_result: PyType,
       address: PyType,
       path_globs: PyType,
       merge_digests: PyType,
@@ -421,25 +417,19 @@ py_class!(class PyTypes |py| {
       url_to_fetch: PyType,
       string: PyType,
       bytes: PyType,
-      construct_interactive_process_result: PyObject,
       interactive_process: PyType,
       interactive_process_result: PyType,
-      snapshot_subset: PyType,
-      construct_platform: PyObject
+      snapshot_subset: PyType
   ) -> CPyResult<Self> {
     Self::create_instance(
         py,
         RefCell::new(Some(Types {
-        construct_directory_digest: Function(externs::key_for(construct_directory_digest.into())?),
         directory_digest: externs::type_for(directory_digest),
-        construct_snapshot: Function(externs::key_for(construct_snapshot.into())?),
         snapshot: externs::type_for(snapshot),
-        construct_file_content: Function(externs::key_for(construct_file_content.into())?),
-        construct_digest_contents: Function(externs::key_for(construct_digest_contents.into())?),
+        file_content: externs::type_for(file_content),
         digest_contents: externs::type_for(digest_contents),
-        construct_process_result: Function(externs::key_for(construct_process_result.into())?),
-        construct_materialize_directories_results: Function(externs::key_for(construct_materialize_directories_results.into())?),
-        construct_materialize_directory_result: Function(externs::key_for(construct_materialize_directory_result.into())?),
+        materialize_directories_results: externs::type_for(materialize_directories_results),
+        materialize_directory_result: externs::type_for(materialize_directory_result),
         address: externs::type_for(address),
         path_globs: externs::type_for(path_globs),
         merge_digests: externs::type_for(merge_digests),
@@ -456,11 +446,9 @@ py_class!(class PyTypes |py| {
         url_to_fetch: externs::type_for(url_to_fetch),
         string: externs::type_for(string),
         bytes: externs::type_for(bytes),
-        construct_interactive_process_result: Function(externs::key_for(construct_interactive_process_result.into())?),
         interactive_process: externs::type_for(interactive_process),
         interactive_process_result: externs::type_for(interactive_process_result),
         snapshot_subset: externs::type_for(snapshot_subset),
-        construct_platform: Function(externs::key_for(construct_platform.into())?),
     })),
     )
   }
@@ -1497,7 +1485,7 @@ fn run_local_interactive_process(
       block_in_place_and_wait(py, ||
         session.with_console_ui_disabled(|| {
           let types = &scheduler.core.types;
-          let construct_interactive_process_result = types.construct_interactive_process_result;
+          let interactive_process_result = types.interactive_process_result;
 
           let value: Value = request.into();
 
@@ -1561,7 +1549,7 @@ fn run_local_interactive_process(
           let code = exit_status.code().unwrap_or(-1);
 
           Ok(externs::unsafe_call(
-            &construct_interactive_process_result,
+            interactive_process_result,
             &[externs::store_i64(i64::from(code))],
           ).into())
         })
@@ -1597,9 +1585,8 @@ fn materialize_directories(
       // TODO: A parent_id should be an explicit argument.
       session.workunit_store().init_thread_state(None);
       let types = &scheduler.core.types;
-      let construct_materialize_directories_results =
-        types.construct_materialize_directories_results;
-      let construct_materialize_directory_result = types.construct_materialize_directory_result;
+      let materialize_directories_results = types.materialize_directories_results;
+      let materialize_directory_result = types.materialize_directory_result;
 
       block_in_place_and_wait(py, || {
         future::join_all(
@@ -1637,15 +1624,15 @@ fn materialize_directories(
                 .collect();
 
               externs::unsafe_call(
-                &construct_materialize_directory_result,
+                materialize_directory_result,
                 &[externs::store_tuple(path_values)],
               )
             },
           )
           .collect();
 
         externs::unsafe_call(
-          &construct_materialize_directories_results,
+          materialize_directories_results,
           &[externs::store_tuple(entries)],
         )
         .into()

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

@@ -16,7 +16,7 @@ use std::collections::BTreeMap;
 use std::fmt;
 use std::sync::atomic;
 
-use crate::core::{Failure, Function, Key, TypeId, Value};
+use crate::core::{Failure, Key, TypeId, Value};
 use crate::interning::Interns;
 
 use cpython::{
@@ -328,14 +328,25 @@ pub fn generator_send(generator: &Value, arg: &Value) -> Result<GeneratorRespons
 }
 
 ///
-/// NB: Panics on failure. Only recommended for use with built-in functions, such as
+/// NB: Panics on failure. Only recommended for use with built-in types, such as
 /// those configured in types::Types.
 ///
-pub fn unsafe_call(func: &Function, args: &[Value]) -> Value {
-  let func_val = with_interns(|interns| interns.key_get(&func.0).clone());
-  call(&func_val, args).unwrap_or_else(|e| {
-    panic!("Core function `{}` failed: {:?}", val_to_str(&func_val), e);
-  })
+pub fn unsafe_call(type_id: TypeId, args: &[Value]) -> Value {
+  let py_type = type_for_type_id(type_id);
+  let arg_handles: Vec<PyObject> = args.iter().map(|v| v.clone().into()).collect();
+  let gil = Python::acquire_gil();
+  let args_tuple = PyTuple::new(gil.python(), &arg_handles);
+  py_type
+    .call(gil.python(), args_tuple, None)
+    .map(Value::from)
+    .unwrap_or_else(|e| {
+      let gil = Python::acquire_gil();
+      panic!(
+        "Core type constructor `{}` failed: {:?}",
+        py_type.name(gil.python()),
+        e
+      );
+    })
 }
 
 lazy_static! {

src/rust/engine/src/intrinsics.rs

@@ -163,16 +163,13 @@ fn multi_platform_process_request_to_process_result(
 
     let platform_name: String = result.platform.into();
     Ok(externs::unsafe_call(
-      &core.types.construct_process_result,
+      core.types.process_result,
       &[
         externs::store_bytes(&stdout_bytes),
         externs::store_bytes(&stderr_bytes),
         externs::store_i64(result.exit_code.into()),
         Snapshot::store_directory(&core, &result.output_directory),
-        externs::unsafe_call(
-          &core.types.construct_platform,
-          &[externs::store_utf8(&platform_name)],
-        ),
+        externs::unsafe_call(core.types.platform, &[externs::store_utf8(&platform_name)]),
       ],
     ))
   }

src/rust/engine/src/nodes.rs

@@ -516,7 +516,7 @@ impl Snapshot {
 
   pub fn store_directory(core: &Arc<Core>, item: &hashing::Digest) -> Value {
     externs::unsafe_call(
-      &core.types.construct_directory_digest,
+      core.types.directory_digest,
       &[
         externs::store_utf8(&item.0.to_hex()),
         externs::store_i64(item.1 as i64),
@@ -538,7 +538,7 @@ impl Snapshot {
       }
     }
     Ok(externs::unsafe_call(
-      &core.types.construct_snapshot,
+      core.types.snapshot,
       &[
         Self::store_directory(core, &item.digest),
         externs::store_tuple(files),
@@ -557,7 +557,7 @@ impl Snapshot {
 
   fn store_file_content(context: &Context, item: &FileContent) -> Result<Value, String> {
     Ok(externs::unsafe_call(
-      &context.core.types.construct_file_content,
+      context.core.types.file_content,
       &[
         Self::store_path(&item.path)?,
         externs::store_bytes(&item.content),
@@ -572,7 +572,7 @@ impl Snapshot {
       .map(|e| Self::store_file_content(context, e))
       .collect::<Result<Vec<_>, _>>()?;
     Ok(externs::unsafe_call(
-      &context.core.types.construct_digest_contents,
+      context.core.types.digest_contents,
       &[externs::store_tuple(entries)],
     ))
   }

src/rust/engine/src/types.rs

@@ -1,19 +1,15 @@
 // Copyright 2020 Pants project contributors (see CONTRIBUTORS.md).
 // Licensed under the Apache License, Version 2.0 (see LICENSE).
 
-use crate::core::{Function, TypeId};
+use crate::core::TypeId;
 
 pub struct Types {
-  pub construct_directory_digest: Function,
   pub directory_digest: TypeId,
-  pub construct_snapshot: Function,
   pub snapshot: TypeId,
-  pub construct_file_content: Function,
-  pub construct_digest_contents: Function,
+  pub file_content: TypeId,
   pub digest_contents: TypeId,
-  pub construct_process_result: Function,
-  pub construct_materialize_directories_results: Function,
-  pub construct_materialize_directory_result: Function,
+  pub materialize_directories_results: TypeId,
+  pub materialize_directory_result: TypeId,
   pub address: TypeId,
   pub path_globs: TypeId,
   pub merge_digests: TypeId,
@@ -30,9 +26,7 @@ pub struct Types {
   pub url_to_fetch: TypeId,
   pub string: TypeId,
   pub bytes: TypeId,
-  pub construct_interactive_process_result: Function,
   pub interactive_process: TypeId,
   pub interactive_process_result: TypeId,
   pub snapshot_subset: TypeId,
-  pub construct_platform: Function,
 }