Split records of SymbolT and DefinitionS #384

matil019 · 2019-10-31T08:16:17Z

Part of #382

This PR refactors the internal of the compiler so that the two types, SymbolT and DefinitionS no longer are simultaneously sum types and record types.

A lot of the code was modified to adapt the changes but most of them are just to please the compiler. The following are the important changes.

Main changes

Building blocks

A tiny part of the Haskell package lens was ported as frege.compiler.common.Lens.
instance Alt Maybe was added to port the lens.

SymbolT

SymbolT was split into independent data types: SymT, SymL, SymD, SymC, SymI, SymV and SymA.
Two additional types were added to express a part of the set of the symbols:
- SymVal a sum of SymV and SymD
- SymMeth a sum of SymV and SymL
SymC.env and SymI.env was renamed to meth and its type changed to TreeMap String (SymMeth Global)
Getters and lenses are added to SymbolT, SymVal and SymMeth to help accessing fields. Lenses are prefixed with an underscore. For example:
- SymbolT._name can update the name field without pattern matching.
```
set SymbolT._name name sym ===
case sym of { SymbolT.V symv -> symv.{name}; SymbolT.T symt -> symt.{name}; ...; }
```
- SymbolT.name is a simple getter, and is much faster than view SymbolT._name.

DefinitionS

DefinitionS was split into independent data types.
Two additional types were added to express a part of the set of the definitions:
- ClassMemberS a sum of AnnDcl, NatDcl and FunDcl
- LetMemberS a sum of AnnDcl and FunDcl
Frege.y was modified to adapt the changes. The syntax remains the same.

New errors

Most of the type-unsafe uses of the sum-records were proved safe by the process of refactoring, but a few remained.

A genuine warning

frege.compiler.passes.Fix.unDoc.apply: Added a new warning. Emitted when a documentation comment is found next to a ModDcl i.e. a native module declaration. The comment is ignored after emitting the warning.

Internal errors

The following are internal errors. I added them anyway to please the compiler but I doubt any of them actually occurs unless something goes very wrong.

frege.compiler.common.SymbolTable.insUpdSymByName (formerly enterByName and updateSym): Emits an error when a Symbol which will belong to a SymC or SymI is neither SymV nor SymL.
frege.compiler.common.SymbolTable.toSymVBecauseLocal :: Symbol -> SymV Global is a new partial function. Used on an assumption that a Symbol is a SymV if its name is Local.
frege.compiler.gen.java.InstanceCode.lowerKindSpecialClasses: May emit an error if specialClassNames contains a non-class name.
frege.compiler.gen.java.InstanceCode.lowerKindSpecialClasses: May emit an error if any of the class resolved by specialClassNames has SymL as its member.
frege.compiler.gen.java.InstanceCode.symMethAsSymV :: SymMeth Global -> SymV Global is a new partial function. Used on an assumption that all SymLs in SymC.meth and SymI.meth are resolved to SymVs at the time instanceCode is running.
frege.compiler.types.Global.instTSym: Emits an error if TName pPreludeBase "->" resolves to a non-SymT.
frege.compiler.types.Global.instTauSym: Emits an error if TCon.name resolves to a non-SymT.

I hope some of the above can be eliminated by applying the same refactoring to QName some other time.

Performance concerns

fregec

The performance of the compiler was tested by compiling the source at master with different versions of fregec.jar.

The command make clean compiler1 was run 5 times each.

branch	commit id	average secs	%
master	`a3c4f77`	78.59	100
no-sum-symbolt	`11bdb3f`	81.25	103.3

The performance impact appears to be minimal. (Previously I reported the time increase was 0.9% but I was wrong.)

fregedoc

On the other hand, fregedoc suffers from a significant performance penalty.

The command java -jar fregec-<version>.jar frege.tools.Doc -d out fregec-<master>.jar was run 5 times each.

branch	commit id	average secs	%
master	`a3c4f77`	51.86	100
no-sum-symbolt	`11bdb3f`	65.33	126

frege IDE

Not tested because I don't have an IDE but the same performance drop as fregedoc is expected.

Notes

This is not an exhaustive effort. Only parts of the code which were complained by the compiler were modified, so there may be redundant uses of SymbolT left around.

This is needed by the lenses to be added later.

See frege/compiler/types/Symbols.fr As a first step to decomposing the sum-record SymbolT, SymT was isolated to an independent data type. Some parts of code were deliberately rewritten into refutable case analyses so that compiler warnings are triggered.

The field accessors of SymbolT were replaced with lenses. Partial functions were deliberately introduced to trigger compilation warnings where certain constructor is implicitly assumed (for example, an unsafe conversion `SymbolT g -> SymD g` is inserted to a piece of code which accesses `flds`, which exists only on `SymD`.)

Lots of redundant unsafeToSymVs and its friends were removed.

by tightening the types of parameters

