Skip to content

Commit a56f1cd

Browse files
crisbetoatscott
crisbeto
authored and
atscott
committed
fix(compiler): more robust logic to check if regex can be optimized
Currently we only skip regex optimization if it has the `g` flag, however regexes can also have a state with the `y` flag. These changes move to an allowlist model where we only optimize for a set of know flags. (cherry picked from commit 636cc94)
1 parent 32b348a commit a56f1cd

6 files changed

Lines changed: 77 additions & 5 deletions

File tree

‎packages/compiler-cli/test/compliance/test_cases/r3_compiler_compliance/components_and_directives/value_composition/GOLDEN_PARTIAL.js‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -980,6 +980,33 @@ export declare class TestComp {
980980
static ɵcmp: i0.ɵɵComponentDeclaration<TestComp, "ng-component", never, {}, {}, never, never, true, never>;
981981
}
982982

983+
/****************************************************************************************************
984+
* PARTIAL FILE: regular_expression_with_sticky_flag.js
985+
****************************************************************************************************/
986+
import { Component } from '@angular/core';
987+
import * as i0 from "@angular/core";
988+
export class TestComp {
989+
value = '123';
990+
static ɵfac = i0.ɵɵngDeclareFactory({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: TestComp, deps: [], target: i0.ɵɵFactoryTarget.Component });
991+
static ɵcmp = i0.ɵɵngDeclareComponent({ minVersion: "14.0.0", version: "0.0.0-PLACEHOLDER", type: TestComp, isStandalone: true, selector: "ng-component", ngImport: i0, template: `{{/^hello/y.test(value)}}`, isInline: true });
992+
}
993+
i0.ɵɵngDeclareClassMetadata({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: TestComp, decorators: [{
994+
type: Component,
995+
args: [{
996+
template: `{{/^hello/y.test(value)}}`,
997+
}]
998+
}] });
999+
1000+
/****************************************************************************************************
1001+
* PARTIAL FILE: regular_expression_with_sticky_flag.d.ts
1002+
****************************************************************************************************/
1003+
import * as i0 from "@angular/core";
1004+
export declare class TestComp {
1005+
value: string;
1006+
static ɵfac: i0.ɵɵFactoryDeclaration<TestComp, never>;
1007+
static ɵcmp: i0.ɵɵComponentDeclaration<TestComp, "ng-component", never, {}, {}, never, never, true, never>;
1008+
}
1009+
9831010
/****************************************************************************************************
9841011
* PARTIAL FILE: call_rest.js
9851012
****************************************************************************************************/

‎packages/compiler-cli/test/compliance/test_cases/r3_compiler_compliance/components_and_directives/value_composition/TEST_CASES.json‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -317,6 +317,16 @@
317317
}
318318
]
319319
},
320+
{
321+
"description": "should not optimize sticky regular expressions",
322+
"inputFiles": ["regular_expression_with_sticky_flag.ts"],
323+
"expectations": [
324+
{
325+
"failureMessage": "Invalid template",
326+
"files": ["regular_expression_with_sticky_flag.js"]
327+
}
328+
]
329+
},
320330
{
321331
"description": "should support rest arguments in a function call",
322332
"inputFiles": ["call_rest.ts"],

‎packages/compiler-cli/test/compliance/test_cases/r3_compiler_compliance/components_and_directives/value_composition/regular_expression_with_sticky_flag.js‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
function TestComp_Template(rf, ctx) {
2+
if (rf & 1) {
3+
$r3$.ɵɵtext(0);
4+
}
5+
if (rf & 2) {
6+
$r3$.ɵɵtextInterpolate(/^hello/y.test(ctx.value));
7+
}
8+
}

‎packages/compiler-cli/test/compliance/test_cases/r3_compiler_compliance/components_and_directives/value_composition/regular_expression_with_sticky_flag.ts‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
import {Component} from '@angular/core';
2+
3+
@Component({
4+
template: `{{/^hello/y.test(value)}}`,
5+
})
6+
export class TestComp {
7+
value = '123';
8+
}

‎packages/compiler-cli/test/compliance/test_cases/r3_compiler_compliance/components_and_directives/value_composition/regular_expression_with_sticky_flag_isolated.golden.d.ts‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
import * as i0 from "@angular/core";
2+
export declare class TestComp {
3+
value: string;
4+
static ɵfac: i0.ɵɵFactoryDeclaration<TestComp, never>;
5+
static ɵcmp: i0.ɵɵComponentDeclaration<TestComp, "ng-component", never, {}, {}, never, never, true, never>;
6+
}

‎packages/compiler/src/template/pipeline/src/phases/regular_expression_optimization.ts‎

Lines changed: 18 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -12,18 +12,17 @@ import * as ir from '../../ir';
1212

1313
import type {CompilationJob} from '../compilation';
1414

15+
/** Regex flags that can be safely optimized. */
16+
const SAFE_REGEX_FLAGS = new Set(['d', 'i', 'm', 's', 'u', 'v']);
17+
1518
/** Optimizes regular expressions used in expressions. */
1619
export function optimizeRegularExpressions(job: CompilationJob): void {
1720
for (const view of job.units) {
1821
for (const op of view.ops()) {
1922
ir.transformExpressionsInOp(
2023
op,
2124
(expr) => {
22-
if (
23-
expr instanceof o.RegularExpressionLiteralExpr &&
24-
// We can't optimize global regexes, because they're stateful.
25-
(expr.flags === null || !expr.flags.includes('g'))
26-
) {
25+
if (expr instanceof o.RegularExpressionLiteralExpr && canOptimizeRegex(expr)) {
2726
return job.pool.getSharedConstant(new RegularExpressionConstant(), expr);
2827
}
2928
return expr;
@@ -34,6 +33,20 @@ export function optimizeRegularExpressions(job: CompilationJob): void {
3433
}
3534
}
3635

36+
function canOptimizeRegex(expr: o.RegularExpressionLiteralExpr): boolean {
37+
if (!expr.flags) {
38+
return true;
39+
}
40+
41+
for (let i = 0; i < expr.flags.length; i++) {
42+
if (!SAFE_REGEX_FLAGS.has(expr.flags[i])) {
43+
return false;
44+
}
45+
}
46+
47+
return true;
48+
}
49+
3750
class RegularExpressionConstant extends GenericKeyFn implements SharedConstantDefinition {
3851
toSharedConstantDeclaration(declName: string, keyExpr: o.Expression): o.Statement {
3952
return new o.DeclareVarStmt(declName, keyExpr, undefined, o.StmtModifier.Final);

0 commit comments

Comments
 (0)