Import module collisions #235

0xtimmy · 2023-12-07T19:12:04Z

added compiler passes to resolve imports:
- ModuleFillImports.hs runs in the parser to handle import metadata (qualified/unqualified, specs, aliases)
- ModuleRename.hs renames imported identifiers so they have compliant characters

…, next up is actual resolution logic

minor cleanup of README and gibbon-compiler/gibbon.cabal

ulysses4ever · 2023-12-07T19:37:33Z

Cool! The current issue is that some tests fail in the build phase when you run them in bulk via

cabal run test-gibbon-examples

(as can be seen in the CI here) versus when you run them individually via

cabal run exe:gibbon -- --run examples/poly/AnonLambdas.hs

Is that right?

0xtimmy · 2023-12-07T23:18:20Z

Cool! The current issue is that some tests fail in the build phase when you run them in bulk via
cabal run test-gibbon-examples
(as can be seen in the CI here) versus when you run them individually via
cabal run exe:gibbon -- --run examples/poly/AnonLambdas.hs
Is that right?

yes, I'm also not sure why it's printing out the rust and c config

ulysses4ever · 2023-12-08T17:19:37Z

I'm also not sure why it's printing out the rust and c config

It looks like it does this on the main branch too, so not something to worry for this PR. But something to investigate, indeed.

In the meantime: can you try to rebase on main once in a while? It'd make certain things easier.

ulysses4ever · 2023-12-11T00:07:03Z

Fingers crossed, it should pass the tests. I hope to look into code quality this coming week, so let’s maybe not merge it right away.

In the meantime, you absolutely have to rebase your branch on top of main, @0xtimmy!

ulysses4ever

Some excellent work here. But please, act on the comments.

ulysses4ever · 2023-12-13T20:30:03Z

gibbon-compiler/src/Gibbon/Passes/FreshConstructors.hs

