Invalidate cached noop typechecks during typealias instantiation

[HT154]

This also fixes TypeAliasTypeNode always performing lazy type checks even when they should be eager.

Resolves #1710
Resolves #1716

[stackoverflow]

Nested unions still suffer from the same issue:

typealias Flat<A, B> = A | B
res_flat = new Dynamic {} is Flat<module, module>

typealias Nested<A, B> = (A | A) | (B | B)
res_nested = new Dynamic {} is Nested<module, module>

Results in

res_flat = false
res_nested = true

Children should be invalidated before parents. The truffle visitor does the opposite (goes from parent to children).

[bioball]

I don't think we need an invalidate() method. Instead, we can just call if (elementTypeNode.isNoopTypeCheck()) inside the execute methods of these nodes.

invalidate() suggests that a type node has stale state. But, the core problem is that we don't know yet if this is a noop check yet when inside the constructor of ListTypeNode, SetTypeNode, etc.

We can probably also change the isNoopTypeCheck method in class TypeVariableNode to this:

    @Override
    public boolean isNoopTypeCheck() {
      CompilerDirectives.transferToInterpreter();
      throw exceptionBuilder().bug("isNoopTypeCheck called on an unreplaced type variable").build();
    }

[HT154]

I don't think we need an invalidate() method. Instead, we can just call if (elementTypeNode.isNoopTypeCheck()) inside the execute methods of these nodes.

Are you suggesting removing the skip_TypeChecks fields from each class?

We can probably also change the isNoopTypeCheck method in class TypeVariableNode to this:

    @Override
    public boolean isNoopTypeCheck() {
      CompilerDirectives.transferToInterpreter();
      throw exceptionBuilder().bug("isNoopTypeCheck called on an unreplaced type variable").build();
    }

This particular change isn't safe since TypeVariableNode is not replaced in generic classes/methods.

[bioball]

This particular change isn't safe since TypeVariableNode is not replaced in generic classes/methods.

Ah, gotcha. Then, yeah, we should keep that impl as-is for now (it basically stands for Unknown).

Are you suggesting removing the skip_TypeChecks fields from each class?

Yup! Or, possibly, defer setting that field until execute gets called. But, it seems fine to just remove.

[stackoverflow]

Looks good!

[HT154]

An alternate approach here is having TypeVariableNode.isNoopTypeCheck() return false (only when it's in a type alias body?). This would mean slightly less efficient type checks when type vars are replaced with no-ops, but would preserve the optimization in other cases (like when no typealiases are used). Thoughts?

[HT154]

An alternate approach here is having TypeVariableNode.isNoopTypeCheck() return false only when it's in a type alias body.

#1722 implements this approach. I think this is probably preferable to this PR as it avoids losing the caching optimization in non-type-parameter scenarios.

[bioball]

LGTM! Merging this one; I'm not totally sold that #1722 semantically makes sense (TypeVariableNode is always meant to be replaced, I think)