Drasil icon indicating copy to clipboard operation
Drasil copied to clipboard

Refactor `drasil-gool`

Open B-rando1 opened this issue 1 year ago โ€ข 13 comments

The changes I made this summer went pretty well, but the organization of drasil-gool fell behind a bit. Some of the structure and naming is a bit misleading.

  • It is called drasil-gool when it contains both GOOL and GProc.
  • All of its modules are in drasil-gool/Drasil/GOOL.
  • Some of the helper files have misleading names and/or have functions in the wrong places:
    • LanguagePolymorphic.hs has functions that the Julia renderer doesn't use.
    • CommonPseudoOO.hs has functions that are Shared, i.e. not intrinsically OO.
    • There may be others. For example, CLike.hs may have functions that are used by languages that are not particularly C-like, e.g. Julia.
  • There are some naming inconsistencies in both drasil-gool and drasil-code:
    • Procedural is sometimes referred to as Proc, sometimes as GProc.
    • Features common to both generic languages are referred to as Common or Shared.
  • There's a lot of code duplication between CodeInfoOO.hs and CodeInfoProc.hs.

I would recommend making the following changes:

  • [x] I don't think it's overly important to rename drasil-gool, but if we can find a way to refer to GOOL and GProc collectively, that could be nice. But we would need to use the name consistently for it to be a positive change.
    • drasil-genlang sounds good, but drasil-gen and drasil-lang exist ๐Ÿ˜„.
  • [x] I think we should split up drasil-gool/Drasil into GOOL, GProc, and Shared folders.
  • [x] We should inventory the helper functions (LanguagePolymorphic.hs, CommonPseudoOO.hs, etc.). We'll want to rename files and move functions to different files, so that the name of each file accurately describes the kinds of functions that can be found in it.
  • [x] We need to pick a name for procedural and shared programs and stick with it. I like GProc and Shared personally.

B-rando1 avatar Aug 21 '24 17:08 B-rando1

@sarrasoussia Do you want to try having a look at some of the points @B-rando1 mentioned? I was thinking specifically the last point might be a good starting point.

We should merge the implementations for most of the typeclass methods in the CodeInfoX.hs files. This should be pretty easy, and will look a lot like the LanguagePolymorphic.hs functions for the renderers.

balacij avatar May 14 '25 14:05 balacij

I appreciate anything you can do here, it will certainly make my life easier in the fall ๐Ÿ™‚

Regarding my last point above, I agree that it could be a good place to start. I believe this PR is the one I was referring to. There's a lot of changes, but if you zoom in on the LanguagePolymorphic diff it should hopefully give you the right idea.

I can't quite remember if I was envisioning us ending up with 3 files (CodeInfoShared, CodeInfoProc, CodeInfoOO) or 1 containing everything. The RendererClassesX files follow the former, while the gooltest (e.g. this file) files consolidate everything into a single file. Either should work, and @balacij and @JacquesCarette should be able to help you make the decision.

Hope this helps!

B-rando1 avatar May 15 '25 12:05 B-rando1

I think since drasil-gen and drasil-lang exist and we want to avoid ambiguity with drasil-genlang, we can opt for one of these names:

  • [ ] drasil-codegen: it's clear but slightly generic
  • [ ] drasil-corelang: may be too generic here
  • [ ] drasil-goolproc: I think this is the most explicit one but it's less elegant

sarrasoussia avatar May 15 '25 17:05 sarrasoussia

drasil-codegen is probably best, at least for our current level of understanding.

JacquesCarette avatar May 17 '25 14:05 JacquesCarette

I dont understand why this point is an issue: LanguagePolymorphic.hs has functions that the Julia renderer doesn't use.

If it's a shared toolbox, then Julia just doesnโ€™t need all the tools in it. Does the unused code impacts performance?

sarrasoussia avatar May 20 '25 16:05 sarrasoussia

Could you be more precise about what code in LanguagePolymorphic is not used by Julia? [I mostly want to be more informed, I don't have any conclusions in any direction yet.]

JacquesCarette avatar May 20 '25 17:05 JacquesCarette

@sarrasoussia Good question! I believe I my motivations for that point were more organizational - making sure that the names of our files match what's inside them. I don't actually remember, but I'm guessing that other than the Julia renderer, every function in LanuagePolymorphic is used by every renderer. Would you be able to check it and confirm?

I wouldn't say that this change is as important as some of the others, but if you could look around and make any suggestions about which functions belong where, it might make things easier to find.

B-rando1 avatar May 20 '25 17:05 B-rando1

Julia does use LanguagePolymorphic, but only a subset of its functions. Similarly for the other renderers so I think no changes should be made to LanuagePolymorphic.hs

Image

sarrasoussia avatar May 27 '25 20:05 sarrasoussia

I checked the CodeInfoProc.hs and CodeInfoOO.hs and found some common instances:

  • SharedProg
  • ProgramSym
  • FileSym
  • BodySym
  • BlockSym
  • TypeSym, TypeElim
  • ScopeSym
  • VariableSym, VariableElim
  • ValueSym
  • Argument, Literal, MathConstant, VariableValue, CommandLineArgs
  • NumericExpression, BooleanExpression, Comparison, ValueExpression
  • FunctionSym
  • List, Set, InternalList
  • ThunkSym, ThunkAssign
  • VectorType, VectorDecl, VectorThunk, VectorExpression
  • StatementSym, AssignStatement, DeclStatement
  • IOStatement, StringStatement
  • FuncAppStatement, CommentStatement, ControlStatement
  • VisibilitySym, ParameterSym, MethodSym
  • ModuleSym

Im thinking of refactoring all the shared instance implementations into a common base module Drasil.Shared.CodeInfoBase

sarrasoussia avatar May 27 '25 20:05 sarrasoussia

Looking back to the CLike.hs, Julia does indeed use some of its functions

Image

Do you suggest moving them to a seperate file?

sarrasoussia avatar May 27 '25 20:05 sarrasoussia

Regarding the CLike file, after thinking about it some more I think there's some interpretation involved. Specifically, the structures from the CLike file that Julia uses definitely belong in the 'C-Like' bin, but it's unclear if it was an intentional choice for the Julia language to follow that design. I guess the question then becomes, is it enough for Julia and C to look similar for these structures for us to merge the implementations, particularly when the filename CLike is involved? Or do we make Julia use its own implementation, simply because it's not 'C-like enough'.

This has me thinking about the idea of 'puns' (i.e. independent designs/structures with identical encodings).

Sorry if that doesn't make a lot of sense ๐Ÿ˜… . Let me know if you want me to rephrase or have a question. I would say that for now you're probably fine to leave that file as-is, but I'd be interested to hear if @JacquesCarette has any thoughts here.

B-rando1 avatar May 28 '25 12:05 B-rando1

I checked the CodeInfoProc.hs and CodeInfoOO.hs and found some common instances: [...]

In a subsequent PR, yes, that likely makes sense.

JacquesCarette avatar May 28 '25 19:05 JacquesCarette

Amusingly, for all but inlineIf and while, everything that's called C-like and re-used by Julia could have been called "functional" too! Maybe some of that stuff deserves to be made even more generic?

JacquesCarette avatar May 28 '25 19:05 JacquesCarette