EDIT: `set!` sometimes gets untyped forms.

Dynamic.mod will cause loops on some arguments, the following code causes a hang:

;; assume arr is ['x 'y]
(= 0 (mod (length arr) 2)))

Changing mod to imod fixes the code above. However, the definition of Dynamic.mod itself seems fine, so this is likely stemming from some compiler issue.

The underlying problem is coming from the length call. Using a plain old number works fine:

(= 0 (mod 2 2))

Here's the fix: call eval on length

鲤 (= 0 (mod (eval (length ['x 'y])) 2))
=> true
鲤 

This is likely a regression from some other change—seems related to some other differences re: the necessity of calling eval. What's strange is that it doesn't seem to have affected things uniformly.

Taking a quick look at the code, there's no obvious reason for this issue. length is defined as a command, which afaict, is still evaluated immediately in the evaluator 🤔

If I had to guess, we might need to reconsider the evaluator's symbol lookup clause, since this seems to be the only place where the hang might be happening (command handling hasn't changed)—we don't evaluate enough when retrieving some symbol or something.

Is it possible length is recomputed every time the first argument is used/looked up (maybe this is related to laziness in a subtle way)?

Is it possible length is recomputed every time the first argument is used/looked up (maybe this is related to laziness in a subtle way)?

Ah that's a good thought. I do think the issue is definitely too much laziness somewhere. Tinkering now.

Ah, it's the final clause in the Evaluator symbol lookup (use modules lookup) that's causing the problem; in rare cases, e.g. this one, the use module name winds up conflicting with a parent module name and causes a loop. Yet another issue related to module relationship mgmt 😢

( foldl                                                                                                                                                                                       
                    (<|>)                                                                                                                                                                                     
                    Nothing                                                                                                                                                                                   
                    ( map                                                                                                                                                                                     
                        ( \(SymPath p' n') ->                                                                                                                                                                 
                            lookupBinder (SymPath (p' ++ (n' : p)) n) (contextGlobalEnv ctx)  -- this is the problematic part.  depending on symbol qualification, parent env paths, and use paths, this can intro a loop                                                                                                           
                              >>= \(Binder meta found) -> checkPrivate meta found                                                                                                                             
                        )                                                                                                                                                                                     
                        (Set.toList (envUseModules (contextGlobalEnv ctx)))                                                                                                                                   
                    )                                                                                                                                                                                         
                

Hopefully the fix is as simple as nubbing the Use modules from the parent modules or some such. This might be another case where the approach of requiring qualification of a sympath as an explicit step before lookup is performed (like I did recently for contexts) helps.

@scolsen Can we enforce that at the type level?

Yep, That's what I tried to do recently with the QualifiedPath type in Context.hs and elsewhere

In terms of lookups, the problem that occurs somewhat consistently is that the sympaths we wind up passing to the env lookup routines and what we actually expect to be passing differ or wind up containing duplications/conflict w/ parent paths or other qualifications being performed. I can't totally pinpoint where the duplication is creeping in for this case but, what basically seems to be happening in this code path for length is that we somehow lookup length in something like Dynamic.Dynamic.length, we find the first Dynamic, but not the second, the Dynamics parent is the global env, and somehow we perform the same exact lookup in the global env all over again, causing a loop.

@scolsen Hmm, yes it's annoying that it keeps popping up. Perhaps we could add some pretty big (expensive) checks in the lookup functions to validate the inputs, and have a way of turning those off "in production"? The type based solution is of course much neater, but maybe it's too hard to prove correctness here given the unlimited recursion.

There's actually something more nuanced happening here. Turns out the loop is being triggered by (= m b) and only when the input is something like (length ['x 'y])

so, it turns out, the values being passed to set are untyped, while they should be typed as dynamic

In particular, this seems to happen when calling a command, such as length. #1209 is a temporary fix, but we should really figure out why some forms are coming into set! completely untyped.

@scolsen Sounds like a missing clause for the set! form somewhere perhaps?