Permalink
Browse files

Fix some TUninit vs TInitNull issues

Summary: I was auditing all uses of is_opt and unopt to see what needs to change when we allow TUninit to be mixed into any optional type, and found some cases where the analysis could be improved, and one bug.

Reviewed By: alexeyt

Differential Revision: D8661507

fbshipit-source-id: 5560155a723c9a048ad18fc3a55531c2bc78996b
  • Loading branch information...
markw65 authored and hhvm-bot committed Jun 28, 2018
1 parent 142f789 commit cf27f5de93633300927312a38d1475b65c9d1b41
@@ -1200,12 +1200,13 @@ void isTypeHelper(ISS& env,
return intersection_of(t, testTy);
};
auto const was_false = [&] (Type t) {
auto tinit = remove_uninit(t);
if (testTy.subtypeOf(TNull)) {
t = remove_uninit(std::move(t));
return is_opt(t) ? unopt(t) : t;
return is_opt(tinit) ? unopt(tinit) : tinit;
}
if (is_opt(t)) {
if (unopt(t).subtypeOf(testTy)) return TInitNull;
if (is_opt(tinit)) {
assertx(!testTy.couldBe(TNull));
if (unopt(tinit).subtypeOf(testTy)) return TNull;
}
return t;
};
@@ -1421,11 +1422,10 @@ void instanceOfJmpImpl(ISS& env,
if (locId == NoLocalId || interface_supports_non_objects(inst.str1)) {
return bail();
}
auto const val = peekLocation(env, locId, 1);
assertx(!val.couldBe(TRef)); // we shouldn't have an equivLoc if it was
auto const rcls = env.index.resolve_class(env.ctx, inst.str1);
if (!rcls) return bail();
auto const val = topC(env);
auto const instTy = subObj(*rcls);
if (val.subtypeOf(instTy) || !val.couldBe(instTy)) {
return bail();
@@ -1438,8 +1438,7 @@ void instanceOfJmpImpl(ISS& env,
popC(env);
auto const negate = jmp.op == Op::JmpNZ;
auto const result = [&] (Type t, bool pass) {
return pass ? instTy :
fail_implies_null ? (t.couldBe(TUninit) ? TNull : TInitNull) : t;
return pass ? instTy : fail_implies_null ? TNull : t;
};
auto const pre = [&] (Type t) { return result(t, negate); };
auto const post = [&] (Type t) { return result(t, !negate); };
@@ -1483,11 +1482,25 @@ void isTypeStructJmpImpl(ISS& env,
auto const negate = jmp.op == Op::JmpNZ;
auto const result = [&] (Type t, bool pass) {
if (!pass) {
if ((ts_type.value()).subtypeOf(TNull) && is_opt(t)) {
return unopt(std::move(t));
auto tinit = remove_uninit(t);
if (tinit.subtypeOf(*ts_type)) return TBottom;
if (t.couldBe(TNull)) {
auto tnonnull = is_opt(tinit) ? unopt(tinit) : tinit;
if (ts_type->couldBe(TNull)) {
return tnonnull;
}
if (tnonnull.subtypeOf(*ts_type)) {
return TNull;
}
}
return t;
}
if (t.couldBe(TUninit) && ts_type->couldBe(TNull)) {
return union_of(intersection_of(std::move(t),
std::move(ts_type.value())),
TUninit);
}
return intersection_of(std::move(t), std::move(ts_type.value()));
};
auto const pre = [&] (Type t) { return result(std::move(t), negate); };
@@ -2846,12 +2846,6 @@ folly::Optional<Type> type_of_type_structure(SArray ts) {
return is_nullable ? TOptVec : TVec;
case TypeStructure::Kind::T_keyset:
return is_nullable ? TOptKeyset : TKeyset;
case TypeStructure::Kind::T_vec_or_dict:
return is_nullable ? union_of(TOptVec, TOptDict) : union_of(TVec, TDict);
case TypeStructure::Kind::T_arraylike:
return is_nullable
? union_of(union_of(union_of(TOptArr, TOptVec), TOptDict), TOptKeyset)
: union_of(union_of(union_of(TArr, TVec), TDict), TKeyset);
case TypeStructure::Kind::T_void:
return TNull;
case TypeStructure::Kind::T_tuple: {
@@ -2886,6 +2880,22 @@ folly::Optional<Type> type_of_type_structure(SArray ts) {
auto const arrT = arr_map_darray(map);
return is_nullable ? union_of(std::move(arrT), TNull) : arrT;
}
case TypeStructure::Kind::T_vec_or_dict:
// Ideally, we would return this union; but thats not an allowed type, so
// we end up with TInitCell as the result, which makes hhbbc think that
// the condition is always true.
//
// return is_nullable ?
// union_of(TOptVec, TOptDict) : union_of(TVec, TDict);
return folly::none;
case TypeStructure::Kind::T_arraylike:
// Similar to the above, we can't (yet) do this.
//
// return is_nullable
// ? union_of(union_of(union_of(TOptArr, TOptVec), TOptDict), TOptKeyset)
// : union_of(union_of(union_of(TArr, TVec), TDict), TKeyset);
return folly::none;
case TypeStructure::Kind::T_noreturn:
case TypeStructure::Kind::T_mixed:
case TypeStructure::Kind::T_nonnull:
@@ -0,0 +1,7 @@
<?hh
function test() {
if (@$y is ?int) echo "Yes\n";
}
test();

0 comments on commit cf27f5d

Please sign in to comment.