Skip to content

compiler: re-engineer the treatment of rebindable if

Alp Mestanogullari requested to merge alp/ghc:wip/alp/rebindable into master

The commit message has a pretty good summary:

Executing on the plan described in #17582, this patch changes the way if expressions are handled in the compiler in the presence of rebindable syntax. We get rid of the SyntaxExpr field of HsIf and instead, when rebindable syntax is on, we rewrite the HsIf node to the appropriate sequence of applications of the local ifThenElse function.

In order to be able to report good error messages, with expressions as they were written by the user (and not as desugared by the renamer), we make use of TTG extensions to extend GhcRn expression ASTs with an HsExpansion construct, which keeps track of a source (GhcPs) expression and the desugared (GhcRn) expression that it gives rise to. This way, we can typecheck the latter while reporting the former in error messages.

In order to discard the error context lines that arise from typechecking the desugared expressions (because they talk about expressions that the user has not written), we turn ErrCtx into more than a synonym and add a special "expansion context" construct. When such a context is pushed, only other expansion contexts can be added on top. Since only HsExpansion constructs give rise to such contexts, it is enough to restrict subsequent context lines to only mention the original expressions.


The current state however isn't perfect (hence the WIP in the title). The reference output for the rebindable11 test that I am adding contains the error messages produced by 8.6.5, and my branch fails to give the same error message on the last example, with nested ifs (a4 below).

ifThenElse :: Bool -> () -> () -> Int
ifThenElse cond b1 b2 = 0
...
a4 = if (if 'a' then () else ()) == 10 then () else ()

The expected error message:

rebindable11.hs:12:13: error:
    • Couldn't match expected type ‘Bool’
                  with actual type ‘GHC.Types.Char’
    • In the expression: 'a'
      In the first argument of ‘(==)’, namely ‘(if 'a' then () else ())’
      In the expression: (if 'a' then () else ()) == 10

The diff:

$ diff <(ghc -c testsuite/tests/rebindable/rebindable11.hs |& cat) <(_build/stage1/bin/ghc -c testsuite/tests/rebindable/rebindable11.hs |& cat)
37d36
<       In the first argument of ‘(==)’, namely ‘(if 'a' then () else ())
38a38,39
>       In the expression:
>         if (if 'a' then () else ()) == 10 then () else ()

Once this is figured out, I'll implement a similar change for HsCmdIf and add more expressions to the test.

Edited by Alp Mestanogullari

Merge request reports