actonlang / acton

The Acton Programming Language
https://www.acton-lang.org/
BSD 3-Clause "New" or "Revised" License
79 stars 7 forks source link

actonc bug for module hierarchy overlap #1657

Closed plajjan closed 1 week ago

plajjan commented 9 months ago

Acton Version

0.19.2.20240121.17.6.48

Steps to Reproduce and Observed Behavior

I am trying to build a module hierarchy like this:

which I think should be valid, so I can later import from import foo or import foo.bar or both... but actonc doesn't like it:

actonc: Maybe.fromJust: Nothing
CallStack (from HasCallStack):
  error, called at libraries/base/Data/Maybe.hs:150:21 in base:Data.Maybe
  fromJust, called at ActonCompiler.hs:460:43 in main:Main

to reproduce:

mkdir base/src/foo
touch base/src/foo.act base/src/foo/bar.act
make

Expected Behavior

It should work!

plajjan commented 9 months ago

Failing function is this:

-- return task where the specified root actor exists
filterMainActor env opts paths binTask
                         = case lookup n (fromJust (Acton.Env.lookupMod m env)) of        -- <<<<<<<< this line
                               Just (A.NAct [] A.TNil{} (A.TRow _ _ _ t A.TNil{}) _)
                                   | prstr t == "Env" || prstr t == "None"
                                      || prstr t == "__builtin__.Env"|| prstr t == "__builtin__.None"-> do   -- TODO: proper check of parameter type
                                      return (Just binTask)
                                   | otherwise -> return Nothing
                               Just t -> return Nothing
                               Nothing -> return Nothing
  where mn                  = A.mname qn
        qn@(A.GName m n)    = rootActor binTask
        (sc,_)              = Acton.QuickType.schemaOf env (A.eQVar qn)

while I committed this function, it's mostly code I stole from elsewhere. I don't feel quite at home with lookupMod etc, so would appreciate @nordlander @sydow help here :)

sydow commented 9 months ago

It seems to me that the problem is in Env.addMod. This function seems not to be prepared for adding module foo after module foo.bar (and file names are reordered in computations of strongly connected components and filtering out acyclic dependencies, so it seems difficult to keep track of that).

Adding foo.bar (which is empty) to an empty environment results in the module type environment [(foo,NModule [(bar,NModule [])])].

When we now add the empty module foo to this environment, the bar entry in the environment for foo disappears because of
line 432 of Env.hs

addM [] te              = newte

which sets the environment for foo to be empty, throwing away the previous content. Change this line to

addM [] te              = newte ++ te

and this example works.

I have not considered whether to check for overlap of the domains of newte and te. What is the semantics if we define a function bar in foo and also have the submodule foo.bar? Probably an error, or...

plajjan commented 8 months ago

Discussed in meeting, #1659.

We agree that we should allow for having hierarchical modules as described in the example. This does indeed mean that a bar in the foo module will collide with the foo.bar module. We need to detect and report this as an error to the user. There are probably many implementation specific questions around this that we did not dive into full details on, for example how this interacts with module aliasing (import foo as bar).

plajjan commented 1 month ago

@nordlander here's that issue we talked about in todays meeting. @sydow posted a fix above and I was thinking I'd just go ahead with that now and we can add in more checks later.

nordlander commented 1 month ago

Ahh, memory comes back! But Björn is right, all that's missing is a check that foo doesn't contain a competing definition of bar. So trivial that it can wait... :-)