scribble icon indicating copy to clipboard operation
scribble copied to clipboard

Typechecking logic for `SId`s is overcomplicated

Open cd1m0 opened this issue 3 years ago • 0 comments

The typing logic for SId is split between several functions: tcId, tcIdVariable, tcLetAnnotation, tcUserConstant, lookupFun, lookupVarDef, lookupTypeDef, tcIdBuiltinSymbol, tcIdImportUnitRef. Whats worse, there are 2 redundant styles of checking types:

a) Using an explicit stack of scopes (see lookupVarDef for an example) b) A messy bunch of ifs all over the place. (see tcId for an example of this)

This logic is ugly, over-complicated and probably has a lot of buggy edge-cases with shadowing.

I think it would be cleaner if we had a single way to type-check SIds, just using a single explicit stack of scopes. In that case we would:

  1. Unify lookupVarDef, lookupFun, lookupTypeDef and tcLetAnnotationId into a single lookupDef function.
  2. Replace all the tc* functions with a single tcId function whose body will look almost exactly like the body of tcIdVariable
  3. We will add a new BuiltinsScope type for all the builtins (its not clear yet exactly where that will go).
  4. For every 2 types of DefSites we will need a test to determine how they get shadowed. For example for builtins and variables, we will need to see if a local variable called msg shadows the msg builtin. Similarly for variables and functions, we will need a test checking their precedence. And so on and so forth.

In practice, its probably better to first start with the tests in (4) and then begin implementing the code changes.

cd1m0 avatar Jul 06 '22 04:07 cd1m0