Skip to content

Commit

Permalink
lub: Better reduce AndTypes created during type inference. (#4435)
Browse files Browse the repository at this point in the history
lub: Better reduce AndTypes created during type inference.

Adds the following support to `lub`:

* When lubbing an `AndType` and an `OrType`, call `lubDistributeOr` if our `AndType` lub attempt fails rather than making a new `OrType`.
* When lubbing an `AndType` and an `AndType`, check if their parts are (deeply) referentially equivalent.

There are tests that demonstrate the change.

Not handled in this PR is the following scenario:

* lubbing an `OrType` and an `OrType`

Attempts to handle this use-case ended up being too expensive to land, and changed the types that Sorbet inferred causing type errors on Stripe's codebase. I may pursue this change in a subsequent PR, as type inference still produces some `T.any` types with redundant members.

### Motivation
<!-- Why make this change? Describe the problem, not the solution. This can also be a link to an issue. -->

Type inference creates AndTypes which get nested into OrTypes. This process causes `lub` to get called in a few scenarios that, prior to this change, it was ill-equipped to deal with:

* An `AndType` and an `OrType`.
* An `AndType` and an `AndType` that are equivalent.

One particular file at Stripe spent a lot of time lubbing these types together, which resulted in monstrous types such as the following (note: `Boolean` != `T::Boolean` in this case; Stripe has a legacy module named `Boolean`):

```
T.any(T.all(Boolean, T.nilable(FalseClass)), T.all(Boolean, NilClass), T.all(Boolean, NilClass), T.all(Boolean, NilClass), T.all(Boolean, T.nilable(FalseClass)), FalseClass, T.all(Boolean, NilClass), TrueClass)
``` 

All of this time spent in lub added up to a long typechecking time. The changes in this PR halves the time it spends in typechecking.
  • Loading branch information
jvilk-stripe committed Aug 17, 2021
1 parent e08172c commit d245e20
Show file tree
Hide file tree
Showing 2 changed files with 52 additions and 0 deletions.
43 changes: 43 additions & 0 deletions core/types/subtyping.cc
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,37 @@ namespace sorbet::core {

using namespace std;

namespace {
bool compositeTypeDeepRefEqual(const OrType &o1, const OrType &o2);
bool compositeTypeDeepRefEqual(const AndType &a1, const AndType &a2);
bool compositeTypeDeepRefEqualHelper(const TypePtr &t1, const TypePtr &t2) {
if (t1 == t2) {
return true;
}
if (t1.tag() != t2.tag()) {
return false;
}
// t1 and t2 are the same kind of type.
if (isa_type<OrType>(t1)) {
return compositeTypeDeepRefEqual(cast_type_nonnull<OrType>(t1), cast_type_nonnull<OrType>(t2));
}
if (isa_type<AndType>(t1)) {
return compositeTypeDeepRefEqual(cast_type_nonnull<AndType>(t1), cast_type_nonnull<AndType>(t2));
}
return false;
}

// Returns 'true' if the tree of types stemming from this AndType are referentially equal.
bool compositeTypeDeepRefEqual(const AndType &a1, const AndType &a2) {
return compositeTypeDeepRefEqualHelper(a1.left, a2.left) && compositeTypeDeepRefEqualHelper(a1.right, a2.right);
}

// Returns 'true' if the tree of types stemming from this OrType are referentially equal.
bool compositeTypeDeepRefEqual(const OrType &o1, const OrType &o2) {
return compositeTypeDeepRefEqualHelper(o1.left, o2.left) && compositeTypeDeepRefEqualHelper(o1.right, o2.right);
}
} // namespace

TypePtr lubGround(const GlobalState &gs, const TypePtr &t1, const TypePtr &t2);

TypePtr Types::any(const GlobalState &gs, const TypePtr &t1, const TypePtr &t2) {
Expand Down Expand Up @@ -219,12 +250,24 @@ TypePtr Types::lub(const GlobalState &gs, const TypePtr &t1, const TypePtr &t2)
categoryCounterInc("lub", "or>");
return lubDistributeOr(gs, t2, t1);
} else if (auto *a2 = cast_type<AndType>(t2)) { // 2, 4
if (auto *a1 = cast_type<AndType>(t1)) {
// Check if the members of a1 and a2 are referentially equivalent. This helps simplify T.all types created
// during type inference.
if (compositeTypeDeepRefEqual(*a1, *a2)) {
categoryCounterInc("lub", "<and>");
return t2;
}
}

categoryCounterInc("lub", "and>");
auto t1d = underlying(gs, t1);
auto t2filtered = dropLubComponents(gs, t2, t1d);
if (t2filtered != t2) {
return lub(gs, t1, t2filtered);
}
if (isa_type<OrType>(t1)) {
return lubDistributeOr(gs, t1, t2);
}
return OrType::make_shared(t1, t2filtered);
} else if (isa_type<OrType>(t1)) {
categoryCounterInc("lub", "<or");
Expand Down
9 changes: 9 additions & 0 deletions test/testdata/infer/lub_and_glb_corner_cases.rb
Original file line number Diff line number Diff line change
Expand Up @@ -74,4 +74,13 @@ def generic_class_nilable_array_tuple_lub
[a, b].each(&:lazy)
# ^^^^^ error: Method `lazy` does not exist on `NilClass`
end

module Boolean
end
T4 = T.type_alias{T.any(T.all(Boolean, T.nilable(FalseClass)), T.all(Boolean, NilClass), T.all(Boolean, NilClass), T.all(Boolean, NilClass), T.all(Boolean, T.nilable(FalseClass)), FalseClass, T.all(Boolean, NilClass), TrueClass)}

def equivalent_and_types_lub
T.reveal_type(T4) # error: Revealed type: `<Type: T.any(T.all(Main::Boolean, T.nilable(FalseClass)), FalseClass, T.all(Main::Boolean, NilClass), TrueClass)>`
end

end

0 comments on commit d245e20

Please sign in to comment.