Skip to content

Commit

Permalink
Fold nameof string into QueryM in array key pos
Browse files Browse the repository at this point in the history
Summary:
There's special logic in HackC to fold array index keys into the QueryM op

```
// $c["C"]
BaseL $c Warn Any
QueryM 0 CGet ET:"C" Any

// $c[C::class]
RaiseClassStringConversionNotice
BaseL $c Warn Any
QueryM 0 CGet ET:"C" Any
```
but for `nameof` we were falling back to the generic expression path.

```
// $c[nameof C];
String "C"
BaseL $c Warn Any
QueryM 1 CGet EC:0 Any
```

This factors out the special `MemberKey::ET` logic for `::class` expressions and makes `nameof` use it + emit the empty instruction sequence (like ::class) so it doesn't push a string onto the stack.

Updated the HackIR printer to disambiguate indices that are cells of immediates vs. folded literals.

Differential Revision: D63041705

fbshipit-source-id: c164c15bd5b387b4285edd66146592183a113611
  • Loading branch information
vassilmladenov authored and facebook-github-bot committed Sep 20, 2024
1 parent 61d8ffc commit b6fc704
Show file tree
Hide file tree
Showing 5 changed files with 132 additions and 25 deletions.
56 changes: 33 additions & 23 deletions hphp/hack/src/hackc/emitter/emit_expression.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4288,17 +4288,19 @@ fn emit_array_get_<'a, 'd>(
}
}

fn class_id_is_class_expr_id(e: &Emitter<'_>, env: &Env<'_>, cname: &ast::ClassId) -> bool {
let expr = ClassExpr::class_id_to_class_expr(e, &env.scope, false, false, cname);
matches!(expr, ClassExpr::Id(_))
}

// TODO(199608418) kill this function when these are banned
fn is_special_class_constant_accessed_with_class_id(
e: &Emitter<'_>,
env: &Env<'_>,
cname: &ast::ClassId,
id: &str,
) -> bool {
if !string_utils::is_class(id) {
return false;
}
let expr = ClassExpr::class_id_to_class_expr(e, &env.scope, false, false, cname);
matches!(expr, ClassExpr::Id(_))
string_utils::is_class(id) && class_id_is_class_expr_id(e, env, cname)
}

