Fix #4098: Check that refinements have good bounds #4122
Conversation
| @@ -107,12 +115,21 @@ class CheckRealizable(implicit ctx: Context) { | |||
| * also lead to base types with bad bounds). | |||
There was a problem hiding this comment.
This comment should be updated.
There was a problem hiding this comment.
Actually, the comment is now correct. The previous implementation did less than the comment implied.
| private def refinedNames(tp: Type): Set[Name] = tp.dealias match { | ||
| case tp: RefinedType => refinedNames(tp.parent) + tp.refinedName | ||
| case tp: AndType => refinedNames(tp.tp1) ++ refinedNames(tp.tp2) | ||
| case tp: OrType => refinedNames(tp.tp1) ++ refinedNames(tp.tp2) |
There was a problem hiding this comment.
It seems that A | B would need a test? Confused what happens there with disjoint ranges.
There was a problem hiding this comment.
refinedNames only collects names that appear in a top-level refinement. So that's the same for & and |. The bounds checking is done later.
There was a problem hiding this comment.
I meant I don't see a testcase exercising this path.
|
I think I can sort-of follow what happens and I can't see anything else wrong with this, but maybe it's good to take a look together with you or somebody who understands the codebase better. |
So far, we did the check only for member symbols. We have to do the same thing for type refinements that do not refer to a member.
When checking whether non-member value refinements could cause unsoundness problems, I noted that the end position of a dynamic select got lost; the dynamic selection would have the same position as its qualifier. I.e. if `z.x` is a dynamic selection, the indicated position would be just `z`. Fixed now. The correct position can be seen when inspecting the error message of i4098.scala. Ideally, this would be a scripting test. I let someone else add one.
Analogously to what we do for members that have symbols.
|
Going to merge now to escape extra work that will be cause by the SBT upgrade. |
So far, we did the check only for member symbols. We have to do the same
for type refinements that do not refer to a member.