-
Notifications
You must be signed in to change notification settings - Fork 12.8k
Change doc-string for Unreliable variance flag. #33036
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Change doc-string for Unreliable variance flag. #33036
Conversation
@@ -4214,7 +4214,7 @@ namespace ts { | |||
Independent = 1 << 2, // Unwitnessed type parameter | |||
VarianceMask = Invariant | Covariant | Contravariant | Independent, // Mask containing all measured variances without the unmeasurable flag | |||
Unmeasurable = 1 << 3, // Variance result is unusable - relationship relies on structural comparisons which are not reflected in generic relationships | |||
Unreliable = 1 << 4, // Variance result is unreliable - relationship relies on structural comparisons which are not reflected in generic relationships | |||
Unreliable = 1 << 4, // Variance result is unreliable - checking may produce false negatives, but not false positives |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought it was the reverse: The variance check may cause a typecheck to pass (positive) where the structural check would fail (negative)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See #33020 and the associated issue for variance checks raising errors where a structural comparison would not.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, I'm just confused because that seemingly contradicts the conversion I had with @weswigham (linked in my post above).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In particular:
Unreliable
is supposed to be "fallback to structural if varianced-based check fails"
Which sounds like it shouldn't be able to cause an error only based on the variance result.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also (I'm done after this I promise):
Only Unreliable rather than Unmeasurable since it should be that in any case where the variance result is wrong, it's because the variance is overstrict because we have more relaxed arity-based rules on the signature than tuples have.
Implying that any false negatives will be discarded in favor of the structural result.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My reading:
The marker indicates that negative results are unreliable, therefore structural checks are used in the event of failure.
The positive results are assumed to be reliable such that false positives do not occur, even if in reality they can. There is no fallback in the event of success.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess when I hear "false positive" I think of, e.g. a virus scanner reporting a virus where none exists; likewise the unreliable
variance can produce a successful typecheck where the canonical one (structural) would fail. We thus seem to be seeing the "positive" and "negative" from opposite sides of the fence, hence the confusion 😄
Reverse polarity! ➕ <--> ➖
Minor documentation change that may be helpful to distinguish unreliable/unmeasurable.