fn emit_elem<'a, 'd>(
Expand Down Expand Up @@ -4327,6 +4329,7 @@ fn emit_elem<'a, 'd>(
(instr::empty(), 0)
}
}
ast::Expr_::Nameof(x) if class_id_is_class_expr_id(e, env, x) => (instr::empty(), 0),
ast::Expr_::ClassConst(x)
if is_special_class_constant_accessed_with_class_id(e, env, &(x.0), &(x.1).1) =>
{
Expand All @@ -4347,6 +4350,24 @@ fn get_elem_member_key<'a, 'd>(
use ast::ClassId_ as CI_;
use ast::Expr;
use ast::Expr_;
let class_name_key = |cid: &ast::ClassId| {
let cname = match (&cid.2, env.scope.get_class()) {
(CI_::CIself, Some(cd)) => string_utils::strip_global_ns(cd.get_name_str()),
(CI_::CIexpr(Expr(_, _, Expr_::Id(id))), _) => string_utils::strip_global_ns(&id.1),
(CI_::CI(id), _) => string_utils::strip_global_ns(&id.1),
_ => {
return Err(Error::unrecoverable(
"Unreachable due to class_id_is_class_expr_id",
));
}
};

let fq_id = ClassName::from_ast_name_and_mangle(cname);
Ok(MemberKey::ET(
hhbc::intern_bytes(fq_id.as_bytes()),
ReadonlyOp::Any,
))
};
match elem {
// ELement missing (so it's array append)
None => Ok((MemberKey::W, instr::empty())),
Expand Down Expand Up @@ -4378,27 +4399,16 @@ fn get_elem_member_key<'a, 'd>(
MemberKey::ET(hhbc::intern_bytes(s.as_slice()), ReadonlyOp::Any),
instr::empty(),
)),
// Special case for class name
// Special cases for class name
Expr_::Nameof(x) if class_id_is_class_expr_id(e, env, x) => {
let key = class_name_key(x)?;
Ok((key, instr::empty()))
}
Expr_::ClassConst(x)
if is_special_class_constant_accessed_with_class_id(e, env, &(x.0), &(x.1).1) =>
{
let cname = match (&(x.0).2, env.scope.get_class()) {
(CI_::CIself, Some(cd)) => string_utils::strip_global_ns(cd.get_name_str()),
(CI_::CIexpr(Expr(_, _, Expr_::Id(id))), _) => {
string_utils::strip_global_ns(&id.1)
}
(CI_::CI(id), _) => string_utils::strip_global_ns(&id.1),
_ => {
return Err(Error::unrecoverable(
"Unreachable due to is_special_class_constant_accessed_with_class_id",
));
}
};
let fq_id = ClassName::from_ast_name_and_mangle(cname);
Ok((
MemberKey::ET(hhbc::intern_bytes(fq_id.as_bytes()), ReadonlyOp::Any),
instr::raise_class_string_conversion_notice(),
))
let key = class_name_key(&x.0)?;
Ok((key, instr::raise_class_string_conversion_notice()))
}
_ => {
// General case
Expand Down
7 changes: 6 additions & 1 deletion hphp/hack/src/hackc/ir/assemble/func.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1106,7 +1106,12 @@ impl FunctionParser<'_> {
let i = parse_i64(tokenizer)?;
(OpKind::I(i), tloc)
} else {
let vid = self.vid(tokenizer)?;
let vid = if ident == "cell" {
parse!(tokenizer, "cell" "(" <vid:self.vid> ")");
vid
} else {
self.vid(tokenizer)?
};
operands.push(vid);
(OpKind::C, tloc)
}
Expand Down
7 changes: 6 additions & 1 deletion hphp/hack/src/hackc/ir/print/print.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1701,7 +1701,12 @@ fn print_member_key(
match *key {
MemberKey::EC => {
let vid = operands.next().unwrap();
write!(w, "{}]", FmtVid(func, vid, verbose))?;
match vid.full() {
FullInstrId::Imm(_) => write!(w, "cell({})]", FmtVid(func, vid, verbose))?,
FullInstrId::Instr(_) | FullInstrId::None => {
write!(w, "{}]", FmtVid(func, vid, verbose))?
}
}
}
MemberKey::EI(i) => write!(w, "{}]", i)?,
MemberKey::EL => {
Expand Down
60 changes: 60 additions & 0 deletions hphp/test/slow/nameof/arraykey_fold.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
<?hh

namespace N {
function f(): void {
$d = dict["N\\C" => "N\\f -- N\\C", "C" => "N\\f -- C"];

\var_dump($d["N\\C"]);
\var_dump($d[C::class]);
\var_dump($d[nameof C]);
\var_dump($d[\N\C::class]);
\var_dump($d[nameof \N\C]);

\var_dump($d["C"]);
\var_dump($d[\C::class]);
\var_dump($d[nameof \C]);
}
}
namespace {
class B {
public static function foo(): void {
$d = dict["B" => "foo -- B", "C" => "foo -- C"];

var_dump($d[self::class]);
var_dump($d[nameof self]);
var_dump($d[static::class]); // not folded
var_dump($d[nameof static]); // not folded
}
}
class C extends B {
public static function bar(): void {
$d = dict["B" => "bar -- B", "C" => "bar -- C"];

var_dump($d[self::class]);
var_dump($d[nameof self]);
var_dump($d[parent::class]); // not folded
var_dump($d[nameof parent]); // not folded
}
}

function f(): void {
$c = dict["C" => "f -- C"];

var_dump($c["C"]);
var_dump($c[C::class]);
var_dump($c[nameof C]);
}

<<__EntryPoint>>
function main(): void {
f();
echo "--------------------\n";
N\f();
echo "--------------------\n";
B::foo();
echo "--------------------\n";
C::foo();
echo "--------------------\n";
C::bar();
}
}
27 changes: 27 additions & 0 deletions hphp/test/slow/nameof/arraykey_fold.php.expect
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
string(6) "f -- C"
string(6) "f -- C"
string(6) "f -- C"
--------------------
string(10) "N\f -- N\C"
string(10) "N\f -- N\C"
string(10) "N\f -- N\C"
string(10) "N\f -- N\C"
string(10) "N\f -- N\C"
string(8) "N\f -- C"
string(8) "N\f -- C"
string(8) "N\f -- C"
--------------------
string(8) "foo -- B"
string(8) "foo -- B"
string(8) "foo -- B"
string(8) "foo -- B"
--------------------
string(8) "foo -- B"
string(8) "foo -- B"
string(8) "foo -- C"
string(8) "foo -- C"
--------------------
string(8) "bar -- C"
string(8) "bar -- C"
string(8) "bar -- B"
string(8) "bar -- B"

0 comments on commit b6fc704

Please sign in to comment.