diff --git a/hphp/hack/src/hackc/emitter/emit_memoize_function.rs b/hphp/hack/src/hackc/emitter/emit_memoize_function.rs index d5fec30af1c5d7..83595d8e7aeea5 100644 --- a/hphp/hack/src/hackc/emitter/emit_memoize_function.rs +++ b/hphp/hack/src/hackc/emitter/emit_memoize_function.rs @@ -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( diff --git a/hphp/hack/src/hackc/emitter/emit_memoize_method.rs b/hphp/hack/src/hackc/emitter/emit_memoize_method.rs index fe7d3742ce679a..8d273fbf8814dd 100644 --- a/hphp/hack/src/hackc/emitter/emit_memoize_method.rs +++ b/hphp/hack/src/hackc/emitter/emit_memoize_method.rs @@ -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, @@ -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; @@ -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, &[])?, @@ -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_ { @@ -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; } } diff --git a/hphp/hack/src/hackc/hhbc/attribute.rs b/hphp/hack/src/hackc/hhbc/attribute.rs index 155358a8862a13..422919cab362e2 100644 --- a/hphp/hack/src/hackc/hhbc/attribute.rs +++ b/hphp/hack/src/hackc/hhbc/attribute.rs @@ -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 { diff --git a/hphp/hack/src/naming/naming_special_names.rs b/hphp/hack/src/naming/naming_special_names.rs index db6825f0bc9c7a..2c275b4fdbec70 100644 --- a/hphp/hack/src/naming/naming_special_names.rs +++ b/hphp/hack/src/naming/naming_special_names.rs @@ -8,7 +8,6 @@ */ /** Module consisting of the special names known to the typechecker */ - pub mod classes { pub const PARENT: &str = "parent"; @@ -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(); diff --git a/hphp/hack/src/parser/lowerer/lowerer.rs b/hphp/hack/src/parser/lowerer/lowerer.rs index 7c01217602450d..8b86a687440b3e 100644 --- a/hphp/hack/src/parser/lowerer/lowerer.rs +++ b/hphp/hack/src/parser/lowerer/lowerer.rs @@ -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, @@ -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), + ), + ), ) } } diff --git a/hphp/hack/src/parser/syntax_error.rs b/hphp/hack/src/parser/syntax_error.rs index ddd05985a3d961..800e4d0a42a742 100644 --- a/hphp/hack/src/parser/syntax_error.rs +++ b/hphp/hack/src/parser/syntax_error.rs @@ -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 )) } diff --git a/hphp/runtime/ext/hh/ext_implicit_context.php b/hphp/runtime/ext/hh/ext_implicit_context.php index 21d74d2147c31c..66c1893ec9779d 100644 --- a/hphp/runtime/ext/hh/ext_implicit_context.php +++ b/hphp/runtime/ext/hh/ext_implicit_context.php @@ -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 diff --git a/hphp/runtime/ext/reflection/ext_reflection.cpp b/hphp/runtime/ext/reflection/ext_reflection.cpp index f90768b7d7b062..1f9e7fb8c382db 100644 --- a/hphp/runtime/ext/reflection/ext_reflection.cpp +++ b/hphp/runtime/ext/reflection/ext_reflection.cpp @@ -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) { @@ -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(sd)); } else { diff --git a/hphp/runtime/vm/func-emitter.cpp b/hphp/runtime/vm/func-emitter.cpp index 523cd2ab4714e6..3ca2da30d60a16 100644 --- a/hphp/runtime/vm/func-emitter.cpp +++ b/hphp/runtime/vm/func-emitter.cpp @@ -209,6 +209,7 @@ const StaticString s_MemoizeLSB("__MemoizeLSB"), s_KeyedByIC("KeyedByIC"), s_MakeICInaccessible("MakeICInaccessible"), + s_ICInaccessibleSpecialCase("ICInaccessibleSpecialCase"), s_SoftInternal("__SoftInternal"); namespace { @@ -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"); } diff --git a/hphp/runtime/vm/func-inl.h b/hphp/runtime/vm/func-inl.h index 0a30470a209f08..176640e8f0739b 100644 --- a/hphp/runtime/vm/func-inl.h +++ b/hphp/runtime/vm/func-inl.h @@ -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()); diff --git a/hphp/runtime/vm/func.h b/hphp/runtime/vm/func.h index e264284287337f..552cb06eae06f5 100644 --- a/hphp/runtime/vm/func.h +++ b/hphp/runtime/vm/func.h @@ -721,6 +721,7 @@ struct Func final { NoIC = 0, KeyedByIC = 1, MakeICInaccessible = 2, + ICInaccessibleSpecialCase = 3, }; MemoizeICType memoizeICType() const; @@ -729,8 +730,6 @@ struct Func final { bool isKeyedByImplicitContextMemoize() const; - bool isMakeICInaccessibleMemoize() const; - /* * Is this string the name of a memoize implementation. */ diff --git a/hphp/test/slow/implicit-context/ic-inaccessible-special-case-coeffects.php b/hphp/test/slow/implicit-context/ic-inaccessible-special-case-coeffects.php new file mode 100644 index 00000000000000..529e9e36b12bf3 --- /dev/null +++ b/hphp/test/slow/implicit-context/ic-inaccessible-special-case-coeffects.php @@ -0,0 +1,12 @@ +> +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); +} diff --git a/hphp/test/slow/implicit-context/ic-inaccessible-special-case-coeffects.php.expectf b/hphp/test/slow/implicit-context/ic-inaccessible-special-case-coeffects.php.expectf new file mode 100644 index 00000000000000..3bd223399e84da --- /dev/null +++ b/hphp/test/slow/implicit-context/ic-inaccessible-special-case-coeffects.php.expectf @@ -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 diff --git a/hphp/test/slow/implicit-context/ic-inaccessible-special-case.php b/hphp/test/slow/implicit-context/ic-inaccessible-special-case.php new file mode 100644 index 00000000000000..9658860f0a88ec --- /dev/null +++ b/hphp/test/slow/implicit-context/ic-inaccessible-special-case.php @@ -0,0 +1,92 @@ +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<>); +} diff --git a/hphp/test/slow/implicit-context/ic-inaccessible-special-case.php.expect b/hphp/test/slow/implicit-context/ic-inaccessible-special-case.php.expect new file mode 100644 index 00000000000000..a5e700a45eb0b3 --- /dev/null +++ b/hphp/test/slow/implicit-context/ic-inaccessible-special-case.php.expect @@ -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