Skip to content

Commit 2727a3f

Browse files
Niharika Devanathanfacebook-github-bot
authored andcommitted
Back out "Revert D76614862 D75026944 D75498477 D75470326" + fix
Summary: Original commit changeset: 64ffb830f49c Original Phabricator Diff: D76664263 S531313 was caused due to an error that was introduced in emit_memoize_method.rs in the original diff (D75026944): {F1979284836} The tparams variable, which is used to identify the generics in the function's scope, was mistakenly updated to include only the generics defined on the function. New changes on top of the backout of the revert: 1. hphp/hack/src/hackc/emitter/emit_memoize_function.rs 2. hphp/test/quick/memoize_generic.php + expect Reviewed By: jano Differential Revision: D76681690 fbshipit-source-id: a26546e8342721d69e5dc4d97e74c6819f264483
1 parent 6818586 commit 2727a3f

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

60 files changed

+665
-222
lines changed

hphp/hack/src/hackc/assemble/assemble.rs

Lines changed: 40 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ use hhbc::ModuleName;
2626
use hhbc::ParamEntry;
2727
use hhbc::StringId;
2828
use hhbc::StringIdMap;
29+
use hhbc::TParamInfo;
2930
use hhbc::TypedValue;
3031
use hhvm_types_ffi::Attr;
3132
use log::trace;
@@ -321,6 +322,7 @@ fn assemble_typedef(token_iter: &mut Lexer<'_>, case_type: bool) -> Result<hhbc:
321322

322323
fn assemble_class(token_iter: &mut Lexer<'_>, adata: &AdataMap) -> Result<hhbc::Class> {
323324
parse!(token_iter, ".class"
325+
<tparams:assemble_tparams()>
324326
<upper_bounds:assemble_upper_bounds()>
325327
<attr:assemble_special_and_user_attrs()>
326328
<name:assemble_class_name()>
@@ -372,6 +374,7 @@ fn assemble_class(token_iter: &mut Lexer<'_>, adata: &AdataMap) -> Result<hhbc::
372374
ctx_constants: ctx_constants.into(),
373375
requirements: requirements.into(),
374376
upper_bounds: upper_bounds.into(),
377+
tparams: tparams.into(),
375378
doc_comment,
376379
flags,
377380
};
@@ -477,7 +480,7 @@ fn assemble_const_or_type_const(
477480
fn assemble_method(token_iter: &mut Lexer<'_>, adata: &AdataMap) -> Result<hhbc::Method> {
478481
let method_tok = token_iter.peek().copied();
479482
token_iter.expect_str(Token::is_decl, ".method")?;
480-
let shadowed_tparams = assemble_shadowed_tparams(token_iter)?;
483+
let tparam_info = assemble_tparam_info(token_iter)?;
481484
let upper_bounds = assemble_upper_bounds(token_iter)?;
482485
let (attrs, attributes) = assemble_special_and_user_attrs(token_iter)?;
483486
let span = assemble_span(token_iter)?;
@@ -494,7 +497,7 @@ fn assemble_method(token_iter: &mut Lexer<'_>, adata: &AdataMap) -> Result<hhbc:
494497
attrs,
495498
params,
496499
return_type,
497-
shadowed_tparams,
500+
tparam_info,
498501
upper_bounds,
499502
span,
500503
)?;
@@ -510,19 +513,28 @@ fn assemble_method(token_iter: &mut Lexer<'_>, adata: &AdataMap) -> Result<hhbc:
510513
})
511514
}
512515

513-
fn assemble_shadowed_tparams(token_iter: &mut Lexer<'_>) -> Result<Vec<ClassName>> {
514-
token_iter.expect(Token::is_open_curly)?;
515-
let mut stp = Vec::new();
516-
while token_iter.peek_is(Token::is_identifier) {
517-
stp.push(ClassName::intern(
518-
token_iter.expect(Token::is_identifier)?.as_str()?,
519-
));
520-
if !token_iter.peek_is(Token::is_close_curly) {
521-
token_iter.expect(Token::is_comma)?;
522-
}
523-
}
524-
token_iter.expect(Token::is_close_curly)?;
525-
Ok(stp)
516+
fn parse_bool(token_iter: &mut Lexer<'_>) -> Result<bool> {
517+
let tok = token_iter.expect(Token::is_identifier)?;
518+
let id = tok.as_str()?;
519+
let b = id.parse()?;
520+
Ok(b)
521+
}
522+
523+
fn assemble_single_tparam_info(token_iter: &mut Lexer<'_>) -> Result<TParamInfo> {
524+
parse!(token_iter, "[" <name:assemble_class_name()> <shadows_class_tparam:parse_bool> "]");
525+
Ok(TParamInfo {
526+
name,
527+
shadows_class_tparam,
528+
})
529+
}
530+
531+
fn assemble_tparam_info(token_iter: &mut Lexer<'_>) -> Result<Vec<TParamInfo>> {
532+
Ok(if token_iter.peek_is(Token::is_lt) {
533+
parse!(token_iter, "<" <tparams:assemble_single_tparam_info(),*> ">");
534+
tparams
535+
} else {
536+
Vec::new()
537+
})
526538
}
527539

528540
fn assemble_method_flags(token_iter: &mut Lexer<'_>) -> Result<hhbc::MethodFlags> {
@@ -1025,6 +1037,7 @@ where
10251037
/// .function {upper bounds} [special_and_user_attrs] (span) <type_info> name (params) flags? {body}
10261038
fn assemble_function(token_iter: &mut Lexer<'_>, adata: &AdataMap) -> Result<hhbc::Function> {
10271039
token_iter.expect_str(Token::is_decl, ".function")?;
1040+
let tparam_info = assemble_tparam_info(token_iter)?;
10281041
let upper_bounds = assemble_upper_bounds(token_iter)?;
10291042
// Special and user attrs may or may not be specified. If not specified, no [] printed
10301043
let (attrs, attributes) = assemble_special_and_user_attrs(token_iter)?;
@@ -1040,7 +1053,6 @@ fn assemble_function(token_iter: &mut Lexer<'_>, adata: &AdataMap) -> Result<hhb
10401053
let mut decl_map = DeclMap::default();
10411054
let params = assemble_params(token_iter, &mut decl_map)?;
10421055
let flags = assemble_function_flags(name, token_iter)?;
1043-
let shadowed_tparams = Default::default();
10441056
let body = assemble_body(
10451057
token_iter,
10461058
&mut decl_map,
@@ -1049,7 +1061,7 @@ fn assemble_function(token_iter: &mut Lexer<'_>, adata: &AdataMap) -> Result<hhb
10491061
attrs,
10501062
params,
10511063
return_type,
1052-
shadowed_tparams,
1064+
tparam_info,
10531065
upper_bounds,
10541066
span,
10551067
)?;
@@ -1105,6 +1117,15 @@ fn assemble_upper_bounds(token_iter: &mut Lexer<'_>) -> Result<Vec<hhbc::UpperBo
11051117
Ok(ubs)
11061118
}
11071119

1120+
fn assemble_tparams(token_iter: &mut Lexer<'_>) -> Result<Vec<ClassName>> {
1121+
Ok(if token_iter.peek_is(Token::is_lt) {
1122+
parse!(token_iter, "<" <tparams:assemble_class_name(),*> ">");
1123+
tparams
1124+
} else {
1125+
Vec::new()
1126+
})
1127+
}
1128+
11081129
/// Ex: (T as <"HH\\int" "HH\\int" upper_bound>)
11091130
fn assemble_upper_bound(token_iter: &mut Lexer<'_>) -> Result<hhbc::UpperBound> {
11101131
parse!(token_iter, "(" <id:id> "as" <tis:assemble_type_info(TypeInfoKind::NotEnumOrTypeDef),*> ")");
@@ -1376,7 +1397,7 @@ fn assemble_body(
13761397
attrs: hhbc::Attr,
13771398
params: Vec<ParamEntry>,
13781399
return_type: Maybe<hhbc::TypeInfo>,
1379-
shadowed_tparams: Vec<ClassName>,
1400+
tparam_info: Vec<TParamInfo>,
13801401
upper_bounds: Vec<hhbc::UpperBound>,
13811402
span: hhbc::Span,
13821403
) -> Result<hhbc::Body> {
@@ -1438,7 +1459,7 @@ fn assemble_body(
14381459
is_memoize_wrapper_lsb,
14391460
doc_comment,
14401461
return_type,
1441-
shadowed_tparams: shadowed_tparams.into(),
1462+
tparam_info: tparam_info.into(),
14421463
upper_bounds: upper_bounds.into(),
14431464
span,
14441465
repr: hhbc::BcRepr {

hphp/hack/src/hackc/bytecode_printer/print.rs

Lines changed: 26 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@ use hhbc::Span;
4646
use hhbc::SrcLoc;
4747
use hhbc::StringId;
4848
use hhbc::SymbolRefs;
49+
use hhbc::TParamInfo;
4950
use hhbc::TraitReqKind;
5051
use hhbc::TypeConstant;
5152
use hhbc::TypeInfo;
@@ -329,6 +330,7 @@ fn print_fun_def(
329330
let body = &fun_def.body;
330331
newline(w)?;
331332
w.write_all(b".function ")?;
333+
print_tparam_info(w, &body.tparam_info)?;
332334
print_upper_bounds_(w, &body.upper_bounds)?;
333335
w.write_all(b" ")?;
334336
print_special_and_user_attrs(
@@ -524,11 +526,19 @@ fn print_enum_includes(w: &mut dyn Write, enum_includes: &[ClassName]) -> Result
524526
write_bytes!(w, " enum_includes ({})", fmt_separated(" ", enum_includes))
525527
}
526528

527-
fn print_shadowed_tparams(w: &mut dyn Write, shadowed_tparams: &[ClassName]) -> Result<()> {
529+
fn print_tparam_info(w: &mut dyn Write, tparam_info: &[TParamInfo]) -> Result<()> {
530+
if tparam_info.is_empty() {
531+
return Ok(());
532+
}
528533
write_bytes!(
529534
w,
530-
"{{{}}}",
531-
fmt_separated(", ", shadowed_tparams.iter().map(|s| s.as_str()))
535+
"<{}> ",
536+
fmt_separated(
537+
",",
538+
tparam_info
539+
.iter()
540+
.map(|t| { format!("[{} {}]", t.name.as_str(), t.shadows_class_tparam) })
541+
)
532542
)
533543
}
534544

@@ -541,7 +551,7 @@ fn print_method_def(
541551
let body = &method_def.body;
542552
newline(w)?;
543553
w.write_all(b" .method ")?;
544-
print_shadowed_tparams(w, &body.shadowed_tparams)?;
554+
print_tparam_info(w, &body.tparam_info)?;
545555
print_upper_bounds_(w, &body.upper_bounds)?;
546556
w.write_all(b" ")?;
547557
print_special_and_user_attrs(
@@ -582,6 +592,17 @@ fn print_method_def(
582592
})
583593
}
584594

595+
fn print_tparams(w: &mut dyn Write, tparams: &[ClassName]) -> Result<()> {
596+
if tparams.is_empty() {
597+
return Ok(());
598+
}
599+
write_bytes!(
600+
w,
601+
"<{}> ",
602+
fmt_separated(",", tparams.iter().map(|s| s.as_str()))
603+
)
604+
}
605+
585606
fn print_class_def(
586607
ctx: &Context<'_>,
587608
w: &mut dyn Write,
@@ -590,6 +611,7 @@ fn print_class_def(
590611
) -> Result<()> {
591612
newline(w)?;
592613
w.write_all(b".class ")?;
614+
print_tparams(w, class_def.tparams.as_ref())?;
593615
print_upper_bounds(w, class_def.upper_bounds.as_ref())?;
594616
w.write_all(b" ")?;
595617
print_special_and_user_attrs(

hphp/hack/src/hackc/emitter/emit_body.rs

Lines changed: 18 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ use hhbc::Param;
2929
use hhbc::ParamEntry;
3030
use hhbc::Span;
3131
use hhbc::StringId;
32+
use hhbc::TParamInfo;
3233
use hhbc::TypeInfo;
3334
use hhbc::TypedValue;
3435
use hhbc::UpperBound;
@@ -115,7 +116,7 @@ pub fn emit_body<'b, 'd>(
115116
args.class_tparam_names,
116117
args.flags.contains(Flags::SKIP_AWAITABLE),
117118
);
118-
let shadowed_tparams = emit_shadowed_tparams(args.immediate_tparams, args.class_tparam_names);
119+
let tparam_info = emit_tparam_info(args.immediate_tparams, args.class_tparam_names);
119120
let decl_vars = make_decl_vars(
120121
emitter,
121122
&scope,
@@ -178,7 +179,7 @@ pub fn emit_body<'b, 'd>(
178179
false, // is_memoize_wrapper
179180
false, // is_memoize_wrapper_lsb
180181
upper_bounds,
181-
shadowed_tparams,
182+
tparam_info,
182183
attributes,
183184
attrs,
184185
coeffects,
@@ -388,7 +389,7 @@ pub fn make_body<'a, 'd>(
388389
is_memoize_wrapper: bool,
389390
is_memoize_wrapper_lsb: bool,
390391
upper_bounds: Vec<UpperBound>,
391-
shadowed_tparams: Vec<String>,
392+
tparam_info: Vec<TParamInfo>,
392393
attributes: Vec<Attribute>,
393394
attrs: Attr,
394395
coeffects: Coeffects,
@@ -453,10 +454,7 @@ pub fn make_body<'a, 'd>(
453454
is_memoize_wrapper,
454455
is_memoize_wrapper_lsb,
455456
upper_bounds: upper_bounds.into(),
456-
shadowed_tparams: shadowed_tparams
457-
.into_iter()
458-
.map(|s| ClassName::intern(&s))
459-
.collect(),
457+
tparam_info: tparam_info.into(),
460458
return_type: return_type.into(),
461459
doc_comment: doc_comment
462460
.map(|(_, comment)| comment.into_bytes().into())
@@ -759,18 +757,22 @@ pub fn emit_generics_upper_bounds(
759757
.collect::<Vec<_>>()
760758
}
761759

762-
fn emit_shadowed_tparams(
760+
fn emit_tparam_info(
763761
immediate_tparams: &[ast::Tparam],
764762
class_tparam_names: &[&str],
765-
) -> Vec<String> {
766-
let s1 = get_tp_names_set(immediate_tparams);
763+
) -> Vec<TParamInfo> {
764+
let s1 = get_tp_names(immediate_tparams);
767765
let s2: HashSet<&str> = class_tparam_names.iter().cloned().collect();
768-
// TODO(hrust): remove sort after Rust emitter released
769-
let mut r = s1
770-
.intersection(&s2)
771-
.map(|s| (*s).into())
772-
.collect::<Vec<_>>();
773-
r.sort();
766+
let r: Vec<TParamInfo> = s1
767+
.iter()
768+
.map(|name| {
769+
let shadows_class_tparam = s2.contains(name);
770+
TParamInfo {
771+
name: ClassName::intern(name),
772+
shadows_class_tparam,
773+
}
774+
})
775+
.collect();
774776
r
775777
}
776778

@@ -790,10 +792,6 @@ pub fn get_tp_names(tparams: &[ast::Tparam]) -> Vec<&str> {
790792
tparams.iter().map(get_tp_name).collect()
791793
}
792794

793-
pub fn get_tp_names_set(tparams: &[ast::Tparam]) -> HashSet<&str> {
794-
tparams.iter().map(get_tp_name).collect()
795-
}
796-
797795
fn modify_prog_for_debugger_eval(_: &mut InstrSeq) {
798796
unimplemented!() // SF(2021-03-17): I found it like this.
799797
}

hphp/hack/src/hackc/emitter/emit_class.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -116,7 +116,7 @@ fn make_86method<'d>(
116116
method_is_memoize_wrapper,
117117
method_is_memoize_wrapper_lsb,
118118
vec![], // upper_bounds
119-
vec![], // shadowed_tparams
119+
vec![], // tparam_info
120120
vec![], // attributes
121121
attrs,
122122
coeffects,
@@ -816,6 +816,7 @@ pub fn emit_class<'a, 'd>(emitter: &mut Emitter<'d>, ast_class: &'a ast::Class_)
816816
methods: methods.into(),
817817
enum_type: Maybe::from(enum_type),
818818
upper_bounds: upper_bounds.into(),
819+
tparams: tparams.into_iter().map(ClassName::intern).collect(),
819820
properties: Vec::from_iter(properties.into_iter().map(|p| p.prop)).into(),
820821
requirements: requirements.into(),
821822
type_constants: type_constants.into(),

hphp/hack/src/hackc/emitter/emit_constant.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ fn emit_constant_cinit<'a, 'd>(
6262
false, /* is_memoize_wrapper */
6363
false, /* is_memoize_wrapper_lsb */
6464
vec![], /* upper_bounds */
65-
vec![], /* shadowed_params */
65+
vec![], /* tparam_info */
6666
vec![], /* attributes */
6767
attrs,
6868
Coeffects::default(),

hphp/hack/src/hackc/emitter/emit_memoize_function.rs

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ use env::emitter::Emitter;
1212
use error::Result;
1313
use hhbc::Attribute;
1414
use hhbc::Body;
15+
use hhbc::ClassName;
1516
use hhbc::Coeffects;
1617
use hhbc::FCallArgs;
1718
use hhbc::FCallArgsFlags;
@@ -23,6 +24,7 @@ use hhbc::LocalRange;
2324
use hhbc::Param;
2425
use hhbc::Span;
2526
use hhbc::StringId;
27+
use hhbc::TParamInfo;
2628
use hhbc::TypeInfo;
2729
use hhbc::TypedValue;
2830
use hhbc_string_utils::reified;
@@ -81,6 +83,14 @@ pub(crate) fn emit_wrapper_function<'a, 'd>(
8183
.iter()
8284
.map(|tp| tp.name.1.as_str())
8385
.collect::<Vec<_>>();
86+
let tparam_info = fd
87+
.tparams
88+
.iter()
89+
.map(|s| TParamInfo {
90+
name: ClassName::intern(s.name.1.as_str()),
91+
shadows_class_tparam: false,
92+
})
93+
.collect::<Vec<_>>();
8494
let params = emit_param::from_asts(emitter, &mut tparams, true, &scope, &f.params)?;
8595
let mut attributes = emit_attribute::from_asts(emitter, &f.user_attributes)?;
8696
attributes.extend(emit_attribute::add_reified_attribute(&fd.tparams));
@@ -128,6 +138,7 @@ pub(crate) fn emit_wrapper_function<'a, 'd>(
128138
decl_vars,
129139
instrs,
130140
Span::from_pos(&f.span),
141+
tparam_info,
131142
)?;
132143

133144
let mut flags = FunctionFlags::empty();
@@ -388,6 +399,7 @@ fn make_wrapper_body<'a, 'd>(
388399
decl_vars: Vec<StringId>,
389400
instrs: InstrSeq,
390401
span: Span,
402+
tparam_info: Vec<TParamInfo>,
391403
) -> Result<Body> {
392404
emit_body::make_body(
393405
emitter,
@@ -396,7 +408,7 @@ fn make_wrapper_body<'a, 'd>(
396408
true, /* is_memoize_wrapper */
397409
false, /* is_memoize_wrapper_lsb */
398410
vec![], /* upper_bounds */
399-
vec![], /* shadowed_tparams */
411+
tparam_info,
400412
attributes,
401413
Attr::AttrNone,
402414
coeffects,

0 commit comments

Comments
 (0)