Skip to content

Commit b0e1322

Browse files
sapphi-redBoshen
authored andcommitted
feat(ecmascript): add property_read_side_effects to MayHaveSideEffectsContext
1 parent ac2ecbb commit b0e1322

File tree

5 files changed

+68
-6
lines changed

5 files changed

+68
-6
lines changed

crates/oxc_ecmascript/src/side_effects/context.rs

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,18 @@ use oxc_ast::ast::Expression;
22

33
use crate::is_global_reference::IsGlobalReference;
44

5+
#[derive(Default, Clone, Copy, Debug, PartialEq, Eq)]
6+
pub enum PropertyReadSideEffects {
7+
/// Treat all property read accesses as side effect free.
8+
None,
9+
/// Treat non-member property accesses as side effect free.
10+
/// Member property accesses are still considered to have side effects.
11+
OnlyMemberPropertyAccess,
12+
/// Treat all property read accesses as possible side effects.
13+
#[default]
14+
All,
15+
}
16+
517
pub trait MayHaveSideEffectsContext: IsGlobalReference {
618
/// Whether to respect the pure annotations.
719
///
@@ -14,4 +26,7 @@ pub trait MayHaveSideEffectsContext: IsGlobalReference {
1426
/// This function is called for normal function calls, new calls, and
1527
/// tagged template calls (`foo()`, `new Foo()`, ``foo`b` ``).
1628
fn is_pure_call(&self, callee: &Expression) -> bool;
29+
30+
/// Whether property read accesses have side effects.
31+
fn property_read_side_effects(&self) -> PropertyReadSideEffects;
1732
}

crates/oxc_ecmascript/src/side_effects/may_have_side_effects.rs

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ use crate::{
55
to_primitive::ToPrimitive,
66
};
77

8-
use super::context::MayHaveSideEffectsContext;
8+
use super::{PropertyReadSideEffects, context::MayHaveSideEffectsContext};
99

1010
/// Returns true if subtree changes application state.
1111
///
@@ -398,7 +398,9 @@ impl MayHaveSideEffects for MemberExpression<'_> {
398398
match self {
399399
MemberExpression::ComputedMemberExpression(e) => e.may_have_side_effects(ctx),
400400
MemberExpression::StaticMemberExpression(e) => e.may_have_side_effects(ctx),
401-
MemberExpression::PrivateFieldExpression(_) => true,
401+
MemberExpression::PrivateFieldExpression(_) => {
402+
ctx.property_read_side_effects() != PropertyReadSideEffects::None
403+
}
402404
}
403405
}
404406
}
@@ -446,6 +448,9 @@ fn property_access_may_have_side_effects(
446448
if object.may_have_side_effects(ctx) {
447449
return true;
448450
}
451+
if ctx.property_read_side_effects() == PropertyReadSideEffects::None {
452+
return false;
453+
}
449454

450455
match property {
451456
"length" => {
@@ -464,6 +469,9 @@ fn integer_index_property_access_may_have_side_effects(
464469
if object.may_have_side_effects(ctx) {
465470
return true;
466471
}
472+
if ctx.property_read_side_effects() == PropertyReadSideEffects::None {
473+
return false;
474+
}
467475
match object {
468476
Expression::StringLiteral(s) => property as usize >= s.value.encode_utf16().count(),
469477
Expression::ArrayExpression(arr) => property as usize >= get_array_minimum_length(arr),
Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
mod context;
22
mod may_have_side_effects;
33

4-
pub use context::MayHaveSideEffectsContext;
4+
pub use context::{MayHaveSideEffectsContext, PropertyReadSideEffects};
55
pub use may_have_side_effects::MayHaveSideEffects;

crates/oxc_minifier/src/ctx.rs

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ use oxc_ast::{AstBuilder, ast::*};
44
use oxc_ecmascript::constant_evaluation::{
55
ConstantEvaluation, ConstantEvaluationCtx, ConstantValue, binary_operation_evaluate_value,
66
};
7-
use oxc_ecmascript::side_effects::MayHaveSideEffects;
7+
use oxc_ecmascript::side_effects::{MayHaveSideEffects, PropertyReadSideEffects};
88
use oxc_semantic::{IsGlobalReference, Scoping};
99
use oxc_traverse::TraverseCtx;
1010

@@ -33,6 +33,10 @@ impl oxc_ecmascript::side_effects::MayHaveSideEffectsContext for Ctx<'_, '_> {
3333
fn is_pure_call(&self, _callee: &Expression) -> bool {
3434
false
3535
}
36+
37+
fn property_read_side_effects(&self) -> PropertyReadSideEffects {
38+
PropertyReadSideEffects::All
39+
}
3640
}
3741

3842
impl<'a> ConstantEvaluationCtx<'a> for Ctx<'a, '_> {

crates/oxc_minifier/tests/ecmascript/may_have_side_effects.rs

Lines changed: 37 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ use oxc_allocator::Allocator;
22
use oxc_ast::ast::{Expression, IdentifierReference, Statement};
33
use oxc_ecmascript::{
44
is_global_reference::IsGlobalReference,
5-
side_effects::{MayHaveSideEffects, MayHaveSideEffectsContext},
5+
side_effects::{MayHaveSideEffects, MayHaveSideEffectsContext, PropertyReadSideEffects},
66
};
77
use oxc_parser::Parser;
88
use oxc_span::SourceType;
@@ -11,10 +11,16 @@ struct Ctx {
1111
global_variable_names: Vec<String>,
1212
annotation: bool,
1313
pure_function_names: Vec<String>,
14+
property_read_side_effects: PropertyReadSideEffects,
1415
}
1516
impl Default for Ctx {
1617
fn default() -> Self {
17-
Self { global_variable_names: vec![], annotation: true, pure_function_names: vec![] }
18+
Self {
19+
global_variable_names: vec![],
20+
annotation: true,
21+
pure_function_names: vec![],
22+
property_read_side_effects: PropertyReadSideEffects::All,
23+
}
1824
}
1925
}
2026
impl IsGlobalReference for Ctx {
@@ -34,6 +40,10 @@ impl MayHaveSideEffectsContext for Ctx {
3440
false
3541
}
3642
}
43+
44+
fn property_read_side_effects(&self) -> PropertyReadSideEffects {
45+
self.property_read_side_effects
46+
}
3747
}
3848

3949
fn test(source_text: &str, expected: bool) {
@@ -750,6 +760,31 @@ fn test_is_pure_call_support() {
750760
test_with_ctx("bar``", &ctx, true);
751761
}
752762

763+
#[test]
764+
fn test_property_read_side_effects_support() {
765+
let all_ctx =
766+
Ctx { property_read_side_effects: PropertyReadSideEffects::All, ..Default::default() };
767+
let only_member_ctx = Ctx {
768+
property_read_side_effects: PropertyReadSideEffects::OnlyMemberPropertyAccess,
769+
..Default::default()
770+
};
771+
let none_ctx =
772+
Ctx { property_read_side_effects: PropertyReadSideEffects::None, ..Default::default() };
773+
774+
test_with_ctx("foo.bar", &all_ctx, true);
775+
test_with_ctx("foo.bar", &only_member_ctx, true);
776+
test_with_ctx("foo.bar", &none_ctx, false);
777+
test_with_ctx("foo[0]", &none_ctx, false);
778+
test_with_ctx("foo[0n]", &none_ctx, false);
779+
test_with_ctx("foo[bar()]", &none_ctx, true);
780+
test_with_ctx("foo.#bar", &all_ctx, true);
781+
test_with_ctx("foo.#bar", &only_member_ctx, true);
782+
test_with_ctx("foo.#bar", &none_ctx, false);
783+
test_with_ctx("({ bar } = foo)", &all_ctx, true);
784+
// test_with_ctx("({ bar } = foo)", &only_member_ctx, false);
785+
// test_with_ctx("({ bar } = foo)", &none_ctx, false);
786+
}
787+
753788
#[test]
754789
fn test_object_with_to_primitive_related_properties_overridden() {
755790
test("+{}", false);

0 commit comments

Comments
 (0)