Drasil icon indicating copy to clipboard operation
Drasil copied to clipboard

Lowering GOOLState creates problems for typeclasses with concrete types (e.g. `TypeElim`)

Open B-rando1 opened this issue 3 months ago • 3 comments

@JacquesCarette and @TOTBWF gave the impression that there's more to discuss here, so I'm writing down what we've seen so far.

InterfaceCommon has the TypeElim typeclass, which is used for getting information about a type. Here is its current spec: https://github.com/JacquesCarette/Drasil/blob/9f6dd893f5989f75f620e495398bf2a101ce0156/code/drasil-gool/lib/Drasil/Shared/InterfaceCommon.hs#L92-L94

Then the renderers and CodeInfos implement this typeclass, e.g. for CodeInfoOO: https://github.com/JacquesCarette/Drasil/blob/9f6dd893f5989f75f620e495398bf2a101ce0156/code/drasil-gool/lib/Drasil/GOOL/CodeInfoOO.hs#L105-L107

The issue is, part of moving GOOLState down to individual renderers involves changing the r (Type r) to Type r. My understanding is that this removes the state that we had to 'unwrap' before, so the implementations can't typecheck.

Reed came up with this solution: parameterize the output of getTypeString so that it can be string in some cases, and State String in others:

class (TypeSym r) => TypeElim r where
  type TypeName r = t | t -> r
  getType :: Type r -> CodeType
  getTypeString :: Type r -> TypeName r

And the updated CodeInfoOO:

instance TypeElim CodeInfoOO where
  type TypeName CodeInfoOO = State ValueState String
  getType _     = Void
  getTypeString tp = tp

I believe the problem is that we may need to do this many more times, because other typeclasses also involve types that previously were always stateful, but now are only sometimes stateful.

B-rando1 avatar Sep 18 '25 20:09 B-rando1

What is getTypeString used for? The name gives a clue, yes, but where is it actually called and for what reason? Maybe the reason is not good enough. And maybe the use site could be more informative about what information is actually needed, thereby giving us a hint as to a better solution.

JacquesCarette avatar Sep 19 '25 01:09 JacquesCarette

To provide a bit more context, in all of the renderers except CodeInfoX, the Type type is instantiated as TypeData: https://github.com/JacquesCarette/Drasil/blob/9f6dd893f5989f75f620e495398bf2a101ce0156/code/drasil-gool/lib/Drasil/Shared/AST.hs#L135

Julia, for example, implements TypeElim as: https://github.com/JacquesCarette/Drasil/blob/9f6dd893f5989f75f620e495398bf2a101ce0156/code/drasil-gool/lib/Drasil/GProc/LanguageRenderer/JuliaRenderer.hs#L196-L201

As far as uses go, getTypeString seems to be used wherever the generated code needs the type of a value. The uses are deeply nested and hard to analyze, but here's the uses I found:

  • Function return types (it seems like parameter types might be handled separately)
  • The name of a class, which is necessary for creating new objects, accessing static attributes, etc.
  • Parameterized types like Lists, Sets, and functions use getTypeString to recursively define their own typeString

Along with this, the typeDoc attribute of TypeData seems to have related uses, which all stem from the type' method of the InternalTypeElim typeclass in RendererClassesCommon.

B-rando1 avatar Sep 19 '25 13:09 B-rando1

Since GOOL is supposed to be a renderer, the typeDoc :: Doc parameter is the main one. Also having a parallel String seems like a hack (which I'm sure I condoned at some point). The CodeType is also a slight hack that enables some optimizations. It should probably also be removed and the missing optimizations re-enabled later with a better design.

Recursive uses are not an issue since they are forced by the existence of typeString. It is the use for making the name of a class that seems more problematic. That seems to be the real source of the problem.

JacquesCarette avatar Sep 19 '25 14:09 JacquesCarette