Replace set constructor with flag

It's unlikely that someone would needs alternative `set` implementation,
same way as it's unlikely that someone would need alternative `dict`
implementation.

So replace dynamic `set_constructor` function with simple `set_literals`
flag.

Note `depset` is not an alternative implementation of sets, e. g.
it is not iterable, while `set` should be, and `depset` is initialized
with `order` parameter. So set literal should not be be used to
construct depsets.

starlark/src/environment/mod.rs

@@ -109,22 +109,8 @@ struct EnvironmentContent {
     ///
     /// These bindings include methods for native types, e.g. `string.isalnum`.
     variables: HashMap<String, Value>,
-    /// Optional function which can be used to construct set literals (i.e. `{foo, bar}`).
-    /// If not set, attempts to use set literals will raise an error.
-    set_constructor: SetConstructor,
-}
-
-// Newtype so that EnvironmentContent can derive Debug.
-struct SetConstructor(Option<Box<dyn Fn(Vec<Value>) -> ValueResult>>);
-
-impl std::fmt::Debug for SetConstructor {
-    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> Result<(), std::fmt::Error> {
-        if self.0.is_some() {
-            write!(f, "<set constructor>")
-        } else {
-            write!(f, "<no set constructor>")
-        }
-    }
+    /// When `true`, set `{foo, bar}` literals are allowed.
+    set_literals: bool,
 }
 
 impl Environment {
@@ -135,7 +121,7 @@ impl Environment {
                 name_: name.into(),
                 parent: None,
                 variables: HashMap::new(),
-                set_constructor: SetConstructor(None),
+                set_literals: false,
             })),
         }
     }
@@ -148,7 +134,7 @@ impl Environment {
                 name_: name.into(),
                 parent: Some(self.clone()),
                 variables: HashMap::new(),
-                set_constructor: SetConstructor(None),
+                set_literals: self.env.borrow().set_literals,
             })),
         }
     }
@@ -209,21 +195,13 @@ impl Environment {
     ///
     /// The `Value` returned by this function is expected to be a one-dimensional collection
     /// containing no duplicates.
-    pub fn with_set_constructor(&self, constructor: Box<dyn Fn(Vec<Value>) -> ValueResult>) {
-        self.env.borrow_mut().set_constructor = SetConstructor(Some(constructor));
+    pub fn enable_set_literals(&self) {
+        self.env.borrow_mut().set_literals = true;
     }
 
-    pub(crate) fn make_set(&self, values: Vec<Value>) -> ValueResult {
-        match self.env.borrow().set_constructor.0 {
-            Some(ref ctor) => ctor(values),
-            None => {
-                if let Some(parent) = self.get_parent() {
-                    parent.make_set(values)
-                } else {
-                    Err(ValueError::TypeNotSupported("set".to_string()))
-                }
-            }
-        }
+    /// Is it OK to have set literals?
+    pub(crate) fn set_literals_emabled(&self) -> bool {
+        self.env.borrow().set_literals
     }
 }
 

starlark/src/eval/mod.rs

@@ -36,6 +36,7 @@ use crate::eval::module::Module;
 use crate::eval::stmt::AstStatementCompiled;
 use crate::eval::stmt::BlockCompiled;
 use crate::eval::stmt::StatementCompiled;
+use crate::linked_hash_set::value::Set;
 use crate::syntax::ast::BinOp;
 use crate::syntax::ast::UnOp;
 use crate::syntax::ast::*;
@@ -411,18 +412,6 @@ fn eval_transformed<E: EvaluationContextEnvironment>(
     }
 }
 
-fn make_set<E: EvaluationContextEnvironment>(
-    values: Vec<Value>,
-    context: &EvaluationContext<E>,
-    span: Span,
-) -> EvalResult {
-    context
-        .env
-        .env()
-        .make_set(values)
-        .map_err(|err| EvalException::DiagnosedError(err.to_diagnostic(span)))
-}
-
 // An intermediate transformation that tries to evaluate parameters of function / indices.
 // It is used to cache result of LHS in augmented assignment.
 // This transformation by default should be a deep copy (clone).
@@ -535,11 +524,14 @@ fn eval_expr<E: EvaluationContextEnvironment>(
             Ok(r.into())
         }
         ExprCompiled::Set(ref v) => {
+            if !context.env.env().set_literals_emabled() {
+                return t(Err(ValueError::TypeNotSupported("set".to_string())), expr);
+            }
             let mut values = Vec::with_capacity(v.len());
             for s in v {
                 values.push(eval_expr(s, context)?);
             }
-            make_set(values, context, expr.span)
+            t(Set::from(values), expr)
         }
         ExprCompiled::ListComprehension(ref expr, ref clauses) => {
             let mut list = Vec::new();
@@ -554,6 +546,10 @@ fn eval_expr<E: EvaluationContextEnvironment>(
             Ok(Value::from(list))
         }
         ExprCompiled::SetComprehension(ref expr, ref clauses) => {
+            if !context.env.env().set_literals_emabled() {
+                return t(Err(ValueError::TypeNotSupported("set".to_string())), expr);
+            }
+
             let mut values = Vec::new();
             eval_one_dimensional_comprehension(
                 &mut |context| {
@@ -563,7 +559,8 @@ fn eval_expr<E: EvaluationContextEnvironment>(
                 clauses,
                 context,
             )?;
-            make_set(values, context, expr.span)
+
+            t(Set::from(values), expr)
         }
         ExprCompiled::DictComprehension((ref k, ref v), ref clauses) => {
             let mut dict = Dictionary::new_typed();

starlark/src/linked_hash_set/mod.rs

@@ -7,5 +7,5 @@ pub(crate) mod value;
 /// Include `set` constructor and set functions in environment.
 pub fn global(env: &mut Environment, type_values: &mut TypeValues) {
     self::stdlib::global(env, type_values);
-    env.with_set_constructor(Box::new(crate::linked_hash_set::value::Set::from));
+    env.enable_set_literals();
 }