fix purity for dict.each

Author: giacomocavalieri

CHANGELOG.md

@@ -162,6 +162,10 @@
   unreachable.
   ([Giacomo Cavalieri](https://github.com/giacomocavalieri))
 
+- Fixed a bug where the Gleam standard library's `dict.each` function would
+  incorrectly be assumed to be pure.
+  ([Giacomo Cavalieri](https://github.com/giacomocavalieri))
+
 - The compiler now correctly tracks the minimum required version for constant
   record updates to be `>= 1.14.0`.
   ([Giacomo Cavalieri](https://github.com/giacomocavalieri))

compiler-core/src/analyse.rs

@@ -5,7 +5,7 @@ pub mod name;
 mod tests;
 
 use crate::{
-    GLEAM_CORE_PACKAGE_NAME,
+    GLEAM_CORE_PACKAGE_NAME, STDLIB_PACKAGE_NAME,
     ast::{
         self, Arg, BitArrayOption, CustomType, DefinitionLocation, Function, GroupedDefinitions,
         Import, ModuleConstant, Publicity, RecordConstructor, RecordConstructorArg, SrcSpan,
@@ -632,13 +632,61 @@ impl<'a, A> ModuleAnalyzer<'a, A> {
                 .last()
                 .map_or(prereg_return_type.clone(), |last| last.type_());
 
+            // `dict.do_fold` is a bit special: since it belongs to the stdlib
+            // it is considered pure by default.
+            // However, since it ends up calling its function argument its
+            // purity should actually be `Impure`. We need to special case it
+            // and set the value ourselves.
+            //
+            // You might wonder why `do_fold` needs this but other similar
+            // functions like `list.each` don't need this special handling.
+            // The key difference is `list.each` calls the higher order function
+            // in its gleam body:
+            //
+            // ```gleam
+            // fn each(list, fun) {
+            //   case list {
+            //     [] -> Nil
+            //     [first, ..rest] -> {
+            //       fun(first)
+            //       // ^^^ Here we're calling `fun`. It's happening in Gleam so
+            //       //     the compiler can see this and understand that `each`
+            //       //     is impure.
+            //       //     You might argue the purity actually depends on the
+            //       //     purity of `fun` itself. That's true! But it's a
+            //       //     separate known problem. For the time being we always
+            //       //     assume a function argument is impure.
+            //       each(rest, fun)
+            //     }
+            //   }
+            // }
+            // ```
+            //
+            // But since `do_fold` is an external the compiler can't know what
+            // is going on with its function argument and keeps thinking it must
+            // be pure
+            //
+            // ```gleam
+            // @external(erlang, "", "")
+            // fn do_fold(dict: Dict(k, v), fun: fn(k, v) -> a) -> Nil
+            // ```
+            //
+            let purity = if expr_typer.environment.current_package == STDLIB_PACKAGE_NAME
+                && expr_typer.environment.current_module == "gleam/dict"
+                && name == "do_fold"
+            {
+                Purity::Impure
+            } else {
+                expr_typer.purity
+            };
+
             let type_ = fn_(arguments_types, return_type);
             Ok((
                 type_,
                 body,
                 expr_typer.implementations,
                 expr_typer.minimum_required_version,
-                expr_typer.purity,
+                purity,
             ))
         });
 

compiler-core/src/type_/expression.rs

@@ -324,8 +324,8 @@ impl<'a, 'b> ExprTyper<'a, 'b> {
         };
 
         let purity = if is_trusted_pure_module(environment) {
-            // The standard library uses a lot of FFI, but as we are the maintainers we know that
-            // it can be trusted to pure pure.
+            // The standard library uses a lot of FFI, but as we are the
+            // maintainers we know that it can be trusted to be pure.
             Purity::TrustedPure
         } else if uses_externals {
             Purity::Impure
@@ -5335,11 +5335,10 @@ fn invalid_with_annotated_type(constant: TypedConstant, new_type: Arc<Type>) ->
     }
 }
 
-/// Returns `true` if the current function is one that the Gleam core team
+/// Returns `true` if the current module is one that the Gleam core team
 /// maintains and we know it to be pure.
 /// Used in purity tracking.
 fn is_trusted_pure_module(environment: &Environment<'_>) -> bool {
-    // We only t
     if environment.current_package != STDLIB_PACKAGE_NAME {
         return false;
     }

compiler-core/src/type_/tests/warnings.rs

@@ -3622,6 +3622,115 @@ pub fn main() {
     );
 }
 
+#[test]
+fn stdlib_list_each_is_not_marked_as_pure() {
+    assert_no_warnings!(
+        (
+            "gleam",
+            "gleam/list",
+            r#"
+pub fn each(list, f) {
+  case list {
+    [] -> Nil
+    [first, ..rest] -> {
+      f(first)
+      each(rest, f)
+    }
+  }
+}
+"#
+        ),
+        "
+import gleam/list
+pub fn main() {
+  list.each([1, 2, 3, 4], fn(x) { echo x })
+  Nil
+}
+"
+    );
+}
+
+#[test]
+fn dict_each_function_is_not_marked_as_pure() {
+    assert_no_warnings!(
+        (
+            "gleam_stdlib",
+            "gleam/dict",
+            r#"
+pub type Dict(key, value)
+
+@external(erlang, "maps", "new")
+@external(javascript, "../dict.mjs", "make")
+pub fn new() -> Dict(k, v)
+
+pub fn each(dict: Dict(k, v), fun: fn(k, v) -> a) -> Nil {
+  fold(dict, Nil, fn(nil, k, v) {
+    fun(k, v)
+    nil
+  })
+}
+
+@external(javascript, "../dict.mjs", "fold")
+pub fn fold(
+  over dict: Dict(k, v),
+  from initial: acc,
+  with fun: fn(acc, k, v) -> acc,
+) -> acc {
+  let fun = fn(key, value, acc) { fun(acc, key, value) }
+  do_fold(fun, initial, dict)
+}
+
+@external(erlang, "maps", "fold")
+fn do_fold(fun: fn(k, v, acc) -> acc, initial: acc, dict: Dict(k, v)) -> acc
+"#
+        ),
+        "
+import gleam/dict
+pub fn main() {
+  dict.each(dict.new(), fn(_, _) { echo 1 })
+  Nil
+}
+"
+    );
+}
+
+#[test]
+fn dict_fold_function_is_not_marked_as_pure() {
+    assert_no_warnings!(
+        (
+            "gleam_stdlib",
+            "gleam/dict",
+            r#"
+pub type Dict(key, value)
+
+@external(erlang, "maps", "new")
+@external(javascript, "../dict.mjs", "make")
+pub fn new() -> Dict(k, v)
+
+@external(javascript, "../dict.mjs", "fold")
+pub fn fold(
+  over dict: Dict(k, v),
+  from initial: acc,
+  with fun: fn(acc, k, v) -> acc,
+) -> acc {
+  let fun = fn(key, value, acc) { fun(acc, key, value) }
+  do_fold(fun, initial, dict)
+}
+
+@external(erlang, "maps", "fold")
+fn do_fold(fun: fn(k, v, acc) -> acc, initial: acc, dict: Dict(k, v)) -> acc
+"#
+        ),
+        "
+import gleam/dict
+pub fn main() {
+  dict.fold(dict.new(), Nil, fn(_, _, _) { Nil })
+  Nil
+}
+"
+    );
+}
+
 #[test]
 fn calling_local_variable_not_marked_as_pure() {
     assert_no_warnings!(