From 3d0280ec03f927ae4925aae400600a7b96058587 Mon Sep 17 00:00:00 2001
From: Vincent Ambo <mail@tazj.in>
Date: Wed, 24 Aug 2022 16:58:55 +0300
Subject: refactor(tvix/eval): implement clearer mechanism for globals

The set of things that can leak out of `builtins` into the global
scope is statically known (it is what Nix 2.3 leaks there,
essentially).

This is a mild change over the previous mechanism, where instead at
the point where the `builtins` set is constructed we "lift" the
globals out of there (if they exist).

This way users will still eventually be able to add additional
builtins, HOWEVER they will not be able to leak them into the global
scope.

Note that upstream Nix technically leaks _all_ builtins into the
global scope using the `__*` prefix, but we are trying to avoid this
in Tvix if it is not required in nixpkgs.

Change-Id: Ie9dec2ce33740134f3b2464eba3749f421dd5953
Reviewed-on: https://cl.tvl.fyi/c/depot/+/6258
Tested-by: BuildkiteCI
Reviewed-by: sterni <sternenseemann@systemli.org>
---
 tvix/eval/src/builtins/mod.rs | 67 +++++++++++++++++++++++++++++++++----------
 tvix/eval/src/value/attrs.rs  |  8 ++++++
 2 files changed, 60 insertions(+), 15 deletions(-)

diff --git a/tvix/eval/src/builtins/mod.rs b/tvix/eval/src/builtins/mod.rs
index b5e8bb289b0e..39e29580b4c3 100644
--- a/tvix/eval/src/builtins/mod.rs
+++ b/tvix/eval/src/builtins/mod.rs
@@ -3,29 +3,66 @@
 //! See //tvix/eval/docs/builtins.md for a some context on the
 //! available builtins in Nix.
 
-use std::collections::HashMap;
+use std::{
+    collections::{BTreeMap, HashMap},
+    rc::Rc,
+};
 
-use crate::value::{Builtin, Value};
+use crate::value::{Builtin, NixAttrs, NixString, Value};
 
-macro_rules! builtin {
-    ( $map:ident, $name:literal, $arity:literal, $body:expr ) => {
-        $map.insert($name, Value::Builtin(Builtin::new($name, $arity, $body)));
-    };
+fn pure_builtins() -> Vec<Builtin> {
+    vec![
+        Builtin::new("isNull", 1, |args| {
+            Ok(Value::Bool(matches!(args[0], Value::Null)))
+        }),
+        Builtin::new("toString", 1, |args| {
+            // TODO: toString is actually not the same as Display
+            Ok(Value::String(format!("{}", args[0]).into()))
+        }),
+    ]
+}
+
+fn builtins_set() -> NixAttrs {
+    let mut map: BTreeMap<NixString, Value> = BTreeMap::new();
+
+    for builtin in pure_builtins() {
+        map.insert(builtin.name().into(), Value::Builtin(builtin));
+    }
+
+    NixAttrs::from_map(map)
 }
 
 /// Set of Nix builtins that are globally available.
 pub fn global_builtins() -> HashMap<&'static str, Value> {
-    let mut globals = HashMap::new();
+    let builtins = builtins_set();
+    let mut globals: HashMap<&'static str, Value> = HashMap::new();
 
-    builtin!(globals, "isNull", 1, |args| Ok(Value::Bool(matches!(
-        args[0],
-        Value::Null
-    ))));
+    // known global builtins from the builtins set.
+    for global in &[
+        "abort",
+        "baseNameOf",
+        "derivation",
+        "derivationStrict",
+        "dirOf",
+        "fetchGit",
+        "fetchMercurial",
+        "fetchTarball",
+        "fromTOML",
+        "import",
+        "isNull",
+        "map",
+        "placeholder",
+        "removeAttrs",
+        "scopedImport",
+        "throw",
+        "toString",
+    ] {
+        if let Some(builtin) = builtins.select(global) {
+            globals.insert(global, builtin.clone());
+        }
+    }
 
-    builtin!(globals, "toString", 1, |args| {
-        // TODO: toString is actually not the same as Display
-        Ok(Value::String(format!("{}", args[0]).into()))
-    });
+    globals.insert("builtins", Value::Attrs(Rc::new(builtins)));
 
     globals
 }
diff --git a/tvix/eval/src/value/attrs.rs b/tvix/eval/src/value/attrs.rs
index 319f6bdfa9bb..4109691992ca 100644
--- a/tvix/eval/src/value/attrs.rs
+++ b/tvix/eval/src/value/attrs.rs
@@ -259,6 +259,14 @@ impl NixAttrs {
 
         Ok(attrs)
     }
+
+    /// Construct an attribute set directly from a BTreeMap
+    /// representation. This is only visible inside of the crate, as
+    /// it is intended exclusively for use with the construction of
+    /// global sets for the compiler.
+    pub(crate) fn from_map(map: BTreeMap<NixString, Value>) -> Self {
+        NixAttrs(AttrsRep::Map(map))
+    }
 }
 
 // In Nix, name/value attribute pairs are frequently constructed from
-- 
cgit 1.4.1