Skip to content

Commit

Permalink
add #ICInaccessibleSpecialCase memoize option
Browse files Browse the repository at this point in the history
Summary: Some DEIC special case memoize functions require access to the implicit context while not emitting implicit context key (accesses IC, not sharded by IC). Adding the memoize option to do that.

Reviewed By: jano

Differential Revision: D68542539

fbshipit-source-id: b501b5c4332b843a72e9f6517b83e89a9ecaca52
  • Loading branch information
Max Trivedi authored and facebook-github-bot committed Jan 25, 2025
1 parent c18bc98 commit 2e164dc
Show file tree
Hide file tree
Showing 15 changed files with 168 additions and 23 deletions.
7 changes: 5 additions & 2 deletions hphp/hack/src/hackc/emitter/emit_memoize_function.rs
Original file line number Diff line number Diff line change
Expand Up @@ -93,11 +93,14 @@ pub(crate) fn emit_wrapper_function<'a, 'd>(
.any(|tp| tp.reified.is_reified() || tp.reified.is_soft_reified());
let coeffects = Coeffects::from_ast(f.ctxs.as_ref(), &f.params, &fd.tparams, vec![]);
let should_emit_implicit_context = hhbc::is_keyed_by_ic_memoize(attributes.iter());
// #ICInaccessibleSpecialCase won't shard by IC but will access IC
let is_inaccessible_special_case = hhbc::is_inaccessible_special_case(attributes.iter());

// This fn either has IC unoptimizable static coeffects, or has any dynamic coeffects
let has_ic_unoptimizable_coeffects: bool = coeffects.has_ic_unoptimizable_coeffects();
let should_make_ic_inaccessible: bool =
!should_emit_implicit_context && has_ic_unoptimizable_coeffects;
let should_make_ic_inaccessible: bool = !is_inaccessible_special_case
&& !should_emit_implicit_context
&& has_ic_unoptimizable_coeffects;

let mut env = Env::default(Arc::clone(&fd.namespace)).with_scope(scope);
let (instrs, decl_vars) = make_memoize_function_code(
Expand Down
18 changes: 14 additions & 4 deletions hphp/hack/src/hackc/emitter/emit_memoize_method.rs
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,10 @@ fn make_memoize_wrapper_method<'a, 'd>(
Flags::SHOULD_EMIT_IMPLICIT_CONTEXT,
hhbc::is_keyed_by_ic_memoize(attributes.iter()),
);
arg_flags.set(
Flags::IS_IC_INACCESSIBLE_SPECIAL_CASE,
hhbc::is_inaccessible_special_case(attributes.iter()),
);
let mut args = Args {
info,
method,
Expand Down Expand Up @@ -302,6 +306,8 @@ fn make_memoize_method_with_params_code<'a, 'd>(
// so the first unnamed local is parameter count + 1 when there are reified generics.
let is_reified = args.flags.contains(Flags::IS_REIFIED);
let add_reified = usize::from(is_reified);
// #ICInaccessibleSpecialCase won't shard by IC but will access IC
let is_inaccessible_special_case = args.flags.contains(Flags::IS_IC_INACCESSIBLE_SPECIAL_CASE);
let should_emit_implicit_context = args.flags.contains(Flags::SHOULD_EMIT_IMPLICIT_CONTEXT);
let add_implicit_context = usize::from(should_emit_implicit_context);
let first_unnamed_idx = param_count + add_reified;
Expand Down Expand Up @@ -368,8 +374,9 @@ fn make_memoize_method_with_params_code<'a, 'd>(
let ic_stash_local = Local::new((key_count) as usize + first_unnamed_idx);
// This fn either has IC unoptimizable static coeffects, or has any dynamic coeffects
let has_ic_unoptimizable_coeffects: bool = coeffects.has_ic_unoptimizable_coeffects();
let should_make_ic_inaccessible: bool =
!should_emit_implicit_context && has_ic_unoptimizable_coeffects;
let should_make_ic_inaccessible: bool = !is_inaccessible_special_case
&& !should_emit_implicit_context
&& has_ic_unoptimizable_coeffects;
let instrs = InstrSeq::gather(vec![
begin_label,
emit_body::emit_method_prolog(emitter, env, pos, hhas_params, args.params, &[])?,
Expand Down Expand Up @@ -459,9 +466,12 @@ fn make_memoize_method_no_params_code<'a, 'd>(
},
None,
);
// #ICInaccessibleSpecialCase won't shard by IC but will access IC
let is_inaccessible_special_case = args.flags.contains(Flags::IS_IC_INACCESSIBLE_SPECIAL_CASE);
let ic_stash_local = Local::new(0);
// we are in a no parameter function that sets no zoned IC either, default to what coeffects suggest
let should_make_ic_inaccessible: bool = coeffects.has_ic_unoptimizable_coeffects();
let should_make_ic_inaccessible: bool =
coeffects.has_ic_unoptimizable_coeffects() && !is_inaccessible_special_case;
let instrs = InstrSeq::gather(vec![
deprecation_body,
if args.method.static_ {
Expand Down Expand Up @@ -583,6 +593,6 @@ bitflags! {
const IS_ASYNC = 1 << 4;
const SHOULD_EMIT_IMPLICIT_CONTEXT = 1 << 5;
const SHOULD_MAKE_IC_INACCESSIBLE = 1 << 6;
const SHOULD_SOFT_MAKE_IC_INACCESSIBLE = 1 << 7;
const IS_IC_INACCESSIBLE_SPECIAL_CASE = 1 << 7;
}
}
4 changes: 2 additions & 2 deletions hphp/hack/src/hackc/hhbc/attribute.rs
Original file line number Diff line number Diff line change
Expand Up @@ -86,8 +86,8 @@ pub fn is_keyed_by_ic_memoize(attrs: impl AsRef<[Attribute]>) -> bool {
is_memoize_with(attrs, "KeyedByIC")
}

pub fn is_make_ic_inaccessible_memoize(attrs: impl AsRef<[Attribute]>) -> bool {
is_memoize_with(attrs, "MakeICInaccessible")
pub fn is_inaccessible_special_case(attrs: impl AsRef<[Attribute]>) -> bool {
is_memoize_with(attrs, "ICInaccessibleSpecialCase")
}

fn is_foldable(attr: &Attribute) -> bool {
Expand Down
9 changes: 7 additions & 2 deletions hphp/hack/src/naming/naming_special_names.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
*/

/** Module consisting of the special names known to the typechecker */

pub mod classes {
pub const PARENT: &str = "parent";

Expand Down Expand Up @@ -387,7 +386,13 @@ pub mod memoize_option {

pub const MAKE_IC_INACCESSSIBLE: &str = "MakeICInaccessible";

pub static _ALL: &[&str] = &[KEYED_BY_IC, MAKE_IC_INACCESSSIBLE];
pub const IC_INACCESSSIBLE_SPECIAL_CASE: &str = "ICInaccessibleSpecialCase";

pub static _ALL: &[&str] = &[
KEYED_BY_IC,
MAKE_IC_INACCESSSIBLE,
IC_INACCESSSIBLE_SPECIAL_CASE,
];

lazy_static! {
static ref VALID_SET: HashSet<&'static str> = _ALL.iter().copied().collect();
Expand Down
13 changes: 11 additions & 2 deletions hphp/hack/src/parser/lowerer/lowerer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5654,9 +5654,12 @@ fn check_effect_memoized<'a>(
)
}
}
// #(Soft)?MakeICInaccessible can only be used on functions with defaults
if let Some(u) = user_attributes.iter().find(|u| {
is_memoize_attribute_with_flavor(u, Some(sn::memoize_option::MAKE_IC_INACCESSSIBLE))
|| is_memoize_attribute_with_flavor(
u,
Some(sn::memoize_option::IC_INACCESSSIBLE_SPECIAL_CASE),
)
}) {
if !has_any_context(
contexts,
Expand All @@ -5669,7 +5672,13 @@ fn check_effect_memoized<'a>(
raise_parsing_error_pos(
&u.name.0,
env,
&syntax_error::memoize_make_ic_inaccessible_without_defaults(kind),
&syntax_error::memoize_make_ic_inaccessible_without_defaults(
kind,
is_memoize_attribute_with_flavor(
u,
Some(sn::memoize_option::IC_INACCESSSIBLE_SPECIAL_CASE),
),
),
)
}
}
Expand Down
11 changes: 8 additions & 3 deletions hphp/hack/src/parser/syntax_error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1068,10 +1068,15 @@ pub fn policy_sharded_memoized_without_policied(kind: &str) -> Error {
))
}

pub fn memoize_make_ic_inaccessible_without_defaults(kind: &str) -> Error {
pub fn memoize_make_ic_inaccessible_without_defaults(kind: &str, is_special_case: bool) -> Error {
let context = if is_special_case {
"#ICInaccessibleSpecialCase"
} else {
"#MakeICInaccessible"
};
Cow::Owned(format!(
"This {} requires the defaults, leak_safe_shallow, or leak_safe_local context to be memoized using #MakeICInaccessible",
kind
"This {} requires the defaults, leak_safe_shallow, or leak_safe_local context to be memoized using {}",
kind, context
))
}

Expand Down
1 change: 1 addition & 0 deletions hphp/runtime/ext/hh/ext_implicit_context.php
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,7 @@ protected static function get()[this::CRun]: ?this::T {
enum class MemoizeOption: string {
string KeyedByIC = 'KeyedByIC';
string MakeICInaccessible = 'MakeICInaccessible';
string ICInaccessibleSpecialCase = 'ICInaccessibleSpecialCase';
}

} // namespace HH
Expand Down
6 changes: 4 additions & 2 deletions hphp/runtime/ext/reflection/ext_reflection.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -939,7 +939,8 @@ const StaticString
s_MemoizeLSB("__MemoizeLSB"),
s_systemlib_create_opaque_value("__SystemLib\\create_opaque_value"),
s_KeyedByIC("KeyedByIC"),
s_MakeICInaccessible("MakeICInaccessible");
s_MakeICInaccessible("MakeICInaccessible"),
s_ICInaccessibleSpecialCase("ICInaccessibleSpecialCase");

ALWAYS_INLINE
static Array get_function_user_attributes(const Func* func) {
Expand All @@ -961,7 +962,8 @@ static Array get_function_user_attributes(const Func* func) {
} else {
auto const sd = tv.m_data.pstr;
if (sd->same(s_KeyedByIC.get()) ||
sd->same(s_MakeICInaccessible.get())) {
sd->same(s_MakeICInaccessible.get()) ||
sd->same(s_ICInaccessibleSpecialCase.get())) {
if (Cfg::Eval::EmitNativeEnumClassLabels) {
args.append(make_tv<KindOfEnumClassLabel>(sd));
} else {
Expand Down
3 changes: 3 additions & 0 deletions hphp/runtime/vm/func-emitter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -209,6 +209,7 @@ const StaticString
s_MemoizeLSB("__MemoizeLSB"),
s_KeyedByIC("KeyedByIC"),
s_MakeICInaccessible("MakeICInaccessible"),
s_ICInaccessibleSpecialCase("ICInaccessibleSpecialCase"),
s_SoftInternal("__SoftInternal");

namespace {
Expand Down Expand Up @@ -303,6 +304,8 @@ Func* FuncEmitter::create(Unit& unit, PreClass* preClass /* = NULL */) const {
icType = Func::MemoizeICType::KeyedByIC;
} else if (elem.m_data.pstr->same(s_MakeICInaccessible.get())) {
icType = Func::MemoizeICType::MakeICInaccessible;
} else if (elem.m_data.pstr->same(s_ICInaccessibleSpecialCase.get())) {
icType = Func::MemoizeICType::ICInaccessibleSpecialCase;
} else {
assertx(false && "invalid string");
}
Expand Down
4 changes: 0 additions & 4 deletions hphp/runtime/vm/func-inl.h
Original file line number Diff line number Diff line change
Expand Up @@ -577,10 +577,6 @@ inline bool Func::isKeyedByImplicitContextMemoize() const {
return memoizeICType() == MemoizeICType::KeyedByIC;
}

inline bool Func::isMakeICInaccessibleMemoize() const {
return memoizeICType() == MemoizeICType::MakeICInaccessible;
}


inline bool Func::isMemoizeImpl() const {
return isMemoizeImplName(name());
Expand Down
3 changes: 1 addition & 2 deletions hphp/runtime/vm/func.h
Original file line number Diff line number Diff line change
Expand Up @@ -721,6 +721,7 @@ struct Func final {
NoIC = 0,
KeyedByIC = 1,
MakeICInaccessible = 2,
ICInaccessibleSpecialCase = 3,
};

MemoizeICType memoizeICType() const;
Expand All @@ -729,8 +730,6 @@ struct Func final {

bool isKeyedByImplicitContextMemoize() const;

bool isMakeICInaccessibleMemoize() const;

/*
* Is this string the name of a memoize implementation.
*/
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
<?hh

// we are matching the coeffect requiments for MakeICInaccessible, so leak_safe or less is no allowed
<<__Memoize(#ICInaccessibleSpecialCase)>>
function memo_inaccessible_sc_leaksafe($a, $b)[leak_safe]: mixed{
echo "memo_inaccessible_sc_leaksafe: $a, $b \n";
}

<<__EntryPoint>>
function main() {
memo_inaccessible_sc_leaksafe(1, 2);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@

Fatal error: This function requires the defaults, leak_safe_shallow, or leak_safe_local context to be memoized using #ICInaccessibleSpecialCase in %s/hphp/test/slow/implicit-context/ic-inaccessible-special-case-coeffects.php on line 4
92 changes: 92 additions & 0 deletions hphp/test/slow/implicit-context/ic-inaccessible-special-case.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,92 @@
<?hh

class Base implements HH\IMemoizeParam {
public function getInstanceKey()[]: string {
return 'KEY' . $this->name();
}
public function name()[]: string { return static::class; }
}

abstract final class ClassContext extends HH\ImplicitContext {
const type T = Base;
const bool IS_MEMO_SENSITIVE = true;
const ctx CRun = [defaults];
public static function start(Base $context, (function (): int) $f)[this::CRun, ctx $f] {
return parent::runWith($context, $f);
}
public static function getContext()[this::CRun]: Base {
return parent::get() as nonnull;
}
public static function exists()[this::CRun]: bool {
return parent::exists() as bool;
}
}

class A extends Base {}

class B extends Base {
<<__Memoize(#KeyedByIC)>>
public function memo_kbic($a, $b)[defaults]: mixed {
$context = ClassContext::getContext()->name();
echo "args: $a, $b name: $context\n";
}

<<__Memoize(#MakeICInaccessible)>>
public function memo_inaccessible($a, $b)[defaults]: mixed {
$context = ClassContext::getContext()->name();
echo "args: $a, $b name: $context\n";
}

<<__Memoize(#ICInaccessibleSpecialCase)>>
public function memo_inaccessible_sc($a, $b)[defaults]: mixed {
$context = ClassContext::getContext()->name();
echo "args: $a, $b name: $context\n";
}

}


<<__Memoize(#KeyedByIC)>>
function memo_kbic($a, $b)[defaults]: mixed{
$context = ClassContext::getContext()->name();
echo "args: $a, $b name: $context\n";
}

<<__Memoize(#MakeICInaccessible)>>
function memo_inaccessible($a, $b)[defaults]: mixed{
$context = ClassContext::getContext()->name();
echo "args: $a, $b name: $context\n";
}

<<__Memoize(#ICInaccessibleSpecialCase)>>
function memo_inaccessible_sc($a, $b)[defaults]: mixed{
$context = ClassContext::getContext()->name();
echo "args: $a, $b name: $context\n";
}

function f()[defaults]: mixed{
$klass_b = new B;
$tryout = function($memo_function, $a, $b) use ($klass_b) {
try {
$memo_function($a, $b);
} catch (Exception $e) {
echo "Function $memo_function throws: ".$e->getMessage() . "\n";
}

try {
$klass_b->$memo_function($a, $b);
} catch (Exception $e) {
echo "Method B->$memo_function throws: ".$e->getMessage() . "\n";

}
};
$tryout('memo_kbic', 1, 2);
$tryout('memo_inaccessible', 3, 4);
$tryout('memo_inaccessible_sc', 5, 6);
}


<<__EntryPoint>>
function main(): mixed{
ClassContext::start(new A, f<>);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
args: 1, 2 name: A
args: 1, 2 name: A
Function memo_inaccessible throws: Implicit context is set to inaccessible
Method B->memo_inaccessible throws: Implicit context is set to inaccessible
args: 5, 6 name: A
args: 5, 6 name: A

0 comments on commit 2e164dc

Please sign in to comment.