purescript-deprecated/purescript-maps

Accidental observable mutation

Closed this issue · 2 comments

With thanks to @Thimoteus, who did most of the work tracking this down:

module Main where

import Prelude

import Data.StrMap as SM
import Data.Tuple
import Data.Tuple.Nested
import Data.Monoid.Additive

import Control.Monad.Eff
import Control.Monad.Eff.Console (log)

m :: SM.StrMap (Array Int)
m = SM.fromFoldable ["one" /\ [1]]

go :: SM.StrMap (Array Int) -> Array Int
go = SM.foldMap \_ v -> v

main = do
  log (show (go m))
  log (show (go m))
  log (show (go m))
  log (show (go m))

This program prints:

[1]
[1,1]
[1,1,1]
[1,1,1,1]

I think this is because the implementation of _fold captures the accumulator argument (mz) in a closure, and so defining go in this point-free style means that the same mz is accessible (and gets mutated) in subsequent applications of go. The offending code:

// jshint maxparams: 1
exports._foldM = function (bind) {
  return function (f) {
    return function (mz) {
      return function (m) {
        function g(k) {
          return function (z) {
            return f(z)(k)(m[k]);
          };
        }
        for (var k in m) {
          if (m.hasOwnProperty(k)) {
            mz = bind(mz)(g(k));
          }
        }
        return mz;
      };
    };
  };
};

Interestingly, defining go in a pointful way, or even making it polymorphic by removing the type signature, causes this bug to go away, because it means that you get a new mz each time.

garyb commented

Wow. Once again illustrating how careful one has to be when using the FFI.

garyb commented

This looks like a rule we could probably write a check for using eslint though (assignments to an argument variable above the current scope), maybe it's worth switching to that and putting together a ruleset for PS FFI code.