Fix of #50 - volatile #53
Conversation
Volatile checking needs to take all intersections into account; previously these could be discarded through needsChecking. Plus several refactorings and additions. 1) Module vals now have Final and Stable flags set 2) All logic around isVolatile is now in TypeOps; some of it was moved from Types. 3) Added stability checking to Select and SelectFromType typings. Todo: We should find a better name for isVolatile. Maybe define the negation instead under the name "isRealizable"?.
|
Review by @samuelgruetter @namin |
| * | ||
| * Lazy values are not allowed to have volatile type, as otherwise | ||
| * unsoundness can result. | ||
| */ |
There was a problem hiding this comment.
this (important) documentation is deleted, but not added anywhere
There was a problem hiding this comment.
Good catch. It should have been added to isVolatile in TypeOps, but that change got lost somehow.
+1 |
|
I just pulled in these changes and ran the example I gave in #50 and it's still accepted... |
|
I've started my own |
|
The example is in #50 is still accepted because isVolatile is not supposed to flag this one. is Volatile is an overapproximation of: The type is feasible now but it might become unfeasible by refining some of its abstract components. Feasibility of types has to be tested elsewhere. The right thing to do is to design and write a phase (what sued to be refChecks) that does this. |
|
That makes sense. I've just found out myself why we need to put this into refChecks ;-) It's because my isRealizable function (which tries to check everything) depends on the types of not yet typed trees (that's why isRealizable(NoType) was called...). |
|
LGTM [modulo the lost documentation for isVolatile ;-)] |
|
LGTM, though shouldn't we leave issue #50 open? |
Volatile checking needs to take all intersections into account; previously these
could be discarded through needsChecking.
Plus several refactorings and additions.
Todo: We should find a better name for isVolatile. Maybe define the negation instead under the name
"isRealizable"?.