Skip to content

Commit

Permalink
fix: let LSP suggest fields and methods in LValue chains (noir-lang/n…
Browse files Browse the repository at this point in the history
…oir#6051)

feat(metaprogramming): Add `#[use_callers_scope]` (noir-lang/noir#6050)
feat: show test output when running via LSP (noir-lang/noir#6049)
feat: add `Expr::as_lambda` (noir-lang/noir#6048)
feat: faster LSP by caching file managers (noir-lang/noir#6047)
feat: implement `to_be_radix` in the comptime interpreter (noir-lang/noir#6043)
fix: prevent check_can_mutate crashing on undefined variable (noir-lang/noir#6044)
feat: add `Expr::as_for` and `Expr::as_for_range` (noir-lang/noir#6039)
fix: Fix canonicalization bug (noir-lang/noir#6033)
fix: Parse a statement as an expression (noir-lang/noir#6040)
chore: rename CustomAtrribute to CustomAttribute (noir-lang/noir#6038)
fix: error on `&mut x` when `x` is not mutable (noir-lang/noir#6037)
feat: add `Expr::as_constructor` (noir-lang/noir#5980)
chore: Fix docs (noir-lang/noir#6035)
chore: document array methods (noir-lang/noir#6034)
  • Loading branch information
AztecBot committed Sep 17, 2024
2 parents 7b76c32 + e5d6b18 commit 5bdaa62
Show file tree
Hide file tree
Showing 26 changed files with 704 additions and 64 deletions.
2 changes: 1 addition & 1 deletion .noir-sync-commit
Original file line number Diff line number Diff line change
@@ -1 +1 @@
abcae750f022dd7c49cee616edddd7b1cc93f3b8
5bf6567320629835ef6fa7765ca87e9b38ae4c9a
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,9 @@ impl<'context> Elaborator<'context> {
elaborator.function_context.push(FunctionContext::default());
elaborator.scopes.start_function();

elaborator.local_module = self.local_module;
elaborator.file = self.file;

setup(&mut elaborator);

elaborator.populate_scope_from_comptime_scopes();
Expand Down Expand Up @@ -316,7 +319,7 @@ impl<'context> Elaborator<'context> {
// If the function is varargs, push the type of the last slice element N times
// to account for N extra arguments.
let modifiers = interpreter.elaborator.interner.function_modifiers(&function);
let is_varargs = modifiers.attributes.is_varargs();
let is_varargs = modifiers.attributes.has_varargs();
let varargs_type = if is_varargs { parameters.pop() } else { None };

let varargs_elem_type = varargs_type.as_ref().and_then(|t| t.slice_element_type());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -301,12 +301,13 @@ impl<'context> Elaborator<'context> {
let expr = self.interner.expression(&expr_id);
match expr {
HirExpression::Ident(hir_ident, _) => {
let definition = self.interner.definition(hir_ident.id);
if !definition.mutable {
self.push_err(TypeCheckError::CannotMutateImmutableVariable {
name: definition.name.clone(),
span,
});
if let Some(definition) = self.interner.try_definition(hir_ident.id) {
if !definition.mutable {
self.push_err(TypeCheckError::CannotMutateImmutableVariable {
name: definition.name.clone(),
span,
});
}
}
}
HirExpression::MemberAccess(member_access) => {
Expand Down
11 changes: 10 additions & 1 deletion noir/noir-repo/compiler/noirc_frontend/src/elaborator/scope.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,11 +38,20 @@ impl<'context> Elaborator<'context> {
})
}

pub(super) fn module_id(&self) -> ModuleId {
pub fn module_id(&self) -> ModuleId {
assert_ne!(self.local_module, LocalModuleId::dummy_id(), "local_module is unset");
ModuleId { krate: self.crate_id, local_id: self.local_module }
}

pub fn replace_module(&mut self, new_module: ModuleId) -> ModuleId {
assert_ne!(new_module.local_id, LocalModuleId::dummy_id(), "local_module is unset");
let current_module = self.module_id();

self.crate_id = new_module.krate;
self.local_module = new_module.local_id;
current_module
}

pub(super) fn resolve_path_or_error(
&mut self,
path: Path,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -133,9 +133,14 @@ impl<'local, 'interner> Interpreter<'local, 'interner> {
return self.call_special(function, arguments, return_type, location);
}

// Wait until after call_special to set the current function so that builtin functions like
// `.as_type()` still call the resolver in the caller's scope.
let old_function = self.current_function.replace(function);
// Don't change the current function scope if we're in a #[use_callers_scope] function.
// This will affect where `Expression::resolve`, `Quoted::as_type`, and similar functions resolve.
let mut old_function = self.current_function;
let modifiers = self.elaborator.interner.function_modifiers(&function);
if !modifiers.attributes.has_use_callers_scope() {
self.current_function = Some(function);
}

let result = self.call_user_defined_function(function, arguments, location);
self.current_function = old_function;
result
Expand Down Expand Up @@ -242,6 +247,26 @@ impl<'local, 'interner> Interpreter<'local, 'interner> {
}

fn call_closure(
&mut self,
closure: HirLambda,
environment: Vec<Value>,
arguments: Vec<(Value, Location)>,
function_scope: Option<FuncId>,
module_scope: ModuleId,
call_location: Location,
) -> IResult<Value> {
// Set the closure's scope to that of the function it was originally evaluated in
let old_module = self.elaborator.replace_module(module_scope);
let old_function = std::mem::replace(&mut self.current_function, function_scope);

let result = self.call_closure_inner(closure, environment, arguments, call_location);

self.current_function = old_function;
self.elaborator.replace_module(old_module);
result
}

fn call_closure_inner(
&mut self,
closure: HirLambda,
environment: Vec<Value>,
Expand Down Expand Up @@ -1276,7 +1301,9 @@ impl<'local, 'interner> Interpreter<'local, 'interner> {
}
Ok(result)
}
Value::Closure(closure, env, _) => self.call_closure(closure, env, arguments, location),
Value::Closure(closure, env, _, function_scope, module_scope) => {
self.call_closure(closure, env, arguments, function_scope, module_scope, location)
}
value => {
let typ = value.get_type().into_owned();
Err(InterpreterError::NonFunctionCalled { typ, location })
Expand Down Expand Up @@ -1458,7 +1485,8 @@ impl<'local, 'interner> Interpreter<'local, 'interner> {
try_vecmap(&lambda.captures, |capture| self.lookup_id(capture.ident.id, location))?;

let typ = self.elaborator.interner.id_type(id).follow_bindings();
Ok(Value::Closure(lambda, environment, typ))
let module = self.elaborator.module_id();
Ok(Value::Closure(lambda, environment, typ, self.current_function, module))
}

fn evaluate_quote(&mut self, mut tokens: Tokens, expr_id: ExprId) -> IResult<Value> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,7 @@ impl<'local, 'context> Interpreter<'local, 'context> {
"expr_as_if" => expr_as_if(interner, arguments, return_type, location),
"expr_as_index" => expr_as_index(interner, arguments, return_type, location),
"expr_as_integer" => expr_as_integer(interner, arguments, return_type, location),
"expr_as_lambda" => expr_as_lambda(interner, arguments, return_type, location),
"expr_as_let" => expr_as_let(interner, arguments, return_type, location),
"expr_as_member_access" => {
expr_as_member_access(interner, arguments, return_type, location)
Expand Down Expand Up @@ -174,6 +175,7 @@ impl<'local, 'context> Interpreter<'local, 'context> {
"struct_def_module" => struct_def_module(self, arguments, location),
"struct_def_name" => struct_def_name(interner, arguments, location),
"struct_def_set_fields" => struct_def_set_fields(interner, arguments, location),
"to_be_radix" => to_be_radix(arguments, return_type, location),
"to_le_radix" => to_le_radix(arguments, return_type, location),
"to_be_radix" => to_be_radix(arguments, return_type, location),
"trait_constraint_eq" => trait_constraint_eq(arguments, location),
Expand Down Expand Up @@ -757,6 +759,21 @@ fn quoted_tokens(arguments: Vec<(Value, Location)>, location: Location) -> IResu
))
}

fn to_be_radix(
arguments: Vec<(Value, Location)>,
return_type: Type,
location: Location,
) -> IResult<Value> {
let le_radix_limbs = to_le_radix(arguments, return_type, location)?;

let Value::Array(limbs, typ) = le_radix_limbs else {
unreachable!("`to_le_radix` should always return an array");
};
let be_radix_limbs = limbs.into_iter().rev().collect();

Ok(Value::Array(be_radix_limbs, typ))
}

fn to_le_radix(
arguments: Vec<(Value, Location)>,
return_type: Type,
Expand Down Expand Up @@ -1631,6 +1648,68 @@ fn expr_as_integer(
})
}

// fn as_lambda(self) -> Option<([(Expr, Option<UnresolvedType>)], Option<UnresolvedType>, Expr)>
fn expr_as_lambda(
interner: &NodeInterner,
arguments: Vec<(Value, Location)>,
return_type: Type,
location: Location,
) -> IResult<Value> {
expr_as(interner, arguments, return_type.clone(), location, |expr| {
if let ExprValue::Expression(ExpressionKind::Lambda(lambda)) = expr {
// ([(Expr, Option<UnresolvedType>)], Option<UnresolvedType>, Expr)
let option_type = extract_option_generic_type(return_type);
let Type::Tuple(mut tuple_types) = option_type else {
panic!("Expected the return type option generic arg to be a tuple");
};
assert_eq!(tuple_types.len(), 3);

// Expr
tuple_types.pop().unwrap();

// Option<UnresolvedType>
let option_unresolved_type = tuple_types.pop().unwrap();

let parameters = lambda
.parameters
.into_iter()
.map(|(pattern, typ)| {
let pattern = Value::pattern(pattern);
let typ = if let UnresolvedTypeData::Unspecified = typ.typ {
None
} else {
Some(Value::UnresolvedType(typ.typ))
};
let typ = option(option_unresolved_type.clone(), typ).unwrap();
Value::Tuple(vec![pattern, typ])
})
.collect();
let parameters = Value::Slice(
parameters,
Type::Slice(Box::new(Type::Tuple(vec![
Type::Quoted(QuotedType::Expr),
Type::Quoted(QuotedType::UnresolvedType),
]))),
);

let return_type = lambda.return_type.typ;
let return_type = if let UnresolvedTypeData::Unspecified = return_type {
None
} else {
Some(return_type)
};
let return_type = return_type.map(Value::UnresolvedType);
let return_type = option(option_unresolved_type, return_type).ok()?;

let body = Value::expression(lambda.body.kind);

Some(Value::Tuple(vec![parameters, return_type, body]))
} else {
None
}
})
}

// fn as_let(self) -> Option<(Expr, Option<UnresolvedType>, Expr)>
fn expr_as_let(
interner: &NodeInterner,
Expand Down
22 changes: 9 additions & 13 deletions noir/noir-repo/compiler/noirc_frontend/src/hir/comptime/value.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,11 @@ pub enum Value {
FormatString(Rc<String>, Type),
CtString(Rc<String>),
Function(FuncId, Type, Rc<TypeBindings>),
Closure(HirLambda, Vec<Value>, Type),

// Closures also store their original scope (function & module)
// in case they use functions such as `Quoted::as_type` which require them.
Closure(HirLambda, Vec<Value>, Type, Option<FuncId>, ModuleId),

Tuple(Vec<Value>),
Struct(HashMap<Rc<String>, Value>, Type),
Pointer(Shared<Value>, /* auto_deref */ bool),
Expand Down Expand Up @@ -125,7 +129,7 @@ impl Value {
}
Value::FormatString(_, typ) => return Cow::Borrowed(typ),
Value::Function(_, typ, _) => return Cow::Borrowed(typ),
Value::Closure(_, _, typ) => return Cow::Borrowed(typ),
Value::Closure(_, _, typ, ..) => return Cow::Borrowed(typ),
Value::Tuple(fields) => {
Type::Tuple(vecmap(fields, |field| field.get_type().into_owned()))
}
Expand Down Expand Up @@ -221,11 +225,6 @@ impl Value {
interner.store_instantiation_bindings(expr_id, unwrap_rc(bindings));
ExpressionKind::Resolved(expr_id)
}
Value::Closure(_lambda, _env, _typ) => {
// TODO: How should a closure's environment be inlined?
let item = "Returning closures from a comptime fn".into();
return Err(InterpreterError::Unimplemented { item, location });
}
Value::Tuple(fields) => {
let fields = try_vecmap(fields, |field| field.into_expression(interner, location))?;
ExpressionKind::Tuple(fields)
Expand Down Expand Up @@ -293,6 +292,7 @@ impl Value {
| Value::Zeroed(_)
| Value::Type(_)
| Value::UnresolvedType(_)
| Value::Closure(..)
| Value::ModuleDefinition(_) => {
let typ = self.get_type().into_owned();
let value = self.display(interner).to_string();
Expand Down Expand Up @@ -370,11 +370,6 @@ impl Value {
interner.store_instantiation_bindings(expr_id, unwrap_rc(bindings));
return Ok(expr_id);
}
Value::Closure(_lambda, _env, _typ) => {
// TODO: How should a closure's environment be inlined?
let item = "Returning closures from a comptime fn".into();
return Err(InterpreterError::Unimplemented { item, location });
}
Value::Tuple(fields) => {
let fields =
try_vecmap(fields, |field| field.into_hir_expression(interner, location))?;
Expand Down Expand Up @@ -427,6 +422,7 @@ impl Value {
| Value::Zeroed(_)
| Value::Type(_)
| Value::UnresolvedType(_)
| Value::Closure(..)
| Value::ModuleDefinition(_) => {
let typ = self.get_type().into_owned();
let value = self.display(interner).to_string();
Expand Down Expand Up @@ -601,7 +597,7 @@ impl<'value, 'interner> Display for ValuePrinter<'value, 'interner> {
Value::CtString(value) => write!(f, "{value}"),
Value::FormatString(value, _) => write!(f, "{value}"),
Value::Function(..) => write!(f, "(function)"),
Value::Closure(_, _, _) => write!(f, "(closure)"),
Value::Closure(..) => write!(f, "(closure)"),
Value::Tuple(fields) => {
let fields = vecmap(fields, |field| field.display(self.interner).to_string());
write!(f, "({})", fields.join(", "))
Expand Down
15 changes: 14 additions & 1 deletion noir/noir-repo/compiler/noirc_frontend/src/lexer/token.rs
Original file line number Diff line number Diff line change
Expand Up @@ -698,9 +698,13 @@ impl Attributes {
self.function.as_ref().map_or(false, |func_attribute| func_attribute.is_no_predicates())
}

pub fn is_varargs(&self) -> bool {
pub fn has_varargs(&self) -> bool {
self.secondary.iter().any(|attr| matches!(attr, SecondaryAttribute::Varargs))
}

pub fn has_use_callers_scope(&self) -> bool {
self.secondary.iter().any(|attr| matches!(attr, SecondaryAttribute::UseCallersScope))
}
}

/// An Attribute can be either a Primary Attribute or a Secondary Attribute
Expand Down Expand Up @@ -799,6 +803,7 @@ impl Attribute {
))
}
["varargs"] => Attribute::Secondary(SecondaryAttribute::Varargs),
["use_callers_scope"] => Attribute::Secondary(SecondaryAttribute::UseCallersScope),
tokens => {
tokens.iter().try_for_each(|token| validate(token))?;
Attribute::Secondary(SecondaryAttribute::Custom(CustomAttribute {
Expand Down Expand Up @@ -915,6 +920,11 @@ pub enum SecondaryAttribute {

/// A variable-argument comptime function.
Varargs,

/// Treat any metaprogramming functions within this one as resolving
/// within the scope of the calling function/module rather than this one.
/// This affects functions such as `Expression::resolve` or `Quoted::as_type`.
UseCallersScope,
}

impl SecondaryAttribute {
Expand All @@ -937,6 +947,7 @@ impl SecondaryAttribute {
SecondaryAttribute::Custom(custom) => custom.name(),
SecondaryAttribute::Abi(_) => Some("abi".to_string()),
SecondaryAttribute::Varargs => Some("varargs".to_string()),
SecondaryAttribute::UseCallersScope => Some("use_callers_scope".to_string()),
}
}
}
Expand All @@ -954,6 +965,7 @@ impl fmt::Display for SecondaryAttribute {
SecondaryAttribute::Field(ref k) => write!(f, "#[field({k})]"),
SecondaryAttribute::Abi(ref k) => write!(f, "#[abi({k})]"),
SecondaryAttribute::Varargs => write!(f, "#[varargs]"),
SecondaryAttribute::UseCallersScope => write!(f, "#[use_callers_scope]"),
}
}
}
Expand Down Expand Up @@ -1003,6 +1015,7 @@ impl AsRef<str> for SecondaryAttribute {
SecondaryAttribute::ContractLibraryMethod => "",
SecondaryAttribute::Export => "",
SecondaryAttribute::Varargs => "",
SecondaryAttribute::UseCallersScope => "",
}
}
}
Expand Down
24 changes: 24 additions & 0 deletions noir/noir-repo/compiler/noirc_frontend/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3533,3 +3533,27 @@ fn cannot_mutate_immutable_variable_on_member_access() {

assert_eq!(name, "foo");
}

#[test]
fn does_not_crash_when_passing_mutable_undefined_variable() {
let src = r#"
fn main() {
mutate(&mut undefined);
}
fn mutate(foo: &mut Field) {
*foo = 1;
}
"#;

let errors = get_program_errors(src);
assert_eq!(errors.len(), 1);

let CompilationError::ResolverError(ResolverError::VariableNotDeclared { name, .. }) =
&errors[0].0
else {
panic!("Expected a VariableNotDeclared error");
};

assert_eq!(name, "undefined");
}
Loading

0 comments on commit 5bdaa62

Please sign in to comment.