Skip to content

Commit

Permalink
Fix tagged template creation (#2925)
Browse files Browse the repository at this point in the history
* Fix tagged template creation

* Fix template identifier hash

* Apply suggestion

* Apply suggestion
  • Loading branch information
raskad authored May 17, 2023
1 parent 4f25f2c commit efeaa40
Show file tree
Hide file tree
Showing 13 changed files with 241 additions and 33 deletions.
10 changes: 10 additions & 0 deletions boa_ast/src/expression/tagged_template.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ pub struct TaggedTemplate {
raws: Box<[Sym]>,
cookeds: Box<[Option<Sym>]>,
exprs: Box<[Expression]>,
identifier: u64,
}

impl TaggedTemplate {
Expand All @@ -33,12 +34,14 @@ impl TaggedTemplate {
raws: Box<[Sym]>,
cookeds: Box<[Option<Sym>]>,
exprs: Box<[Expression]>,
identifier: u64,
) -> Self {
Self {
tag: tag.into(),
raws,
cookeds,
exprs,
identifier,
}
}

Expand Down Expand Up @@ -69,6 +72,13 @@ impl TaggedTemplate {
pub const fn exprs(&self) -> &[Expression] {
&self.exprs
}

/// Gets the unique identifier of the template.
#[inline]
#[must_use]
pub const fn identifier(&self) -> u64 {
self.identifier
}
}

impl ToInternedString for TaggedTemplate {
Expand Down
1 change: 1 addition & 0 deletions boa_engine/src/builtins/eval/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,7 @@ impl Eval {
// c. If script Contains ScriptBody is false, return undefined.
// d. Let body be the ScriptBody of script.
let mut parser = Parser::new(Source::from_bytes(&x));
parser.set_identifier(context.next_parser_identifier());
if strict {
parser.set_strict();
}
Expand Down
37 changes: 21 additions & 16 deletions boa_engine/src/builtins/function/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -586,17 +586,22 @@ impl BuiltInFunctionObject {
let parameters = parameters.join(utf16!(","));

// TODO: make parser generic to u32 iterators
let parameters =
match Parser::new(Source::from_bytes(&String::from_utf16_lossy(&parameters)))
.parse_formal_parameters(context.interner_mut(), generator, r#async)
{
Ok(parameters) => parameters,
Err(e) => {
return Err(JsNativeError::syntax()
.with_message(format!("failed to parse function parameters: {e}"))
.into())
}
};
let parameters = String::from_utf16_lossy(&parameters);
let mut parser = Parser::new(Source::from_bytes(&parameters));
parser.set_identifier(context.next_parser_identifier());

let parameters = match parser.parse_formal_parameters(
context.interner_mut(),
generator,
r#async,
) {
Ok(parameters) => parameters,
Err(e) => {
return Err(JsNativeError::syntax()
.with_message(format!("failed to parse function parameters: {e}"))
.into())
}
};

if generator && contains(&parameters, ContainsSymbol::YieldExpression) {
return Err(JsNativeError::syntax().with_message(
Expand Down Expand Up @@ -626,11 +631,11 @@ impl BuiltInFunctionObject {
let body = b"\n".chain(body_arg.as_bytes()).chain(b"\n".as_slice());

// TODO: make parser generic to u32 iterators
let body = match Parser::new(Source::from_reader(body, None)).parse_function_body(
context.interner_mut(),
generator,
r#async,
) {
let mut parser = Parser::new(Source::from_reader(body, None));
parser.set_identifier(context.next_parser_identifier());

let body = match parser.parse_function_body(context.interner_mut(), generator, r#async)
{
Ok(statement_list) => statement_list,
Err(e) => {
return Err(JsNativeError::syntax()
Expand Down
25 changes: 11 additions & 14 deletions boa_engine/src/bytecompiler/expression/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ use boa_ast::{
},
Expression,
};
use boa_interner::Sym;

impl ByteCompiler<'_, '_> {
fn compile_literal(&mut self, lit: &AstLiteral, use_expr: bool) {
Expand Down Expand Up @@ -255,31 +254,29 @@ impl ByteCompiler<'_, '_> {
}
}

self.emit_opcode(Opcode::PushNewArray);
for cooked in template.cookeds() {
let site = template.identifier();
let count = template.cookeds().len() as u32;

let jump_label = self.emit_opcode_with_operand(Opcode::TemplateLookup);
self.emit_u64(site);

for (cooked, raw) in template.cookeds().iter().zip(template.raws()) {
if let Some(cooked) = cooked {
self.emit_push_literal(Literal::String(
self.interner().resolve_expect(*cooked).into_common(false),
));
} else {
self.emit_opcode(Opcode::PushUndefined);
}
self.emit_opcode(Opcode::PushValueToArray);
}
self.emit_opcode(Opcode::Dup);
self.emit_opcode(Opcode::Dup);

self.emit_opcode(Opcode::PushNewArray);
for raw in template.raws() {
self.emit_push_literal(Literal::String(
self.interner().resolve_expect(*raw).into_common(false),
));
self.emit_opcode(Opcode::PushValueToArray);
}

let index = self.get_or_insert_name(Sym::RAW.into());
self.emit(Opcode::SetPropertyByName, &[index]);
self.emit(Opcode::Pop, &[]);
self.emit(Opcode::TemplateCreate, &[count]);
self.emit_u64(site);

self.patch_jump(jump_label);

for expr in template.exprs() {
self.compile_expr(expr, true);
Expand Down
12 changes: 12 additions & 0 deletions boa_engine/src/context/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,9 @@ pub struct Context<'host> {

optimizer_options: OptimizerOptions,
root_shape: SharedShape,

/// Unique identifier for each parser instance used during the context lifetime.
parser_identifier: u32,
}

impl std::fmt::Debug for Context<'_> {
Expand Down Expand Up @@ -230,6 +233,7 @@ impl<'host> Context<'host> {
) -> Result<StatementList, ParseError> {
let _timer = Profiler::global().start_event("Script parsing", "Main");
let mut parser = Parser::new(src);
parser.set_identifier(self.next_parser_identifier());
if self.strict {
parser.set_strict();
}
Expand All @@ -247,6 +251,7 @@ impl<'host> Context<'host> {
) -> Result<ModuleItemList, ParseError> {
let _timer = Profiler::global().start_event("Module parsing", "Main");
let mut parser = Parser::new(src);
parser.set_identifier(self.next_parser_identifier());
parser.parse_module(&mut self.interner)
}

Expand Down Expand Up @@ -656,6 +661,12 @@ impl Context<'_> {
std::mem::swap(&mut self.realm, realm);
}

/// Increment and get the parser identifier.
pub(crate) fn next_parser_identifier(&mut self) -> u32 {
self.parser_identifier += 1;
self.parser_identifier
}

/// `CanDeclareGlobalFunction ( N )`
///
/// More information:
Expand Down Expand Up @@ -1025,6 +1036,7 @@ impl<'icu, 'hooks, 'queue> ContextBuilder<'icu, 'hooks, 'queue> {
}),
optimizer_options: OptimizerOptions::OPTIMIZE_ALL,
root_shape,
parser_identifier: 0,
};

builtins::set_default_global_bindings(&mut context)?;
Expand Down
16 changes: 13 additions & 3 deletions boa_engine/src/realm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,15 +6,15 @@
//!
//! A realm is represented in this implementation as a Realm struct with the fields specified from the spec.

use std::fmt;

use crate::{
context::{intrinsics::Intrinsics, HostHooks},
environments::DeclarativeEnvironment,
object::{shape::shared_shape::SharedShape, JsObject},
};
use boa_gc::{Finalize, Gc, Trace};
use boa_gc::{Finalize, Gc, GcRefCell, Trace};
use boa_profiler::Profiler;
use rustc_hash::FxHashMap;
use std::fmt;

/// Representation of a Realm.
///
Expand Down Expand Up @@ -49,6 +49,7 @@ struct Inner {
environment: Gc<DeclarativeEnvironment>,
global_object: JsObject,
global_this: JsObject,
template_map: GcRefCell<FxHashMap<u64, JsObject>>,
}

impl Realm {
Expand All @@ -70,6 +71,7 @@ impl Realm {
environment,
global_object,
global_this,
template_map: GcRefCell::new(FxHashMap::default()),
}),
};

Expand Down Expand Up @@ -110,4 +112,12 @@ impl Realm {
bindings.resize(binding_number, None);
}
}

pub(crate) fn push_template(&self, site: u64, template: JsObject) {
self.inner.template_map.borrow_mut().insert(site, template);
}

pub(crate) fn lookup_template(&self, site: u64) -> Option<JsObject> {
self.inner.template_map.borrow().get(&site).cloned()
}
}
7 changes: 7 additions & 0 deletions boa_engine/src/vm/code_block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -300,6 +300,13 @@ impl CodeBlock {
*pc += size_of::<u32>();
format!("{operand1}, {operand2}")
}
Opcode::TemplateLookup | Opcode::TemplateCreate => {
let operand1 = self.read::<u32>(*pc);
*pc += size_of::<u32>();
let operand2 = self.read::<u64>(*pc);
*pc += size_of::<u64>();
format!("{operand1}, {operand2}")
}
Opcode::GeneratorAsyncDelegateResume => {
let operand1 = self.read::<u32>(*pc);
*pc += size_of::<u32>();
Expand Down
10 changes: 10 additions & 0 deletions boa_engine/src/vm/flowgraph/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,16 @@ impl CodeBlock {
graph.add_node(previous_pc, NodeShape::None, label.into(), Color::Red);
graph.add_edge(previous_pc, pc, None, Color::None, EdgeStyle::Line);
}
Opcode::TemplateLookup | Opcode::TemplateCreate => {
let start_address = self.read::<u32>(pc);
pc += size_of::<u32>();
let end_address = self.read::<u64>(pc);
pc += size_of::<u64>();

let label = format!("{opcode_str} {start_address}, {end_address}");
graph.add_node(previous_pc, NodeShape::None, label.into(), Color::Red);
graph.add_edge(previous_pc, pc, None, Color::None, EdgeStyle::Line);
}
Opcode::Break => {
let jump_operand = self.read::<u32>(pc);
pc += size_of::<u32>();
Expand Down
17 changes: 17 additions & 0 deletions boa_engine/src/vm/opcode/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ mod rest_parameter;
mod set;
mod swap;
mod switch;
mod templates;
mod to;
mod unary_ops;
mod value;
Expand Down Expand Up @@ -79,6 +80,8 @@ pub(crate) use swap::*;
#[doc(inline)]
pub(crate) use switch::*;
#[doc(inline)]
pub(crate) use templates::*;
#[doc(inline)]
pub(crate) use to::*;
#[doc(inline)]
pub(crate) use unary_ops::*;
Expand Down Expand Up @@ -1602,6 +1605,20 @@ generate_impl! {
/// Stack: value **=>** is_object
IsObject,

/// Lookup if a tagged template object is cached and skip the creation if it is.
///
/// Operands: jump: `u32`, site: `u64`
///
/// Stack: **=>** template (if cached)
TemplateLookup,

/// Create a new tagged template object and cache it.
///
/// Operands: count: `u32`, site: `u64`
///
/// Stack: count * (cooked_value, raw_value) **=>** template
TemplateCreate,

/// No-operation instruction, does nothing.
///
/// Operands:
Expand Down
Loading

0 comments on commit efeaa40

Please sign in to comment.