Skip to content

Commit

Permalink
fix: lsp hover wasn't always working (noir-lang/noir#5515)
Browse files Browse the repository at this point in the history
fix: Mutability in the comptime interpreter (noir-lang/noir#5517)
chore: Noir version for Pedersen commitment and hash (noir-lang/noir#5431)
chore: add benchmark for ecdsa (noir-lang/noir#5113)
feat(nargo): Default expression width field in `Nargo.toml` (noir-lang/noir#5505)
chore: bump hardhat version to `2.22.6` (noir-lang/noir#5514)
chore: added regression test for check_for_underconstrained_values resolve bug (noir-lang/noir#5490)
feat: add debug codelens action (noir-lang/noir#5474)
chore(ci): remove bad colon in yaml file (noir-lang/noir#5520)
chore(ci): add workflow to run `nargo check` on external repos (noir-lang/noir#5355)
feat: add support for usage of `super` in import paths (noir-lang/noir#5502)
feat: Allow comptime attributes on traits & functions (noir-lang/noir#5496)
feat: skip reading values immediately after it being written into an array (noir-lang/noir#5449)
fix: Don't type error when calling certain trait impls in the interpreter (noir-lang/noir#5471)
feat: LSP hover (noir-lang/noir#5491)
chore: update typo PR script (noir-lang/noir#5488)
feat: Handle ACIR calls in the debugger (noir-lang/noir#5051)
feat: Add unquote function (noir-lang/noir#5497)
feat: Allow arguments to attribute functions (noir-lang/noir#5494)
  • Loading branch information
AztecBot committed Jul 16, 2024
2 parents c87edd2 + 35a5e9d commit 768114c
Show file tree
Hide file tree
Showing 9 changed files with 81 additions and 48 deletions.
2 changes: 1 addition & 1 deletion .noir-sync-commit
Original file line number Diff line number Diff line change
@@ -1 +1 @@
029584b7e936abf6eb8ebce043585725b3b93e72
951e821a585fe7e0697291cadd4d3c3aa49fd8e4
Original file line number Diff line number Diff line change
Expand Up @@ -241,6 +241,8 @@ impl<'a> Interpreter<'a> {
Ok(())
}
HirPattern::Mutable(pattern, _) => {
// Create a mutable reference to store to
let argument = Value::Pointer(Shared::new(argument), true);
self.define_pattern(pattern, typ, argument, location)
}
HirPattern::Tuple(pattern_fields, _) => match (argument, typ) {
Expand Down Expand Up @@ -334,8 +336,19 @@ impl<'a> Interpreter<'a> {
}
}

/// Evaluate an expression and return the result
/// Evaluate an expression and return the result.
/// This will automatically dereference a mutable variable if used.
pub fn evaluate(&mut self, id: ExprId) -> IResult<Value> {
match self.evaluate_no_dereference(id)? {
Value::Pointer(elem, true) => Ok(elem.borrow().clone()),
other => Ok(other),
}
}

/// Evaluating a mutable variable will dereference it automatically.
/// This function should be used when that is not desired - e.g. when
/// compiling a `&mut var` expression to grab the original reference.
fn evaluate_no_dereference(&mut self, id: ExprId) -> IResult<Value> {
match self.interner.expression(&id) {
HirExpression::Ident(ident, _) => self.evaluate_ident(ident, id),
HirExpression::Literal(literal) => self.evaluate_literal(literal, id),
Expand Down Expand Up @@ -592,7 +605,10 @@ impl<'a> Interpreter<'a> {
}

fn evaluate_prefix(&mut self, prefix: HirPrefixExpression, id: ExprId) -> IResult<Value> {
let rhs = self.evaluate(prefix.rhs)?;
let rhs = match prefix.operator {
UnaryOp::MutableReference => self.evaluate_no_dereference(prefix.rhs)?,
_ => self.evaluate(prefix.rhs)?,
};
self.evaluate_prefix_with_value(rhs, prefix.operator, id)
}

Expand Down Expand Up @@ -634,9 +650,17 @@ impl<'a> Interpreter<'a> {
Err(InterpreterError::InvalidValueForUnary { value, location, operator: "not" })
}
},
UnaryOp::MutableReference => Ok(Value::Pointer(Shared::new(rhs))),
UnaryOp::MutableReference => {
// If this is a mutable variable (auto_deref = true), turn this into an explicit
// mutable reference just by switching the value of `auto_deref`. Otherwise, wrap
// the value in a fresh reference.
match rhs {
Value::Pointer(elem, true) => Ok(Value::Pointer(elem, false)),
other => Ok(Value::Pointer(Shared::new(other), false)),
}
}
UnaryOp::Dereference { implicitly_added: _ } => match rhs {
Value::Pointer(element) => Ok(element.borrow().clone()),
Value::Pointer(element, _) => Ok(element.borrow().clone()),
value => {
let location = self.interner.expr_location(&id);
Err(InterpreterError::NonPointerDereferenced { value, location })
Expand Down Expand Up @@ -1303,7 +1327,7 @@ impl<'a> Interpreter<'a> {
HirLValue::Ident(ident, typ) => self.mutate(ident.id, rhs, ident.location),
HirLValue::Dereference { lvalue, element_type: _, location } => {
match self.evaluate_lvalue(&lvalue)? {
Value::Pointer(value) => {
Value::Pointer(value, _) => {
*value.borrow_mut() = rhs;
Ok(())
}
Expand Down Expand Up @@ -1353,10 +1377,13 @@ impl<'a> Interpreter<'a> {

fn evaluate_lvalue(&mut self, lvalue: &HirLValue) -> IResult<Value> {
match lvalue {
HirLValue::Ident(ident, _) => self.lookup(ident),
HirLValue::Ident(ident, _) => match self.lookup(ident)? {
Value::Pointer(elem, true) => Ok(elem.borrow().clone()),
other => Ok(other),
},
HirLValue::Dereference { lvalue, element_type: _, location } => {
match self.evaluate_lvalue(lvalue)? {
Value::Pointer(value) => Ok(value.borrow().clone()),
Value::Pointer(value, _) => Ok(value.borrow().clone()),
value => {
Err(InterpreterError::NonPointerDereferenced { value, location: *location })
}
Expand Down
12 changes: 12 additions & 0 deletions noir/noir-repo/compiler/noirc_frontend/src/hir/comptime/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,18 @@ fn mutating_mutable_references() {
assert_eq!(result, Value::I64(4));
}

#[test]
fn mutation_leaks() {
let program = "comptime fn main() -> pub i8 {
let mut x = 3;
let y = &mut x;
*y = 5;
x
}";
let result = interpret(program, vec!["main".into()]);
assert_eq!(result, Value::I8(5));
}

#[test]
fn mutating_arrays() {
let program = "comptime fn main() -> pub u8 {
Expand Down
10 changes: 5 additions & 5 deletions noir/noir-repo/compiler/noirc_frontend/src/hir/comptime/value.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ pub enum Value {
Closure(HirLambda, Vec<Value>, Type),
Tuple(Vec<Value>),
Struct(HashMap<Rc<String>, Value>, Type),
Pointer(Shared<Value>),
Pointer(Shared<Value>, /* auto_deref */ bool),
Array(Vector<Value>, Type),
Slice(Vector<Value>, Type),
Code(Rc<Tokens>),
Expand Down Expand Up @@ -79,7 +79,7 @@ impl Value {
Value::Slice(_, typ) => return Cow::Borrowed(typ),
Value::Code(_) => Type::Quoted(QuotedType::Quoted),
Value::StructDefinition(_) => Type::Quoted(QuotedType::StructDefinition),
Value::Pointer(element) => {
Value::Pointer(element, _) => {
let element = element.borrow().get_type().into_owned();
Type::MutableReference(Box::new(element))
}
Expand Down Expand Up @@ -199,7 +199,7 @@ impl Value {
}
};
}
Value::Pointer(_)
Value::Pointer(..)
| Value::StructDefinition(_)
| Value::TraitDefinition(_)
| Value::FunctionDefinition(_)
Expand Down Expand Up @@ -309,7 +309,7 @@ impl Value {
HirExpression::Literal(HirLiteral::Slice(HirArrayLiteral::Standard(elements)))
}
Value::Code(block) => HirExpression::Unquote(unwrap_rc(block)),
Value::Pointer(_)
Value::Pointer(..)
| Value::StructDefinition(_)
| Value::TraitDefinition(_)
| Value::FunctionDefinition(_)
Expand Down Expand Up @@ -400,7 +400,7 @@ impl Display for Value {
let fields = vecmap(fields, |(name, value)| format!("{}: {}", name, value));
write!(f, "{typename} {{ {} }}", fields.join(", "))
}
Value::Pointer(value) => write!(f, "&mut {}", value.borrow()),
Value::Pointer(value, _) => write!(f, "&mut {}", value.borrow()),
Value::Array(values, _) => {
let values = vecmap(values, ToString::to_string);
write!(f, "[{}]", values.join(", "))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -732,7 +732,11 @@ impl<'a> ModCollector<'a> {

context.def_interner.add_module_attributes(
mod_id,
ModuleAttributes { name: mod_name.0.contents.clone(), location: mod_location },
ModuleAttributes {
name: mod_name.0.contents.clone(),
location: mod_location,
parent: self.module_id,
},
);
}

Expand Down
5 changes: 5 additions & 0 deletions noir/noir-repo/compiler/noirc_frontend/src/node_interner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ const IMPL_SEARCH_RECURSION_LIMIT: u32 = 10;
pub struct ModuleAttributes {
pub name: String,
pub location: Location,
pub parent: LocalModuleId,
}

type StructAttributes = Vec<SecondaryAttribute>;
Expand Down Expand Up @@ -1016,6 +1017,10 @@ impl NodeInterner {
self.module_attributes.get(module_id)
}

pub fn try_module_parent(&self, module_id: &ModuleId) -> Option<LocalModuleId> {
self.try_module_attributes(module_id).map(|attrs| attrs.parent)
}

pub fn global_attributes(&self, global_id: &GlobalId) -> &[SecondaryAttribute] {
&self.global_attributes[global_id]
}
Expand Down
3 changes: 3 additions & 0 deletions noir/noir-repo/noir_stdlib/src/embedded_curve_ops.nr
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,9 @@ fn multi_scalar_mul_array_return<let N: u32>(points: [EmbeddedCurvePoint; N], sc
#[foreign(multi_scalar_mul)]
pub(crate) fn multi_scalar_mul_slice(points: [EmbeddedCurvePoint], scalars: [EmbeddedCurveScalar]) -> [Field; 3] {}

#[foreign(multi_scalar_mul)]
pub(crate) fn multi_scalar_mul_slice(points: [EmbeddedCurvePoint], scalars: [EmbeddedCurveScalar]) -> [Field; 3] {}

// docs:start:fixed_base_scalar_mul
pub fn fixed_base_scalar_mul(scalar: EmbeddedCurveScalar) -> EmbeddedCurvePoint
// docs:end:fixed_base_scalar_mul
Expand Down
37 changes: 14 additions & 23 deletions noir/noir-repo/tooling/lsp/src/requests/hover.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,17 +56,14 @@ fn format_reference(reference: ReferenceId, args: &ProcessRequestCallbackArgs) -
}
fn format_module(id: ModuleId, args: &ProcessRequestCallbackArgs) -> String {
let module_attributes = args.interner.module_attributes(&id);
let parent_module_id = args.def_maps[&id.krate].modules()[id.local_id.0].parent;

let mut string = String::new();
if let Some(parent_module_id) = parent_module_id {
if format_parent_module_from_module_id(
&ModuleId { krate: id.krate, local_id: parent_module_id },
args,
&mut string,
) {
string.push('\n');
}
if format_parent_module_from_module_id(
&ModuleId { krate: id.krate, local_id: module_attributes.parent },
args,
&mut string,
) {
string.push('\n');
}
string.push_str(" ");
string.push_str("mod ");
Expand Down Expand Up @@ -319,6 +316,7 @@ fn format_parent_module_from_module_id(
CrateId::Stdlib(_) => Some("std".to_string()),
CrateId::Dummy => None,
};

let wrote_crate = if let Some(crate_name) = crate_name {
string.push_str(" ");
string.push_str(&crate_name);
Expand All @@ -331,27 +329,20 @@ fn format_parent_module_from_module_id(
return wrote_crate;
};

let modules = args.def_maps[&module.krate].modules();
let module_data = &modules[module.local_id.0];

if wrote_crate {
string.push_str("::");
} else {
string.push_str(" ");
}

let mut segments = Vec::new();
let mut current_parent = module_data.parent;
while let Some(parent) = current_parent {
let parent_attributes = args
.interner
.try_module_attributes(&ModuleId { krate: module.krate, local_id: parent });
if let Some(parent_attributes) = parent_attributes {
segments.push(&parent_attributes.name);
current_parent = modules[module.local_id.0].parent;
} else {
break;
}
let mut current_attributes = module_attributes;
while let Some(parent_attributes) = args.interner.try_module_attributes(&ModuleId {
krate: module.krate,
local_id: current_attributes.parent,
}) {
segments.push(&parent_attributes.name);
current_attributes = parent_attributes;
}

for segment in segments.iter().rev() {
Expand Down
13 changes: 2 additions & 11 deletions noir/noir-repo/tooling/lsp/src/requests/mod.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,4 @@
use std::{
collections::{BTreeMap, HashMap},
future::Future,
};
use std::{collections::HashMap, future::Future};

use crate::{
parse_diff, resolve_workspace_for_source_path,
Expand All @@ -17,11 +14,7 @@ use lsp_types::{
use nargo::insert_all_files_for_workspace_into_file_manager;
use nargo_fmt::Config;
use noirc_driver::file_manager_with_stdlib;
use noirc_frontend::{
graph::{CrateId, Dependency},
hir::def_map::CrateDefMap,
macros_api::NodeInterner,
};
use noirc_frontend::{graph::Dependency, macros_api::NodeInterner};
use serde::{Deserialize, Serialize};

use crate::{
Expand Down Expand Up @@ -285,7 +278,6 @@ pub(crate) struct ProcessRequestCallbackArgs<'a> {
interners: &'a HashMap<String, NodeInterner>,
root_crate_name: String,
root_crate_dependencies: &'a Vec<Dependency>,
def_maps: &'a BTreeMap<CrateId, CrateDefMap>,
}

pub(crate) fn process_request<F, T>(
Expand Down Expand Up @@ -340,7 +332,6 @@ where
interners: &state.cached_definitions,
root_crate_name: package.name.to_string(),
root_crate_dependencies: &context.crate_graph[context.root_crate_id()].dependencies,
def_maps: &context.def_maps,
}))
}
pub(crate) fn find_all_references_in_workspace(
Expand Down

0 comments on commit 768114c

Please sign in to comment.