Skip to content

Commit

Permalink
Fix subtyping dict < shape being too lenient
Browse files Browse the repository at this point in the history
Summary:
The root cause of D56416941 is subtyping being too lenient for dynamic dicts to shapes, causing dynamic dicts to be "approximated away" when joining types.

This fixes it by considering `#{dynamic() => dynamic()}` to be a subtype of any shape only when the subtyping polarity of the dict is negative.

Also change `narrow.adjustMapType` to preserve dynamic maps, to avoid introducing too much noise.

Reviewed By: ilya-klyuchnikov

Differential Revision: D56416893

fbshipit-source-id: 88e3f68b9e88f6d8b61556a3edbbdad2933a4235
  • Loading branch information
VLanvin authored and facebook-github-bot committed Apr 22, 2024
1 parent 1dbdbbd commit f2556cd
Show file tree
Hide file tree
Showing 5 changed files with 32 additions and 67 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -475,6 +475,8 @@ class Narrow(pipelineContext: PipelineContext) {
BoundedDynamicType(adjustMapType(bound, keyT, valT))
case shapeMap: ShapeMap =>
adjustShapeMap(shapeMap, keyT, valT)
case DictMap(kT, vT) if subtype.isDynamicType(kT) && subtype.isDynamicType(vT) =>
DictMap(kT, vT)
case dictMap: DictMap =>
adjustDictMap(dictMap, keyT, valT)
case UnionType(elems) =>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -199,13 +199,12 @@ class Subtype(pipelineContext: PipelineContext) {
if (!subTypePol(t1, t2, seen)) return false
}
true
case (DictMap(kT, vT), ShapeMap(_)) if isDynamicType(kT) && isDynamicType(vT) =>
true
case (DictMap(kT, vT), ShapeMap(props)) =>
val (reqProps, optProps) = props.partition {
case ReqProp(_, _) => true
case OptProp(_, _) => false
}
if (isDynamicType(kT) && isDynamicType(vT) && v1 == -) return true
if (reqProps.nonEmpty) return false
val shapeDomain = join(optProps.map(prop => AtomLitType(prop.key)))
subTypePol(kT, shapeDomain, seen) && props.forall(prop => subTypePol(vT, prop.tp, seen))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ generic(_, _) -> error(stub). | OK |
(dyn_map(), #{a => atom()}) -> ok. | |
use_generic_shape(DM, S) -> | ERROR |
generic(DM, S). | | generic(DM, S).
| | Expression has type: #S{a => atom()}
| | Expression has type: dyn_map() | #S{a => atom()}
| | Context expected type: 'ok'
| |
-spec tuple_mismatch(dyn_map()) -> | |
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -369,11 +369,11 @@ maps(M) -> | ERROR |
| | Expression has type: 'b'
| | Context expected type: number()
M1 + M2. | | M1.
| | Expression has type: #D{dynamic() | 'id' => dynamic() | number()}
| | Expression has type: #D{dynamic() => dynamic()}
| | Context expected type: number()
| | ---
| | M2.
| | Expression has type: #D{dynamic() | 'id' => dynamic() | number()}
| | Expression has type: #D{dynamic() => dynamic()}
| | Context expected type: number()
| | ---
| | _ + _.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3623,7 +3623,7 @@
"end": 3252
},
"lineAndCol": null,
"message": "Expression has type: #D{dynamic() | 'id' => dynamic() | number()}\nContext expected type: number()",
"message": "Expression has type: #D{dynamic() => dynamic()}\nContext expected type: number()",
"uri": "https://fb.me/eqwalizer_errors#incompatible_types",
"code": "incompatible_types",
"expressionOrNull": "M1",
Expand Down Expand Up @@ -3652,39 +3652,21 @@
"got": {
"DictMap": {
"k_type": {
"UnionType": {
"tys": [
{
"RemoteType": {
"id": {
"module": "eqwalizer",
"name": "dynamic",
"arity": 0
}
}
},
{
"AtomLitType": {
"atom": "id"
}
}
]
"RemoteType": {
"id": {
"module": "eqwalizer",
"name": "dynamic",
"arity": 0
}
}
},
"v_type": {
"UnionType": {
"tys": [
{
"RemoteType": {
"id": {
"module": "eqwalizer",
"name": "dynamic",
"arity": 0
}
}
},
"NumberType"
]
"RemoteType": {
"id": {
"module": "eqwalizer",
"name": "dynamic",
"arity": 0
}
}
}
}
Expand Down Expand Up @@ -3778,7 +3760,7 @@
"end": 3257
},
"lineAndCol": null,
"message": "Expression has type: #D{dynamic() | 'id' => dynamic() | number()}\nContext expected type: number()",
"message": "Expression has type: #D{dynamic() => dynamic()}\nContext expected type: number()",
"uri": "https://fb.me/eqwalizer_errors#incompatible_types",
"code": "incompatible_types",
"expressionOrNull": "M2",
Expand Down Expand Up @@ -3807,39 +3789,21 @@
"got": {
"DictMap": {
"k_type": {
"UnionType": {
"tys": [
{
"RemoteType": {
"id": {
"module": "eqwalizer",
"name": "dynamic",
"arity": 0
}
}
},
{
"AtomLitType": {
"atom": "id"
}
}
]
"RemoteType": {
"id": {
"module": "eqwalizer",
"name": "dynamic",
"arity": 0
}
}
},
"v_type": {
"UnionType": {
"tys": [
{
"RemoteType": {
"id": {
"module": "eqwalizer",
"name": "dynamic",
"arity": 0
}
}
},
"NumberType"
]
"RemoteType": {
"id": {
"module": "eqwalizer",
"name": "dynamic",
"arity": 0
}
}
}
}
Expand Down

0 comments on commit f2556cd

Please sign in to comment.