From 7a059ab53ac049c580bb1742ecc4dc5cffbfc6fd Mon Sep 17 00:00:00 2001 From: rzvxa <3788964+rzvxa@users.noreply.github.com> Date: Thu, 11 Jul 2024 01:39:28 +0000 Subject: [PATCH 1/3] fix(cfg): double resolution of labeled statements. (#4177) Fixes #4173 --- crates/oxc_cfg/src/builder/context.rs | 7 +++-- crates/oxc_cfg/tests/builder.rs | 38 +++++++++++++++++++++++++++ 2 files changed, 43 insertions(+), 2 deletions(-) create mode 100644 crates/oxc_cfg/tests/builder.rs diff --git a/crates/oxc_cfg/src/builder/context.rs b/crates/oxc_cfg/src/builder/context.rs index 692368ebc103..3097f3053213 100644 --- a/crates/oxc_cfg/src/builder/context.rs +++ b/crates/oxc_cfg/src/builder/context.rs @@ -130,9 +130,12 @@ impl<'a, 'c> QueryCtx<'a, 'c> { self.resolve_ctx(ctx); - // mark the upper label continue jump point the same as ours, + // mark the upper label continue jump point the same as ours if it isn't already assigned, + // NOTE: if it is already assigned there's a resolution before this context. if let Some(jmp) = continue_jmp { - if let Some(label_ctx) = self.0.immediate_labeled_ctx() { + if let Some(label_ctx @ RefCtxCursor(Ctx { continue_jmp: None, .. })) = + self.0.immediate_labeled_ctx() + { label_ctx.mark_continue(jmp); } } diff --git a/crates/oxc_cfg/tests/builder.rs b/crates/oxc_cfg/tests/builder.rs new file mode 100644 index 000000000000..58e2dc4d8c40 --- /dev/null +++ b/crates/oxc_cfg/tests/builder.rs @@ -0,0 +1,38 @@ +use oxc_cfg::{ControlFlowGraphBuilder, CtxCursor}; +use oxc_syntax::node::AstNodeId; +/// same as but just the skeleton +/// ```js +/// A: { +/// do {} while (a); +/// do {} while (b); +/// break A; +/// } +/// ``` +#[test] +fn labeled_statement_with_multiple_loops_continue_and_break() { + const A: Option<&str> = Some("A"); + + let mut cfg = ControlFlowGraphBuilder::default(); + cfg.attach_error_harness(oxc_cfg::ErrorEdgeKind::Implicit); + + // labeled block start + let labeled = cfg.new_basic_block_normal(); + cfg.ctx(A).default().allow_break().allow_continue(); + + // loop context 1 + let c1 = cfg.new_basic_block_normal(); + cfg.ctx(None).default().allow_break().allow_continue(); + cfg.ctx(None).mark_break(c1).mark_continue(c1).resolve_with_upper_label(); + + // loop context 2 + let c2 = cfg.new_basic_block_normal(); + cfg.ctx(None).default().allow_break().allow_continue(); + cfg.ctx(None).mark_break(c2).mark_continue(c2).resolve_with_upper_label(); + + cfg.append_break(AstNodeId::dummy(), A); + + // labeled block end + cfg.ctx(A).mark_break(labeled).resolve(); + + cfg.build(); +} From ddfa3434753022877fdced077c5a72a6695d91dd Mon Sep 17 00:00:00 2001 From: DonIsaac <22823424+DonIsaac@users.noreply.github.com> Date: Thu, 11 Jul 2024 01:44:13 +0000 Subject: [PATCH 2/3] perf(diagnostic): use `Cow<'static, str>` over `String` (#4175) Allows us to use `&'static str` for error and help messages without allocating them into `String`s. --- crates/oxc_diagnostics/src/lib.rs | 11 ++++++----- .../oxc_linter/src/rules/oxc/no_optional_chaining.rs | 2 +- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/crates/oxc_diagnostics/src/lib.rs b/crates/oxc_diagnostics/src/lib.rs index e955dab7cc23..34fa0ed9945b 100644 --- a/crates/oxc_diagnostics/src/lib.rs +++ b/crates/oxc_diagnostics/src/lib.rs @@ -7,6 +7,7 @@ mod reporter; mod service; use std::{ + borrow::Cow, fmt::{self, Display}, ops::Deref, }; @@ -42,9 +43,9 @@ impl Deref for OxcDiagnostic { #[derive(Debug, Clone)] pub struct OxcDiagnosticInner { - pub message: String, + pub message: Cow<'static, str>, pub labels: Option>, - pub help: Option, + pub help: Option>, pub severity: Severity, } @@ -76,7 +77,7 @@ impl Diagnostic for OxcDiagnostic { impl OxcDiagnostic { #[must_use] - pub fn error>(message: T) -> Self { + pub fn error>>(message: T) -> Self { Self { inner: Box::new(OxcDiagnosticInner { message: message.into(), @@ -88,7 +89,7 @@ impl OxcDiagnostic { } #[must_use] - pub fn warn>(message: T) -> Self { + pub fn warn>>(message: T) -> Self { Self { inner: Box::new(OxcDiagnosticInner { message: message.into(), @@ -106,7 +107,7 @@ impl OxcDiagnostic { } #[must_use] - pub fn with_help>(mut self, help: T) -> Self { + pub fn with_help>>(mut self, help: T) -> Self { self.inner.help = Some(help.into()); self } diff --git a/crates/oxc_linter/src/rules/oxc/no_optional_chaining.rs b/crates/oxc_linter/src/rules/oxc/no_optional_chaining.rs index 363d0f5b5bd8..b2e967e648cb 100644 --- a/crates/oxc_linter/src/rules/oxc/no_optional_chaining.rs +++ b/crates/oxc_linter/src/rules/oxc/no_optional_chaining.rs @@ -11,7 +11,7 @@ fn no_optional_chaining_diagnostic(span0: Span, x1: &str) -> OxcDiagnostic { .with_label(span0) } else { OxcDiagnostic::warn("oxc(no-optional-chaining): Optional chaining is not allowed.") - .with_help(x1) + .with_help(x1.to_owned()) .with_label(span0) } } From 07506ece83e7101f618093afa74be23babfc58c3 Mon Sep 17 00:00:00 2001 From: Boshen Date: Thu, 11 Jul 2024 10:18:13 +0800 Subject: [PATCH 3/3] ci: add a `git diff` check after running tests --- .github/workflows/ci.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 8bfe0e1af006..71333597d57e 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -76,6 +76,7 @@ jobs: - run: cargo ck - run: cargo test --no-run - run: cargo test + - run: git diff --exit-code # Must commit everything test-windows: needs: optimize_ci