Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: alternative hoisting method for optimizer #371

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 36 additions & 0 deletions crates/core/src/test_files.rs
Original file line number Diff line number Diff line change
Expand Up @@ -429,3 +429,39 @@ fn avoid_unsafe_hoists() {
assert_eq!(results.len(), 4);
assert!(results.iter().any(|r| r.is_match()));
}

#[test]
fn hoist_or() {
let pattern_src = r#"
`console.log($msg)` where {
$filename <: includes or { "foo.js", "target.js", "two.js" }
}
"#;
let libs = BTreeMap::new();

let matching_src = r#"
console.log("target.js");
"#;

let pattern = src_to_problem_libs(
pattern_src.to_string(),
&libs,
TargetLanguage::default(),
None,
None,
None,
None,
)
.unwrap()
.problem;

// All together now
let test_files = vec![
SyntheticFile::new("wrong.js".to_owned(), matching_src.to_owned(), false),
SyntheticFile::new("target.js".to_owned(), matching_src.to_owned(), true),
SyntheticFile::new("other.js".to_owned(), matching_src.to_owned(), false),
SyntheticFile::new("two.js".to_owned(), matching_src.to_owned(), true),
];
let results = run_on_test_files(&pattern, &test_files);
assert!(results.iter().any(|r| r.is_match()));
}
28 changes: 19 additions & 9 deletions crates/grit-pattern-matcher/src/pattern/accessor.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use super::{
container::{Container, PatternOrResolved, PatternOrResolvedMut},
map::GritMap,
patterns::{Matcher, Pattern, PatternName},
patterns::{Matcher, Pattern, PatternName, PatternResult},
resolved_pattern::ResolvedPattern,
state::State,
variable::Variable,
Expand Down Expand Up @@ -128,16 +128,25 @@ impl<Q: QueryContext> Matcher<Q> for Accessor<Q> {
state: &mut State<'a, Q>,
context: &'a Q::ExecContext<'a>,
logs: &mut AnalysisLogs,
) -> Result<bool> {
) -> Result<PatternResult> {
match self.get(state, context.language())? {
Some(PatternOrResolved::Resolved(r)) => {
execute_resolved_with_binding(r, binding, state, context.language())
match execute_resolved_with_binding(r, binding, state, context.language()) {
Ok(result) => result,
Err(_) => PatternResult::Unknown,
}
}
Some(PatternOrResolved::ResolvedBinding(r)) => {
execute_resolved_with_binding(&r, binding, state, context.language())
match execute_resolved_with_binding(&r, binding, state, context.language()) {
Ok(result) => result,
Err(_) => PatternResult::Unknown,
}
}
Some(PatternOrResolved::Pattern(p)) => p.execute(binding, state, context, logs),
None => Ok(binding.matches_false_or_undefined()),
Some(PatternOrResolved::Pattern(p)) => match p.execute(binding, state, context, logs) {
Ok(result) => result,
Err(_) => PatternResult::Unknown,
},
None => PatternResult::False,
}
}
}
Expand All @@ -147,10 +156,11 @@ pub fn execute_resolved_with_binding<'a, Q: QueryContext>(
binding: &Q::ResolvedPattern<'a>,
state: &State<'a, Q>,
language: &Q::Language<'a>,
) -> Result<bool> {
) -> Result<PatternResult> {
if let (Some(r), Some(b)) = (r.get_last_binding(), binding.get_last_binding()) {
Ok(r.is_equivalent_to(b, language))
Ok(r.is_equivalent_to(b, language).into())
} else {
Ok(r.text(&state.files, language)? == binding.text(&state.files, language)?)
let status = r.text(&state.files, language)? == binding.text(&state.files, language)?;
Ok(status.into())
}
}
16 changes: 15 additions & 1 deletion crates/grit-pattern-matcher/src/pattern/maybe.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,28 @@ use crate::context::QueryContext;
use anyhow::Result;
use grit_util::AnalysisLogs;

/// Maybe is used to indicate patterns where we *might* might match
/// In default operation, it will evaluate all pattern but return true no matter what.
///
/// In strict mode, it will evaluate the pattern but return true only if pattern *might* match
/// There are 3 possible outcomes when evaluating subpatterns in strict mode:
/// 1. Pattern definitely matches -> maybe matches
/// 2. Pattern definitely does not match -> maybe does not match
/// 3. Pattern hits an error or unresolved variable/binding during evaluation -> maybe matches
#[derive(Debug, Clone)]
pub struct Maybe<Q: QueryContext> {
pub pattern: Pattern<Q>,

/// If true, we should exclude cases where the pattern is definite
strict: bool,
}

impl<Q: QueryContext> Maybe<Q> {
pub fn new(pattern: Pattern<Q>) -> Self {
Self { pattern }
Self {
pattern,
strict: false,
}
}
}

Expand Down
21 changes: 20 additions & 1 deletion crates/grit-pattern-matcher/src/pattern/patterns.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,25 @@ use anyhow::{bail, Result};
use core::fmt::Debug;
use grit_util::AnalysisLogs;

pub enum PatternResult {
/// The pattern matched
Matched,
/// The pattern did not match
NotMatched,
/// The pattern might match, but some error occurred during evaluation
Unknown,
}

impl From<bool> for PatternResult {
fn from(item: bool) -> Self {
if item {
PatternResult::Matched
} else {
PatternResult::NotMatched
}
}
}

pub trait Matcher<Q: QueryContext>: Debug {
// it is important that any implementors of Pattern
// do not compute-expensive things in execute
Expand All @@ -66,7 +85,7 @@ pub trait Matcher<Q: QueryContext>: Debug {
state: &mut State<'a, Q>,
context: &'a Q::ExecContext<'a>,
logs: &mut AnalysisLogs,
) -> Result<bool>;
) -> Result<PatternResult>;

// for the future:
// we could speed up computation by filtering on the sort of pattern
Expand Down
10 changes: 10 additions & 0 deletions execute.grit
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
language rust

`fn execute(
$_
) -> Result<$res> {
$body
}` where {
$res <: `bool` => `PatternResult`,
$body => ai_transform($body, "Rewrite this body so that it returns PatternResult::True or PatternResult::False, or PatternResult::Unknown if there's an error")
}