+{-# LANGUAGE FlexibleInstances #-}
+{-# LANGUAGE FlexibleContexts  #-}
+
+module Gibbon.Passes.FreshConstructors (freshConstructors) where 


Please, add haddock-comments for every of the three passes you add: module-level and the main-function level. For inspiration, see Fllatten.hs:

module-level:

gibbon/gibbon-compiler/src/Gibbon/Passes/Flatten.hs

Lines 6 to 7 in efd6b60

-- | Put the program in A-normal form where only varrefs and literals are

-- allowed in operand position.

main-function level:

gibbon/gibbon-compiler/src/Gibbon/Passes/Flatten.hs

Lines 24 to 31 in efd6b60

-- | Flatten ensures that function operands are "trivial".

--

-- In the process, it also lifts lets out of case scrutinees, if

-- conditions, and tuple operands.

--

-- Note that it does not require tail expressions to be trivial.

-- For example, it allows AppE and PrimAppE in the body of a

-- let-expression.

ulysses4ever · 2023-12-13T20:30:29Z

gibbon-compiler/src/Gibbon/Language/Syntax.hs

@@ -313,6 +313,7 @@ data FunMeta =
  FunMeta
    { funRec                 :: FunRec
    , funInline              :: FunInline
+    --, funModule              :: String


Mm, why is this commented-out?

ulysses4ever · 2023-12-13T20:39:30Z

gibbon-compiler/src/Gibbon/Passes/ModuleFillImports.hs

+                  Nothing -> acc
+              Nothing -> error "how did we get here?"
+
+      let importedConstrs = foldr (\(Prog idefs _ _) acc -> acc ++ (foldr applyImportMeta [] (L.map (\(constrName, _) -> (toVar constrName)) (foldr (\(DDef _ _ dataCons) acc -> acc ++ dataCons) [] idefs)))) initImportedNames imported_progs


How about we split this line into several? E.g. one foldr per line...

ulysses4ever · 2023-12-28T03:02:17Z

gibbon-compiler/src/Gibbon/HaskellFrontend.hs

@@ -179,6 +181,40 @@ type TopTyEnv = TyEnv TyScheme

 type TypeSynEnv = M.Map TyCon Ty0

+-- ========================================================
+
+getImportMeta :: [H.ImportDecl a] -> M.Map Var (Var, Bool, Maybe [Var])


I may have mentioned it elsewhere but I want to reiterate: this Map is a core data structure in your work, and that's why It should have a name introduced either as type or a newtype (the former is easier of a change but long-term less sustainable).

…}_{name} effect)

… of internal type names

Test case changes

ulysses4ever · 2024-05-15T17:37:01Z

gibbon-compiler/solve_ilp_-34.py

@@ -0,0 +1,22 @@
+from docplex.mp.model import Model


This PR adds three py-files inside gibbon-compiler: I don't think they should be there. they seem to be leftovers from the layout work which is not related to this PR?..

ulysses4ever · 2024-05-15T17:39:29Z

gibbon-compiler/src/Gibbon/Common.hs

@@ -148,7 +148,7 @@ newUniq = state (\x -> (x, x+1))

 -- | Generate a unique symbol by attaching a numeric suffix.
 gensym :: MonadState Int m => Var -> m Var
-gensym v = state (\n -> (cleanFunName v `varAppend` "_" `varAppend` toVar (show n), n + 1))
+gensym v = state (\n -> (cleanFunName v `varAppend` toVar (show n), n + 1))


Could you explain why this underscore was removed? Preferably with examples.

ulysses4ever · 2024-05-15T17:40:12Z

gibbon-compiler/build.log

@@ -0,0 +1,8 @@
+Build profile: -w ghc-9.2.8 -O1


The *.log files don't belong in the repo.

ulysses4ever · 2024-05-15T17:42:27Z

gibbon-compiler/src/Gibbon/Language/Syntax.hs

+deriving instance
+         (NFData (TyOf ex), NFData (ArrowTy (TyOf ex)), NFData ex,
+          Generic (ArrowTy (TyOf ex))) =>
+         NFData (Prog ex)


Did you actually edit this line or just reformatted it? If the latter, then better to revert this. If you want to improve formatting, please, submit a separate patch. Alternatively, you could have a separate commit for formatting changes. Currently, there are 73 commits in this PR: that's too many. Please, research (or ask me) how to "squash" commits. One commit is probably enough for the feature (modules) itself. Another commit could be about formatting.

Same applies to all the formatting-only changes, not just this line. E.g. in this file above there are many import-list changes that only reformat it. It better go into a separate commit.

ulysses4ever · 2024-05-15T17:44:36Z

gibbon-compiler/src/Gibbon/Language/Syntax.hs

+data ProgModule ex = ProgModule String (Prog ex) [ImportDecl SrcSpanInfo]
+data ProgBundle ex = ProgBundle [ProgModule ex] (ProgModule ex)


Would be awesome to add comments to individual fields, see an example here: https://kowainik.github.io/posts/haddock-tips#what-can-be-documented

This is great, I'll go through and clean up what comments I have

ulysses4ever · 2024-05-15T17:51:19Z

gibbon-compiler/src/Gibbon/L0/Syntax.hs

+
+type Prog0 = Prog Exp0
+
+-------------------------------------------------------------------------------


Why the empty lines and the dashes? I'd just continue the list of aliases that was already there (with Ddfe0, FunDef0, etc.)

ulysses4ever · 2024-05-15T17:52:15Z

gibbon-compiler/src/Gibbon/L0/Typecheck.hs

@@ -34,13 +34,24 @@ newtype TcM a = TcM (ExceptT Doc PassM a)
 instance MonadFail TcM where
  fail = error

-runTcM :: TcM a -> PassM (Either Doc a)
+runTcM ::   TcM a -> PassM (Either Doc a)


there are many space-only edits in type signatures in this file that don't make sense to me. Could we revert them?

ulysses4ever · 2024-05-15T17:52:51Z

gibbon-compiler/src/Gibbon/L1/Interp.hs

+   (PrintChar, [c]) -> do
+        tell $ string8 (show c)
+        pure $ VProd []


Mm, I'm surprised to see this change. Is it related to modules at all?

ulysses4ever · 2024-05-15T17:53:50Z

gibbon-compiler/src/Gibbon/Compiler.hs

+        {-
        fst $ runPassM defaultConfig cnt0
-                (freshNames l0 >>=
-                 (\fresh -> dbgTrace 5 ("\nFreshen:\n"++sepline++ "\n" ++pprender fresh) (L0.tcProg fresh)))
+                (freshBundleNames l0_bundle >>=
+                  (\bundled -> dbgTrace 5 ("\nFreshen:\n" ++ sepline ++ "\n" ++ pprender bundled) 
+                    (L0.tcProg (fst $ runPassM defaultConfig 1 
+                      (freshNames (fst $ runPassM defaultConfig 0 
+                        (bundleModules bundled)
+                      ))
+                    ))
+                  )
+                )
+        -}


The stuff that's commented out should just be erased.

ulysses4ever · 2024-05-15T17:54:58Z

gibbon-compiler/examples/imports/failing.md

@@ -0,0 +1,61 @@
+# Poly


This file is also a layout-patch leftover?

this was leftover notes for myself, good to remove from the diff

ulysses4ever · 2024-05-15T17:57:16Z

Hey @0xtimmy! I pushed the first bulk of comments to your modules work. The comments are very cosmetic so far, but addressing them would make further reviewing easier...

ulysses4ever · 2024-08-26T13:59:04Z

gibbon-compiler/src/Gibbon/L0/Typecheck.hs

-tcFun :: DDefs0 -> Gamma -> FunDef0 -> PassM FunDef0
+tcFun ::   DDefs0 -> Gamma -> FunDef0 -> PassM FunDef0


There is a bunch of this <space><space> in the diff. How did it happen and can we get rid of them?

ulysses4ever · 2024-08-26T14:02:02Z

gibbon-compiler/examples/add1.hs

-main :: Tree
-main = add1 (Node (Leaf 1) (Leaf 2))
+gibbon_main :: Tree
+gibbon_main = Add.add1 (Node (Leaf 1) (Leaf 2))


I was annoyed that this example doesn't compile too. But does this change fix it? I seem to remember that gibbon_main can only return scalar types, I think (Int, etc.). Maybe adding a summation over the resulting tree could make it work?

ulysses4ever · 2024-08-26T14:04:25Z

Hey @0xtimmy! Will you be able to work on finishing touches to it a little more over the semester? (It's okay if not, we just need an idea of what to expect.) There's some minor comments left.

And then the CI (GitHub Actions) is red: we have to make it green... Rebasing on top of main would be the first step with that.

0xtimmy and others added 14 commits October 25, 2023 08:09

collision example

e1ec3aa

repeatable module import collisions

4e81af3

declaration renaming works

a2deafb

module rename can modify function callsites and function decalrations…

7ff377a

…, next up is actual resolution logic

finished basic module import resolution

1b2cfaf

parses modules in defs and main as well

f05cbdd

added a pass to resolve imports in desugarModule

2776226

minor cleanup of README and gibbon-compiler/gibbon.cabal

3d80c84

aliases work

6145e02

qualified imports

434712c

spec imports

34e6618

finished test cases

c8bb2d1

Merge pull request #233 from ulysses4ever/ap-minor-cleanup

eca7cdd

minor cleanup of README and gibbon-compiler/gibbon.cabal

fix? handling local identifier

97b9c39

some testing

2659da7

vidsinghal force-pushed the main branch from a3fa399 to 5b34c91 Compare December 10, 2023 20:25

0xtimmy added 2 commits December 10, 2023 18:34

fixed interp

c3e9aca

fixing a test (did not specify the sum function)

22cc526

0xtimmy added 3 commits December 10, 2023 20:05

added ext parsing

ddbae71

fixed an issue with LetE

cd2ec79

some oddness

b114741

ulysses4ever requested changes Dec 13, 2023

View reviewed changes

ulysses4ever reviewed Dec 28, 2023

View reviewed changes

0xtimmy added 3 commits January 11, 2024 07:13

more bug fixing

2f4992c

add fixed linear ext handling

19d4a65

let _default contructors pass through

9bc6837

0xtimmy and others added 24 commits February 14, 2024 11:28

fixing merge issue

0df2511

removed qualification from unique name (gets rid of the ugly {package…

9824c51

…}_{name} effect)

fixed .gib extension

ba66015

fixed verbodity issue

237b0e5

adding the fix to the interpreter

e10b206

PR cleanup

9662f04

cleaning up build file

f66afdc

reverting common.hs

41d3b82

some more test fixes

1349a7c

some more test changes

86c50d6

alternatives for showing internal type names

6062341

Merge branch 'main' of github.com:iu-parfunc/gibbon

d1ff39b

changed layout benches to print the blogs in a way that's independent…

c6b38d2

… of internal type names

removing interp changes

24b33ba

handling _default in freshBundle CaseE

55373d2

fixed more test cases

1387304

fixing manyFuncs

1d9fed9

solve layout constrs error

4330552

test cases pass

0d17d3a

Merge pull request #256 from iu-parfunc/test-case-changes

65ab88c

Test case changes

removed some tests missing from main

bbaa2d7

adressed comments and added more robust descriptions to all new modules

97eff1d

added a pass to resolve imports in desugarModule

a335c8f

merged in main

ecac589

ulysses4ever requested changes May 15, 2024

View reviewed changes

removing extra files from diff

6756e1d

ulysses4ever reviewed Aug 26, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Import module collisions #235

Import module collisions #235

0xtimmy commented Dec 7, 2023 •

edited by ulysses4ever

Loading

ulysses4ever commented Dec 7, 2023

0xtimmy commented Dec 7, 2023

ulysses4ever commented Dec 8, 2023

ulysses4ever commented Dec 11, 2023

ulysses4ever left a comment

ulysses4ever Dec 13, 2023

ulysses4ever Dec 13, 2023

ulysses4ever Dec 13, 2023

ulysses4ever Dec 28, 2023

ulysses4ever May 15, 2024

ulysses4ever May 15, 2024

ulysses4ever May 15, 2024

ulysses4ever May 15, 2024

ulysses4ever May 15, 2024

0xtimmy Jun 21, 2024

ulysses4ever May 15, 2024

ulysses4ever May 15, 2024

ulysses4ever May 15, 2024

ulysses4ever May 15, 2024

ulysses4ever May 15, 2024

0xtimmy Jun 21, 2024

ulysses4ever commented May 15, 2024

ulysses4ever Aug 26, 2024

ulysses4ever Aug 26, 2024

ulysses4ever commented Aug 26, 2024

	-- \| Put the program in A-normal form where only varrefs and literals are
	-- allowed in operand position.

	-- \| Flatten ensures that function operands are "trivial".
	--
	-- In the process, it also lifts lets out of case scrutinees, if
	-- conditions, and tuple operands.
	--
	-- Note that it does not require tail expressions to be trivial.
	-- For example, it allows AppE and PrimAppE in the body of a
	-- let-expression.

		data ProgModule ex = ProgModule String (Prog ex) [ImportDecl SrcSpanInfo]
		data ProgBundle ex = ProgBundle [ProgModule ex] (ProgModule ex)


		type Prog0 = Prog Exp0

		-------------------------------------------------------------------------------

		tcFun :: DDefs0 -> Gamma -> FunDef0 -> PassM FunDef0
		tcFun :: DDefs0 -> Gamma -> FunDef0 -> PassM FunDef0

Import module collisions #235

Are you sure you want to change the base?

Import module collisions #235

Conversation

0xtimmy commented Dec 7, 2023 • edited by ulysses4ever Loading

ulysses4ever commented Dec 7, 2023

0xtimmy commented Dec 7, 2023

ulysses4ever commented Dec 8, 2023

ulysses4ever commented Dec 11, 2023

ulysses4ever left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ulysses4ever commented May 15, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ulysses4ever commented Aug 26, 2024

0xtimmy commented Dec 7, 2023 •

edited by ulysses4ever

Loading