Skip to content

Commit

Permalink
Update type constraint checks in VerifyRetNonNullC
Browse files Browse the repository at this point in the history
Summary:
VerifyRetNonNullC is emitted when the statically known return type passes all constraints modulo nullability.

This diff expands the type constraint checks in VerifyRetNonNullC to cover upper bounds. The logic to further simplify and infer the output type of this bytecode is udpated as follows:

1. If the static type can never be null, VerifyRetNonNullC is redundant and can be removed.
2. If the static type is null, then check:
    a. if there is any constraint that will fatal (i.e. non-nullable, non-soft), mark as unreachable .
    b. if all non-nullable constraints are soft, then the nullability of the type cannot be further refined
3. If the nullability of the type is not statically known, then we can assume that the output type is not null as long as all constraints are non-soft non-nullable.

Reviewed By: jano

Differential Revision: D70006138

fbshipit-source-id: a95fac508bb3223ff563c0c382941d87f3273033
  • Loading branch information
Niharika Devanathan authored and facebook-github-bot committed Mar 5, 2025
1 parent 544915a commit 1efcd2f
Show file tree
Hide file tree
Showing 4 changed files with 28 additions and 15 deletions.
27 changes: 21 additions & 6 deletions hphp/hhbbc/interp.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5395,15 +5395,30 @@ void in(ISS& env, const bc::VerifyRetTypeTS& /*op*/) {
}

void in(ISS& env, const bc::VerifyRetNonNullC&) {
auto const& constraint = env.ctx.func->retTypeConstraints.main();
if (constraint.isSoft()) return;

auto stackT = topC(env);
if (!stackT.couldBe(BInitNull)) return reduce(env);

auto const& constraints = env.ctx.func->retTypeConstraints;
if (stackT.subtypeOf(BInitNull)) {
popC(env);
push(env, TBottom);
return unreachable(env);
if (std::any_of(
constraints.range().begin(),
constraints.range().end(),
[&](const TypeConstraint& tc) {
return !tc.isSoft() && !tc.isNullable();
})) {
popC(env);
push(env, TBottom);
return unreachable(env);
}
return;
}

if (std::all_of(
constraints.range().begin(),
constraints.range().end(),
[](auto const& tc) { return tc.isNullable() || tc.isSoft();}
)) {
return;
}
popC(env);
push(env, unopt(std::move(stackT)));
Expand Down
11 changes: 7 additions & 4 deletions hphp/runtime/vm/bytecode.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4706,10 +4706,13 @@ OPTBLD_INLINE void iopVerifyRetTypeTS() {
}

OPTBLD_INLINE void iopVerifyRetNonNullC() {
const auto func = vmfp()->func();
const auto tc = func->returnTypeConstraints().main();
auto const ctx = tc.isThis() ? frameStaticClass(vmfp()) : nullptr;
tc.verifyReturnNonNull(vmStack().topC(), ctx, func);
auto const func = vmfp()->func();
auto const& constraints = func->returnTypeConstraints();
for (auto const& tc : constraints.range()) {
if (tc.isNullable()) continue;
auto const ctx = tc.isThis() ? frameStaticClass(vmfp()) : nullptr;
tc.verifyReturnNonNull(vmStack().topC(), ctx, func);
}
}

OPTBLD_INLINE JitResumeAddr iopNativeImpl(PC& pc) {
Expand Down
3 changes: 0 additions & 3 deletions hphp/runtime/vm/jit/irgen-types.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1673,9 +1673,6 @@ void emitVerifyRetTypeTS(IRGS& env) {
}

void emitVerifyRetNonNullC(IRGS& env) {
auto const func = curFunc(env);
auto const& tc = func->returnTypeConstraints().main();
always_assert(!tc.isNullable());
verifyRetTypeImpl(env, TypeConstraint::ReturnId, 0, true);
}

Expand Down
2 changes: 0 additions & 2 deletions hphp/runtime/vm/type-constraint.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1727,8 +1727,6 @@ void TypeConstraint::verifyStaticProperty(tv_lval val,
void TypeConstraint::verifyReturnNonNull(TypedValue* tv,
const Class* ctx,
const Func* func) const {
const auto DEBUG_ONLY tcs = func->returnTypeConstraints();
assertx(!tcs.isNullable());
if (UNLIKELY(tvIsNull(tv))) {
verifyReturnFail(tv, ctx, func);
} else if (debug) {
Expand Down

0 comments on commit 1efcd2f

Please sign in to comment.