Skip to content

Commit 627909b

Browse files
committed
Implement evaluation of relational values
Relational values were supported in aggregate contexts before where they were immediately eliminated. This change extends the planner and evaluator to be able to evaluate them more generally. This change comes with support for a new method: `Parameter.getName`. While this is not itself relational, it was impossible to use previously since the only way to get a parameter was `Method.getAParameter`. The combination of the two is now supported. This change required two major extensions: - The type checker now allows relational values in many more contexts - The planner supports evaluation of relational values as lists in some contexts All of this ended up requiring a refactoring that I was expecting. Many values that were previously boxed are now ref counted, as we are now constructing closures with strange lifetimes that can't just take ownership of their inputs. This change also includes a major fix to the integration tests. Before, errors weren't being detected because it was cleanly returning `Err`. Now it fails loudly with `unwrap`, which revealed that all of the tests were actually broken (for a silly reason).
1 parent ffcfbb1 commit 627909b

File tree

13 files changed

+398
-110
lines changed

13 files changed

+398
-110
lines changed

doc/library.kdl

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ type "Callable" {
4343

4444
type "Parameter" {
4545
method "getType" type="Type" status="Unimplemented"
46-
method "getName" type="string" status="Unimplemented"
46+
method "getName" type="string"
4747
method "getIndex" type="int" status="Unimplemented"
4848
}
4949

examples/4-argument-functions.ql

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
from Function f
2+
where count(f.getAParameter()) = 4
3+
select f

examples/param-name-eq.ql

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
from Function f
2+
where
3+
f.getAParameter().getName() == "buf"
4+
select f

src/evaluate.rs

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,9 +42,21 @@ fn evaluate_filter(target : &source_file::SourceFile, ctx : &mut EvaluationConte
4242
let s = (nm.extract)(ctx, target.source.as_bytes());
4343
Ok(Value::Constant(Constant::String_(s)))
4444
},
45+
NodeFilter::PredicateListComputation(nm) => {
46+
// We evaluate a list of predicates p as `any(p)` (i.e., it is true
47+
// if any of the predicates evaluates to true)
48+
let ps = (nm.extract)(ctx, target.source.as_bytes());
49+
Ok(Value::Constant(Constant::Boolean(ps.iter().any(|p| *p))))
50+
},
51+
NodeFilter::ArgumentComputation(_c) => {
52+
panic!("Not evaluation arguments yet");
53+
},
4554
NodeFilter::ArgumentListComputation(_c) => {
4655
panic!("No concrete list evaluation needed yet");
4756
},
57+
NodeFilter::StringListComputation(_c) => {
58+
panic!("No concrete list evaluation needed yet");
59+
},
4860
NodeFilter::CallableComputation(_c) => {
4961
panic!("Not evaluating callables yet");
5062
},

src/plan.rs

Lines changed: 206 additions & 35 deletions
Large diffs are not rendered by default.

src/plan/cpp.rs

Lines changed: 13 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,26 +1,21 @@
1+
use std::rc::Rc;
12
use tree_sitter::Node;
23

34
use crate::query::val_type::Type;
45
use crate::plan::interface::*;
56
use crate::source_file::SourceFile;
67

7-
pub struct CPPTreeInterface<'a> {
8-
file : &'a SourceFile
8+
pub struct CPPTreeInterface {
99
}
1010

11-
impl<'a> CPPTreeInterface<'a> {
12-
pub fn new(f : &'a SourceFile) -> Self {
11+
impl CPPTreeInterface {
12+
pub fn new(_f : &SourceFile) -> Self {
1313
CPPTreeInterface {
14-
file : f
1514
}
1615
}
1716
}
1817

19-
impl<'a> TreeInterface for CPPTreeInterface<'a> {
20-
fn source(&self) -> &'a SourceFile {
21-
self.file
22-
}
23-
18+
impl TreeInterface for CPPTreeInterface {
2419
fn top_level_type(&self, t : &Type) -> Option<TopLevelMatcher> {
2520
match t {
2621
Type::Function => {
@@ -35,11 +30,12 @@ impl<'a> TreeInterface for CPPTreeInterface<'a> {
3530
}
3631
}
3732

38-
fn callable_arguments(&self, base : NodeMatcher<CallableRef>) -> Option<NodeMatcher<Vec<FormalArgument>>>
33+
fn callable_arguments(&self, base : &NodeMatcher<CallableRef>) -> Option<NodeMatcher<Vec<FormalArgument>>>
3934
{
35+
let x = Rc::clone(&base.extract);
4036
let matcher = NodeMatcher {
41-
extract: Box::new(move |ctx, source| {
42-
let callable_ref = (base.extract)(ctx, source);
37+
extract: Rc::new(move |ctx, source| {
38+
let callable_ref = x(ctx, source);
4339
let node = ctx.lookup_callable(&callable_ref);
4440
let mut cur = tree_sitter::QueryCursor::new();
4541
let ql_query = "(parameter_declaration) @parameter";
@@ -52,11 +48,12 @@ impl<'a> TreeInterface for CPPTreeInterface<'a> {
5248
Some(matcher)
5349
}
5450

55-
fn callable_name(&self, base : NodeMatcher<CallableRef>) -> Option<NodeMatcher<String>>
51+
fn callable_name(&self, base : &NodeMatcher<CallableRef>) -> Option<NodeMatcher<String>>
5652
{
53+
let x = Rc::clone(&base.extract);
5754
let matcher = NodeMatcher {
58-
extract: Box::new(move |ctx, source| {
59-
let callable_ref = (base.extract)(ctx, source);
55+
extract: Rc::new(move |ctx, source| {
56+
let callable_ref = x(ctx, source);
6057
let node = ctx.lookup_callable(&callable_ref);
6158
let mut cur = tree_sitter::QueryCursor::new();
6259
let ql_query = "(function_declarator (identifier) @function.name)";

src/plan/errors.rs

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,6 @@ pub enum PlanError {
88
NonSingletonSelect(usize),
99
#[error("Unsupported target for a select expression: `{0:?}`")]
1010
UnsupportedSelectTarget(Expr_<Typed>),
11-
#[error("Unsupported type for a select expression: `{0:?}`")]
12-
UnsupportedSelectType(Type),
1311
#[error("Unsupported type `{0:?}` for language {1:?}")]
1412
UnsupportedTypeForLanguage(Type, Language),
1513
#[error("Use of undefined variable `{0}`")]

src/plan/interface.rs

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
use tree_sitter::Node;
22
use std::collections::HashMap;
3+
use std::rc::Rc;
34

45
use crate::query::val_type::Type;
5-
use crate::source_file::SourceFile;
66

77
pub struct TopLevelMatcher {
88
/// The Tree Sitter query to match the given "top-level" structure
@@ -109,13 +109,14 @@ impl<'a> EvaluationContext<'a> {
109109
/// FIXME: Add another field that records ranges to highlight in results (i.e.,
110110
/// the code that contributes to a Node being included)
111111
pub struct NodeMatcher<R> {
112-
pub extract : Box<dyn for <'a> Fn(&'a EvaluationContext<'a>, &'a [u8]) -> R>
112+
pub extract : Rc<dyn for <'a> Fn(&'a EvaluationContext<'a>, &'a [u8]) -> R>
113113
}
114114

115115
/// A representation of language-level types (e.g., Java or C types)
116116
///
117117
/// This is a type for clarity in the data model. For now it is just a `String`,
118118
/// but eventually we will probably want more structure
119+
#[derive(Clone)]
119120
pub struct LanguageType(String);
120121

121122
impl LanguageType {
@@ -134,6 +135,7 @@ impl LanguageType {
134135
/// arguments to indicate that they are unused (e.g., C).
135136
///
136137
/// The type is optional because some languages are untyped.
138+
#[derive(Clone)]
137139
pub struct FormalArgument {
138140
pub name : Option<String>,
139141
pub declared_type : Option<LanguageType>
@@ -148,9 +150,6 @@ pub struct FormalArgument {
148150
/// In cases where structure is sufficiently uniform, we should use normal
149151
/// functions instead.
150152
pub trait TreeInterface {
151-
/// The underlying source file
152-
fn source(&self) -> &SourceFile;
153-
154153
/// Return a matcher for a type declared in the From clause of a QL query
155154
///
156155
/// This can fail if the given type is not supported for the language
@@ -159,11 +158,11 @@ pub trait TreeInterface {
159158

160159
/// A node matcher that extracts formal arguments from a callable node
161160
/// (e.g., a method or function)
162-
fn callable_arguments(&self, node : NodeMatcher<CallableRef>) -> Option<NodeMatcher<Vec<FormalArgument>>>;
161+
fn callable_arguments(&self, node : &NodeMatcher<CallableRef>) -> Option<NodeMatcher<Vec<FormalArgument>>>;
163162

164163
/// A node matcher that extracts the name of a callable node.
165164
///
166165
/// This is tailored to just callables because getting the name of other
167166
/// types of nodes requires different patterns
168-
fn callable_name(&self, node : NodeMatcher<CallableRef>) -> Option<NodeMatcher<String>>;
167+
fn callable_name(&self, node : &NodeMatcher<CallableRef>) -> Option<NodeMatcher<String>>;
169168
}

src/plan/java.rs

Lines changed: 13 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,26 +1,21 @@
1+
use std::rc::Rc;
12
use tree_sitter::Node;
23

34
use crate::query::val_type::Type;
45
use crate::plan::interface::*;
56
use crate::source_file::SourceFile;
67

7-
pub struct JavaTreeInterface<'a> {
8-
file : &'a SourceFile
8+
pub struct JavaTreeInterface {
99
}
1010

11-
impl<'a> JavaTreeInterface<'a> {
12-
pub fn new(f : &'a SourceFile) -> Self {
11+
impl JavaTreeInterface {
12+
pub fn new(_f : &SourceFile) -> Self {
1313
JavaTreeInterface {
14-
file : f
1514
}
1615
}
1716
}
1817

19-
impl<'a> TreeInterface for JavaTreeInterface<'a> {
20-
fn source(&self) -> &'a SourceFile {
21-
self.file
22-
}
23-
18+
impl TreeInterface for JavaTreeInterface {
2419
// FIXME: If we associate the top-level query with the value of a var ref,
2520
// we get rid of var ref and just have a typed computation
2621
fn top_level_type(&self, t : &Type) -> Option<TopLevelMatcher> {
@@ -37,12 +32,13 @@ impl<'a> TreeInterface for JavaTreeInterface<'a> {
3732
}
3833
}
3934

40-
fn callable_arguments(&self, base : NodeMatcher<CallableRef>) ->
35+
fn callable_arguments(&self, base : &NodeMatcher<CallableRef>) ->
4136
Option<NodeMatcher<Vec<FormalArgument>>>
4237
{
38+
let x = Rc::clone(&base.extract);
4339
let matcher = NodeMatcher {
44-
extract: Box::new(move |ctx, source| {
45-
let callable_ref = (base.extract)(ctx, source);
40+
extract: Rc::new(move |ctx, source| {
41+
let callable_ref = x(ctx, source);
4642
let node = ctx.lookup_callable(&callable_ref);
4743
let mut cur = tree_sitter::QueryCursor::new();
4844
let ql_query = "(formal_parameter) @parameter";
@@ -55,11 +51,12 @@ impl<'a> TreeInterface for JavaTreeInterface<'a> {
5551
Some(matcher)
5652
}
5753

58-
fn callable_name(&self, base : NodeMatcher<CallableRef>) -> Option<NodeMatcher<String>>
54+
fn callable_name(&self, base : &NodeMatcher<CallableRef>) -> Option<NodeMatcher<String>>
5955
{
56+
let x = Rc::clone(&base.extract);
6057
let matcher = NodeMatcher {
61-
extract: Box::new(move |ctx, source| {
62-
let callable_ref = (base.extract)(ctx, source);
58+
extract: Rc::new(move |ctx, source| {
59+
let callable_ref = x(ctx, source);
6360
let node = ctx.lookup_callable(&callable_ref);
6461
let mut cur = tree_sitter::QueryCursor::new();
6562
let ql_query = "(method_declaration (identifier) @method.name)";

src/plan/method_library.rs

Lines changed: 42 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
use once_cell::sync::Lazy;
22
use std::collections::HashMap;
3+
use std::rc::Rc;
4+
use std::sync::Arc;
35

46
use crate::query::ir::*;
57
use crate::query::val_type::Type;
@@ -18,15 +20,17 @@ use crate::plan::interface::{NodeMatcher, TreeInterface};
1820
/// 3. The typed operands of the method call
1921
///
2022
/// The callback returns a node filter that is suitable for evaluating the method call
21-
pub struct Handler(pub Box<dyn for <'a> Fn(&'a Box<dyn TreeInterface + 'a>, NodeFilter, &'a Vec<Expr<Typed>>) -> anyhow::Result<NodeFilter> + Send + Sync>);
23+
///
24+
/// FIXME: The arguments need to be run-time values
25+
pub struct Handler(pub Arc<dyn for <'a> Fn(Rc<dyn TreeInterface>, &'a NodeFilter, &'a Vec<Expr<Typed>>) -> anyhow::Result<NodeFilter> + Send + Sync>);
2226

23-
fn callable_get_name<'a>(ti : &Box<dyn TreeInterface + 'a>, base : NodeFilter, operands : &'a Vec<Expr<Typed>>) -> anyhow::Result<NodeFilter> {
27+
fn callable_get_name<'a>(ti : Rc<dyn TreeInterface>, base : &'a NodeFilter, operands : &'a Vec<Expr<Typed>>) -> anyhow::Result<NodeFilter> {
2428
assert!(operands.is_empty());
2529
// This is not necessarily an assertion, as this may not be supported for
2630
// the current language (which we don't know).
2731
match base {
2832
NodeFilter::CallableComputation(callable_matcher) => {
29-
let name_matcher = ti.callable_name(callable_matcher)
33+
let name_matcher = ti.callable_name(&callable_matcher)
3034
.ok_or_else(|| PlanError::NotSupported("getNames".into(), "callable".into()))?;
3135
Ok(NodeFilter::StringComputation(name_matcher))
3236
},
@@ -37,11 +41,11 @@ fn callable_get_name<'a>(ti : &Box<dyn TreeInterface + 'a>, base : NodeFilter, o
3741
}
3842

3943
/// This is a relational method that returns a *list* of parameters
40-
fn callable_get_a_parameter<'a>(ti : &Box<dyn TreeInterface + 'a>, base : NodeFilter, operands : &'a Vec<Expr<Typed>>) -> anyhow::Result<NodeFilter> {
44+
fn callable_get_a_parameter<'a>(ti : Rc<dyn TreeInterface>, base : &'a NodeFilter, operands : &'a Vec<Expr<Typed>>) -> anyhow::Result<NodeFilter> {
4145
assert!(operands.is_empty());
4246
match base {
4347
NodeFilter::CallableComputation(callable_matcher) => {
44-
let arg_matcher = ti.callable_arguments(callable_matcher)
48+
let arg_matcher = ti.callable_arguments(&callable_matcher)
4549
.ok_or_else(|| PlanError::NotSupported("arguments".into(), "callable".into()))?;
4650
Ok(NodeFilter::ArgumentListComputation(arg_matcher))
4751
},
@@ -51,7 +55,7 @@ fn callable_get_a_parameter<'a>(ti : &Box<dyn TreeInterface + 'a>, base : NodeFi
5155
}
5256
}
5357

54-
fn string_regexp_match<'a>(_ti : &Box<dyn TreeInterface + 'a>, base : NodeFilter, operands : &'a Vec<Expr<Typed>>) -> anyhow::Result<NodeFilter> {
58+
fn string_regexp_match<'a>(_ti : Rc<dyn TreeInterface>, base : &'a NodeFilter, operands : &'a Vec<Expr<Typed>>) -> anyhow::Result<NodeFilter> {
5559
assert!(operands.len() == 1);
5660
// The base must be a string computation (and we require that it was a literal that we were able to already compile)
5761
let c = match base {
@@ -66,15 +70,34 @@ fn string_regexp_match<'a>(_ti : &Box<dyn TreeInterface + 'a>, base : NodeFilter
6670
panic!("Invalid regex for `regexpMatch`")
6771
}
6872
};
73+
let x = Rc::clone(&c.extract);
6974
let comp = NodeMatcher {
70-
extract: Box::new(move |ctx, source| {
71-
let matched_string = (c.extract)(ctx, source);
75+
extract: Rc::new(move |ctx, source| {
76+
let matched_string = x(ctx, source);
7277
rx.is_match(matched_string.as_ref())
7378
})
7479
};
7580
Ok(NodeFilter::Predicate(comp))
7681
}
7782

83+
fn parameter_get_name<'a>(_ti : Rc<dyn TreeInterface>, base : &'a NodeFilter, operands : &'a Vec<Expr<Typed>>) -> anyhow::Result<NodeFilter> {
84+
assert!(operands.is_empty());
85+
match base {
86+
NodeFilter::ArgumentComputation(c) => {
87+
let x = Rc::clone(&c.extract);
88+
let comp = NodeMatcher {
89+
extract: Rc::new(move |ctx, source| {
90+
x(ctx, source).name.unwrap_or("<none>".into())
91+
})
92+
};
93+
Ok(NodeFilter::StringComputation(comp))
94+
},
95+
_ => {
96+
panic!("Invalid base value for Parameter.getName");
97+
}
98+
}
99+
}
100+
78101
/// Validate the implementations of method calls against the claims in the
79102
/// library documentation. The intent is that the library documentation should
80103
/// always correctly reflect what subset of CodeQL ql-grep supports. Any
@@ -132,21 +155,23 @@ fn validate_library(impls : &HashMap<(Type, String), Handler>) {
132155
static METHOD_IMPLS: Lazy<HashMap<(Type, String), Handler>> = Lazy::new(|| {
133156
let mut impls = HashMap::new();
134157

135-
impls.insert((Type::Method, "getName".into()), Handler(Box::new(callable_get_name)));
136-
impls.insert((Type::Function, "getName".into()), Handler(Box::new(callable_get_name)));
137-
impls.insert((Type::Callable, "getName".into()), Handler(Box::new(callable_get_name)));
158+
impls.insert((Type::Method, "getName".into()), Handler(Arc::new(callable_get_name)));
159+
impls.insert((Type::Function, "getName".into()), Handler(Arc::new(callable_get_name)));
160+
impls.insert((Type::Callable, "getName".into()), Handler(Arc::new(callable_get_name)));
161+
162+
impls.insert((Type::Method, "getAParameter".into()), Handler(Arc::new(callable_get_a_parameter)));
163+
impls.insert((Type::Function, "getAParameter".into()), Handler(Arc::new(callable_get_a_parameter)));
164+
impls.insert((Type::Callable, "getAParameter".into()), Handler(Arc::new(callable_get_a_parameter)));
138165

139-
impls.insert((Type::Method, "getAParameter".into()), Handler(Box::new(callable_get_a_parameter)));
140-
impls.insert((Type::Function, "getAParameter".into()), Handler(Box::new(callable_get_a_parameter)));
141-
impls.insert((Type::Callable, "getAParameter".into()), Handler(Box::new(callable_get_a_parameter)));
166+
impls.insert((Type::Parameter, "getName".into()), Handler(Arc::new(parameter_get_name)));
142167

143-
impls.insert((Type::PrimString, "regexpMatch".into()), Handler(Box::new(string_regexp_match)));
168+
impls.insert((Type::PrimString, "regexpMatch".into()), Handler(Arc::new(string_regexp_match)));
144169

145170
validate_library(&impls);
146171

147172
impls
148173
});
149174

150-
pub fn method_impl_for(base_type : Type, method_name : &str) -> Option<&Handler> {
151-
Lazy::force(&METHOD_IMPLS).get(&(base_type, method_name.into()))
175+
pub fn method_impl_for(base_type : Type, method_name : String) -> Option<&'static Handler> {
176+
Lazy::force(&METHOD_IMPLS).get(&(base_type, method_name))
152177
}

0 commit comments

Comments
 (0)