- removed redundant pattern from `sref` - changed the type of `overSig` to take `SymV Global`

By changing the type of the local function `rbSymT` to return `SymT`

by tightening the parameter types

by tightening the types of parameters

by tightening the parameters

allvars was changed from monadic value to a plain function because it doesn't change nay state. Some functions in other modules had their parameters tightened.

Instead of crashing, erroneous cases are simply treated as un-matched cases.

All SymMeths in SymC.meth should have been resolved to SymV in this phase (at least that's how I understand the code). An error here, if any, is an internal error, and is reported as a pure error to help find a bug in the compiler.

Some functions in Global assumes certain propeties of Symbols. Those assumptions should be correct unless something goes very wrong. An error here, if any, is an internal error, and is reported as a pure error to help find a bug in the compiler.

Since only SymMeth (i.e. SymV or SymL) can be a member of SymC.meth or SymI.meth, if 'sym' is not SymMeth, it is an error. A new error message was added to report it instead of just crashing.

A part of MethodCall.nativeCall uses unsafePartialView a lot. The reason is that it was written by myself as a prototype and is not polished yet.

Instead of crashing, erroneous cases are simply treated as un-matched cases.

'allfuns' now contain SymVs only. If non SymVs are in an env of a Symbol, they are just now ignored. Some of the other functions had their types changed in order to reflect that change.

Instead of crashing, erroneous cases are simply treated as un-matched cases.

being "private" crashes fregedoc (but not fregec).

optimizations from now on

Particularly unsafePartialView (defined in f.c.common.Lens) is no longer used anywhere.

Added simple getters to SymbolT to be used instead of 'view'ing a 'Lens'. In Haskell, 'view' can have significant overhead than simple pattern matching. The existing lenses were given a leading underscore, like the prisms.

We don't use 'review', so Traversal is enough. Well, we do use 'is', but 'has' can be used for the purpose. Prism depends on Profunctor. In Haskell, the "profunctor" is a huge package. If, in some day, we port it to Frege, the compiler depending on it may be a problem.

The same refactoring as done to SymbolT. Now all uses of 'view's were replaced by simple getters. This contributes to a big reduction of performance penalty.

A redundant pattern match was removed.

Unused partial function The new type SymVal and its member fromSymbol serves the same purpose.

On invalid data constructor, 'updVis' behaves as 'id' instead of an error, just like 'over' in lens does.

An error case in symInfo was eliminated. Related functions were altered to reflect this change. Most notably Global.GenSt.symi8 now have (SymVal Global) instead of Symbol as keys. A new error message was added to lowerKindSpecialClasses. It didn't make sense to pass SymL to lowerKindAbstractFun because it would eventually put into symi8 but never referenced because symInfo were never called with SymL.

Removed unwanted diffs introduced by a series of modifications

Ingo60 · 2019-11-01T10:46:22Z

Great work, indeed! I think this justifies/requires a bump in the major version.

I shall figure out what I have to change in the eclipse plugin to accommodate this.
However, I prefer to wait until Christmas, where I have a couple of leisure days.

matil019 added 30 commits October 21, 2019 13:01

Split constructors of DefinitionS to individual data types

c5fd565

Add instance Alt Maybe

f577108

This is needed by the lenses to be added later.

[nocompile] Change all of SymbolT constructors to independent types

73d651a

Add instance Nice for SymT, SymL, etc.

e9fd748

Modify findV to return SymV, same for other Sym?s

1b872dc

Lots of redundant unsafeToSymVs and its friends were removed.

Change the return type of patLocal, mkLocal, vSym to SymV

7803c9e

Remuve uses of unsafePartialView from frege.compiler.Kinds

5a0459a

by tightening the types of parameters

Reduce warnings on frege.tools.doc.Utilities

f83552d

- removed redundant pattern from `sref` - changed the type of `overSig` to take `SymV Global`

Remove uses of unsafePartialView from frege.compiler.passes.Imp

8c03d3f

By changing the type of the local function `rbSymT` to return `SymT`

Reduce uses of partial functions in frege.ide.Utilities

596b2e0

Remove redundant cases in frege.compiler.gen.java.Common

a4cb129

Change the return type of envConstructors to [SymD Global]

d9c176b

Remove uses of partial functions in frege.compiler.passes.Instances

d8ce200

by tightening the parameter types

Remove some of partial functions in frege.compiler.Classes

6f2e5a4

by tightening the types of parameters

Remove a dead case in frege.compiler.gen.java.InstanceCode

efac3c0

Remove a dead case in frege.compiler.gen.java.DataCode

727656c

Remove some partial functions and dead cases in frege.c.g.j.VarCode

9caad72

Remove partial functions in frege.compiler.gen.java.Match

81e2594

by tightening the parameters

Change the return type of allourvars and allvars to [SymV Global]

56e7ac8

allvars was changed from monadic value to a plain function because it doesn't change nay state. Some functions in other modules had their parameters tightened.

Change fundep to take SymV

fb417c4

Change ccSym to take SymV

f0b8d1c

Change the type of Global.locals to TreeMap Int (SymV Global)

8bef123

Remove some partial functions in frege.compiler.passes.Transdef

7809062

Add instance Ord for SymT, SymL, etc.

daf9a49

Remove dead cases and partial functions from f.c.passes.LetUnroll

a316f7a

Remove some partial functions and dead cases in frege.compiler.Typecheck

2c7fad5

Remove partial functions in frege.compiler.GenMeta

81a68d2

Remove unused imports

d1cbfc0

matil019 added 28 commits October 30, 2019 17:41

Remove uses of unsafePartialView in Classes

61f2681

Instead of crashing, erroneous cases are simply treated as un-matched cases.

Remove an unused import in Final to avoid warnings

6a60400

Remove uses of partial matches in VarCode

c4c7736

Instead of crashing, erroneous cases are simply treated as un-matched cases.

Add a custom partial function in InstanceCode

3b9cefb

All SymMeths in SymC.meth should have been resolved to SymV in this phase (at least that's how I understand the code). An error here, if any, is an internal error, and is reported as a pure error to help find a bug in the compiler.

Add a custom partial function in Global

da7ebcf

Some functions in Global assumes certain propeties of Symbols. Those assumptions should be correct unless something goes very wrong. An error here, if any, is an internal error, and is reported as a pure error to help find a bug in the compiler.

Add an error message to the new error in SymbolTable

9effb28

Since only SymMeth (i.e. SymV or SymL) can be a member of SymC.meth or SymI.meth, if 'sym' is not SymMeth, it is an error. A new error message was added to report it instead of just crashing.

Add a copy of unsafePartialView to MethodCall

a9b0fa2

A part of MethodCall.nativeCall uses unsafePartialView a lot. The reason is that it was written by myself as a prototype and is not polished yet.

Remove uses of partial matches in doc.Utility

afca4ce

Instead of crashing, erroneous cases are simply treated as un-matched cases.

Remove uses of partial functions in tools.Doc

a42eca7

'allfuns' now contain SymVs only. If non SymVs are in an env of a Symbol, they are just now ignored. Some of the other functions had their types changed in order to reflect that change.

Remove uses of partial matches in ide.Utilities

e7908e5

Instead of crashing, erroneous cases are simply treated as un-matched cases.

Change SymbolTable.InsUpd to protected

e01d94c

being "private" crashes fregedoc (but not fregec).

Make constructors of SymbolT, SymMeth and SymVal strict

4b83084

optimizations from now on

Clean unused imports

e6f82dd

Particularly unsafePartialView (defined in f.c.common.Lens) is no longer used anywhere.

Replace 'view'ing lenses with simple getters

46e2e2f

Added simple getters to SymbolT to be used instead of 'view'ing a 'Lens'. In Haskell, 'view' can have significant overhead than simple pattern matching. The existing lenses were given a leading underscore, like the prisms.

Replace 'view'ing lenses with simple getters for SymMeth and SymVal

98d630e

The same refactoring as done to SymbolT. Now all uses of 'view's were replaced by simple getters. This contributes to a big reduction of performance penalty.

Change the type of allClasses to StG [SymC Global]

2709dab

A redundant pattern match was removed.

Remove Utilities.symVD

d39551c

Unused partial function The new type SymVal and its member fromSymbol serves the same purpose.

Modify Desugar.updVis not to throw an error

a501df7

On invalid data constructor, 'updVis' behaves as 'id' instead of an error, just like 'over' in lens does.

Change unAlias.aliases to return [SymA Global]

cfa58ac

Change isHigherKindedClass to take SymC

1ac6dad

Change DataCode.subDecls to take SymT

389b789

Add instance Ord for SymVal and SymMeth

49e0ca1

Remove redundant parameter from match.matchEnum

1952d83

Restrict parameter type of dataCode, classCode and instanceCode

d39d409

Change transDatDcl.newtCheck to take SymT

b2fec3f

Clean up the code

11bdb3f

Removed unwanted diffs introduced by a series of modifications

matil019 mentioned this pull request Nov 20, 2019

Split records of TauT and RhoT #388

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Split records of SymbolT and DefinitionS #384

Split records of SymbolT and DefinitionS #384

matil019 commented Oct 31, 2019

Ingo60 commented Nov 1, 2019

Split records of SymbolT and DefinitionS #384

Are you sure you want to change the base?

Split records of SymbolT and DefinitionS #384

Conversation

matil019 commented Oct 31, 2019

Main changes

Building blocks

SymbolT

DefinitionS

New errors

A genuine warning

Internal errors

Performance concerns

fregec

fregedoc

frege IDE

Notes

Ingo60 commented Nov 1, 2019