Refactoring: Generic notion of CheckingPhase for the IR checkers. #5120
Conversation
|
@gzm0 I added a commit that switches to using the previous phase rather than the next phase. That means we're back to using Regarding your other comment from the previous PR:
I don't think that would be better. For one thing, we would need to make |
| * When checking IR (with `ClassDefChecker` or `IRChecker`), different nodes | ||
| * and transients are allowed between different phases. The `CheckingPhase` | ||
| * records the *previous* phase to run before the check. We are therefore | ||
| * checking that the IR is a valid *output* to the target phase. |
There was a problem hiding this comment.
| * checking that the IR is a valid *output* to the target phase. | |
| * checking that the IR is a valid *output* of the target phase. |
(but I'm not entirely sure).
| private val analyzer = | ||
| new Analyzer(config, initial = false, checkIR = checkIR, failOnError = true, irLoader) | ||
| private val analyzer = { | ||
| val checkIRFor = Some(CheckingPhase.Optimizer).filter(_ => checkIR) |
There was a problem hiding this comment.
| val checkIRFor = Some(CheckingPhase.Optimizer).filter(_ => checkIR) | |
| val checkIRFor = if (checkIR) Some(CheckingPhase.Optimizer) else None |
? even if a bit longer, IMO easier to grok.
| private val analyzer = | ||
| new Analyzer(config, initial = true, checkIR = checkIR, failOnError = true, irLoader) | ||
| private val analyzer = { | ||
| val checkIRFor = Some(CheckingPhase.Compiler).filter(_ => checkIR) |
There was a problem hiding this comment.
| val ch 8000 eckIRFor = Some(CheckingPhase.Compiler).filter(_ => checkIR) | |
| val checkIRFor = if (checkIR) Some(CheckingPhase.Compiler) else None |
? even if a bit longer, IMO easier to grok.
We previously had two boolean options for deciding what IR is allowed or not between phases. Of the four possible combinations of values, one was actually invalid. With the introduction of the desugarer, there would have been *three* such boolean options. Only 4 out of their 8 combinations would have been valid. We now introduce a generic concept of `CheckingPhase`. Some IR features are allowed as output of each `CheckingPhase`. Since the `ClassDefChecker` and `IRChecker` must agree on what features are valid when, we factor that logic into `FeatureSet`. It represents a set of logical features, and can compute which set is valid for each phase.
b341cc9 to
2a77c9c
Compare
Extracted from #5101.
I have not yet included the following comments:
However, I did add a second commit that removes the distinction between
Emitter(afterOptimizer = true/false). There's onlyEmitternow, and it always accepts optimized IR, even if it is actually run without the optimizer.