From 3d9d07210544c9d27051eb5e629585760f48cd1c Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Thu, 19 Sep 2024 08:06:53 -0300 Subject: [PATCH] feat: (LSP) show global value on hover (#6097) # Description ## Problem I noticed Rust Analyzer shows a `const` value, so I figured we could do the same thing with simple globals. ## Summary This is only done for globals with "simple" types (bools, strings, integers, arrays, tuples). It can be useful to avoid having to go to the global definition to check the value, in case it's needed. ![lsp-globals](https://github.com/user-attachments/assets/8886e78d-f18c-4b2a-939b-c3f98c242044) ## Additional Context ## Documentation Check one: - [x] No documentation needed. - [ ] Documentation included in this PR. - [ ] **[For Experimental Features]** Documentation to be submitted in a separate PR. # PR Checklist - [x] I have tested the changes locally. - [x] I have formatted the changes with [Prettier](https://prettier.io/) and/or `cargo fmt` on default settings. --- .../noirc_frontend/src/elaborator/types.rs | 166 +++++++++--------- tooling/lsp/src/requests/hover.rs | 96 +++++++++- 2 files changed, 175 insertions(+), 87 deletions(-) diff --git a/compiler/noirc_frontend/src/elaborator/types.rs b/compiler/noirc_frontend/src/elaborator/types.rs index f40b62f2f31..6be18df7b52 100644 --- a/compiler/noirc_frontend/src/elaborator/types.rs +++ b/compiler/noirc_frontend/src/elaborator/types.rs @@ -622,7 +622,7 @@ impl<'context> Elaborator<'context> { let length = stmt.expression; let span = self.interner.expr_span(&length); - let result = self.try_eval_array_length_id(length, span); + let result = try_eval_array_length_id(self.interner, length, span); match result.map(|length| length.try_into()) { Ok(Ok(length_value)) => return length_value, @@ -633,88 +633,6 @@ impl<'context> Elaborator<'context> { 0 } - fn try_eval_array_length_id( - &self, - rhs: ExprId, - span: Span, - ) -> Result> { - // Arbitrary amount of recursive calls to try before giving up - let fuel = 100; - self.try_eval_array_length_id_with_fuel(rhs, span, fuel) - } - - fn try_eval_array_length_id_with_fuel( - &self, - rhs: ExprId, - span: Span, - fuel: u32, - ) -> Result> { - if fuel == 0 { - // If we reach here, it is likely from evaluating cyclic globals. We expect an error to - // be issued for them after name resolution so issue no error now. - return Err(None); - } - - match self.interner.expression(&rhs) { - HirExpression::Literal(HirLiteral::Integer(int, false)) => { - int.try_into_u128().ok_or(Some(ResolverError::IntegerTooLarge { span })) - } - HirExpression::Ident(ident, _) => { - if let Some(definition) = self.interner.try_definition(ident.id) { - match definition.kind { - DefinitionKind::Global(global_id) => { - let let_statement = self.interner.get_global_let_statement(global_id); - if let Some(let_statement) = let_statement { - let expression = let_statement.expression; - self.try_eval_array_length_id_with_fuel(expression, span, fuel - 1) - } else { - Err(Some(ResolverError::InvalidArrayLengthExpr { span })) - } - } - _ => Err(Some(ResolverError::InvalidArrayLengthExpr { span })), - } - } else { - Err(Some(ResolverError::InvalidArrayLengthExpr { span })) - } - } - HirExpression::Infix(infix) => { - let lhs = self.try_eval_array_length_id_with_fuel(infix.lhs, span, fuel - 1)?; - let rhs = self.try_eval_array_length_id_with_fuel(infix.rhs, span, fuel - 1)?; - - match infix.operator.kind { - BinaryOpKind::Add => Ok(lhs + rhs), - BinaryOpKind::Subtract => Ok(lhs - rhs), - BinaryOpKind::Multiply => Ok(lhs * rhs), - BinaryOpKind::Divide => Ok(lhs / rhs), - BinaryOpKind::Equal => Ok((lhs == rhs) as u128), - BinaryOpKind::NotEqual => Ok((lhs != rhs) as u128), - BinaryOpKind::Less => Ok((lhs < rhs) as u128), - BinaryOpKind::LessEqual => Ok((lhs <= rhs) as u128), - BinaryOpKind::Greater => Ok((lhs > rhs) as u128), - BinaryOpKind::GreaterEqual => Ok((lhs >= rhs) as u128), - BinaryOpKind::And => Ok(lhs & rhs), - BinaryOpKind::Or => Ok(lhs | rhs), - BinaryOpKind::Xor => Ok(lhs ^ rhs), - BinaryOpKind::ShiftRight => Ok(lhs >> rhs), - BinaryOpKind::ShiftLeft => Ok(lhs << rhs), - BinaryOpKind::Modulo => Ok(lhs % rhs), - } - } - HirExpression::Cast(cast) => { - let lhs = self.try_eval_array_length_id_with_fuel(cast.lhs, span, fuel - 1)?; - let lhs_value = Value::Field(lhs.into()); - let evaluated_value = - Interpreter::evaluate_cast_one_step(&cast, rhs, lhs_value, self.interner) - .map_err(|error| Some(ResolverError::ArrayLengthInterpreter { error }))?; - - evaluated_value - .to_u128() - .ok_or_else(|| Some(ResolverError::InvalidArrayLengthExpr { span })) - } - _other => Err(Some(ResolverError::InvalidArrayLengthExpr { span })), - } - } - pub fn unify( &mut self, actual: &Type, @@ -1851,6 +1769,88 @@ impl<'context> Elaborator<'context> { } } +pub fn try_eval_array_length_id( + interner: &NodeInterner, + rhs: ExprId, + span: Span, +) -> Result> { + // Arbitrary amount of recursive calls to try before giving up + let fuel = 100; + try_eval_array_length_id_with_fuel(interner, rhs, span, fuel) +} + +fn try_eval_array_length_id_with_fuel( + interner: &NodeInterner, + rhs: ExprId, + span: Span, + fuel: u32, +) -> Result> { + if fuel == 0 { + // If we reach here, it is likely from evaluating cyclic globals. We expect an error to + // be issued for them after name resolution so issue no error now. + return Err(None); + } + + match interner.expression(&rhs) { + HirExpression::Literal(HirLiteral::Integer(int, false)) => { + int.try_into_u128().ok_or(Some(ResolverError::IntegerTooLarge { span })) + } + HirExpression::Ident(ident, _) => { + if let Some(definition) = interner.try_definition(ident.id) { + match definition.kind { + DefinitionKind::Global(global_id) => { + let let_statement = interner.get_global_let_statement(global_id); + if let Some(let_statement) = let_statement { + let expression = let_statement.expression; + try_eval_array_length_id_with_fuel(interner, expression, span, fuel - 1) + } else { + Err(Some(ResolverError::InvalidArrayLengthExpr { span })) + } + } + _ => Err(Some(ResolverError::InvalidArrayLengthExpr { span })), + } + } else { + Err(Some(ResolverError::InvalidArrayLengthExpr { span })) + } + } + HirExpression::Infix(infix) => { + let lhs = try_eval_array_length_id_with_fuel(interner, infix.lhs, span, fuel - 1)?; + let rhs = try_eval_array_length_id_with_fuel(interner, infix.rhs, span, fuel - 1)?; + + match infix.operator.kind { + BinaryOpKind::Add => Ok(lhs + rhs), + BinaryOpKind::Subtract => Ok(lhs - rhs), + BinaryOpKind::Multiply => Ok(lhs * rhs), + BinaryOpKind::Divide => Ok(lhs / rhs), + BinaryOpKind::Equal => Ok((lhs == rhs) as u128), + BinaryOpKind::NotEqual => Ok((lhs != rhs) as u128), + BinaryOpKind::Less => Ok((lhs < rhs) as u128), + BinaryOpKind::LessEqual => Ok((lhs <= rhs) as u128), + BinaryOpKind::Greater => Ok((lhs > rhs) as u128), + BinaryOpKind::GreaterEqual => Ok((lhs >= rhs) as u128), + BinaryOpKind::And => Ok(lhs & rhs), + BinaryOpKind::Or => Ok(lhs | rhs), + BinaryOpKind::Xor => Ok(lhs ^ rhs), + BinaryOpKind::ShiftRight => Ok(lhs >> rhs), + BinaryOpKind::ShiftLeft => Ok(lhs << rhs), + BinaryOpKind::Modulo => Ok(lhs % rhs), + } + } + HirExpression::Cast(cast) => { + let lhs = try_eval_array_length_id_with_fuel(interner, cast.lhs, span, fuel - 1)?; + let lhs_value = Value::Field(lhs.into()); + let evaluated_value = + Interpreter::evaluate_cast_one_step(&cast, rhs, lhs_value, interner) + .map_err(|error| Some(ResolverError::ArrayLengthInterpreter { error }))?; + + evaluated_value + .to_u128() + .ok_or_else(|| Some(ResolverError::InvalidArrayLengthExpr { span })) + } + _other => Err(Some(ResolverError::InvalidArrayLengthExpr { span })), + } +} + /// Gives an error if a user tries to create a mutable reference /// to an immutable variable. fn verify_mutable_reference(interner: &NodeInterner, rhs: ExprId) -> Result<(), ResolverError> { diff --git a/tooling/lsp/src/requests/hover.rs b/tooling/lsp/src/requests/hover.rs index edc71e94b08..46d2a5cfc8f 100644 --- a/tooling/lsp/src/requests/hover.rs +++ b/tooling/lsp/src/requests/hover.rs @@ -5,11 +5,12 @@ use fm::FileMap; use lsp_types::{Hover, HoverContents, HoverParams, MarkupContent, MarkupKind}; use noirc_frontend::{ ast::Visibility, + elaborator::types::try_eval_array_length_id, hir::def_map::ModuleId, - hir_def::{stmt::HirPattern, traits::Trait}, - macros_api::{NodeInterner, StructId}, + hir_def::{expr::HirArrayLiteral, stmt::HirPattern, traits::Trait}, + macros_api::{HirExpression, HirLiteral, NodeInterner, StructId}, node_interner::{ - DefinitionId, DefinitionKind, FuncId, GlobalId, ReferenceId, TraitId, TypeAliasId, + DefinitionId, DefinitionKind, ExprId, FuncId, GlobalId, ReferenceId, TraitId, TypeAliasId, }, Generics, Shared, StructType, Type, TypeAlias, TypeBinding, TypeVariable, }; @@ -166,17 +167,34 @@ fn format_trait(id: TraitId, args: &ProcessRequestCallbackArgs) -> String { fn format_global(id: GlobalId, args: &ProcessRequestCallbackArgs) -> String { let global_info = args.interner.get_global(id); let definition_id = global_info.definition_id; + let definition = args.interner.definition(definition_id); let typ = args.interner.definition_type(definition_id); let mut string = String::new(); if format_parent_module(ReferenceId::Global(id), args, &mut string) { string.push('\n'); } + string.push_str(" "); + if definition.comptime { + string.push_str("comptime "); + } + if definition.mutable { + string.push_str("mut "); + } string.push_str("global "); string.push_str(&global_info.ident.0.contents); string.push_str(": "); string.push_str(&format!("{}", typ)); + + // See if we can figure out what's the global's value + if let Some(stmt) = args.interner.get_global_let_statement(id) { + if let Some(value) = get_global_value(args.interner, stmt.expression) { + string.push_str(" = "); + string.push_str(&value); + } + } + string.push_str(&go_to_type_links(&typ, args.interner, args.files)); append_doc_comments(args.interner, ReferenceId::Global(id), &mut string); @@ -184,6 +202,72 @@ fn format_global(id: GlobalId, args: &ProcessRequestCallbackArgs) -> String { string } +fn get_global_value(interner: &NodeInterner, expr: ExprId) -> Option { + let span = interner.expr_span(&expr); + + // Globals as array lengths are extremely common, so we try that first. + if let Ok(result) = try_eval_array_length_id(interner, expr, span) { + return Some(result.to_string()); + } + + match interner.expression(&expr) { + HirExpression::Literal(literal) => match literal { + HirLiteral::Array(hir_array_literal) => { + get_global_array_value(interner, hir_array_literal, false) + } + HirLiteral::Slice(hir_array_literal) => { + get_global_array_value(interner, hir_array_literal, true) + } + HirLiteral::Bool(value) => Some(value.to_string()), + HirLiteral::Integer(field_element, _) => Some(field_element.to_string()), + HirLiteral::Str(string) => Some(format!("{:?}", string)), + HirLiteral::FmtStr(..) => None, + HirLiteral::Unit => Some("()".to_string()), + }, + HirExpression::Tuple(values) => { + get_exprs_global_value(interner, &values).map(|value| format!("({})", value)) + } + _ => None, + } +} + +fn get_global_array_value( + interner: &NodeInterner, + literal: HirArrayLiteral, + is_slice: bool, +) -> Option { + match literal { + HirArrayLiteral::Standard(values) => { + get_exprs_global_value(interner, &values).map(|value| { + if is_slice { + format!("&[{}]", value) + } else { + format!("[{}]", value) + } + }) + } + HirArrayLiteral::Repeated { repeated_element, length } => { + get_global_value(interner, repeated_element).map(|value| { + if is_slice { + format!("&[{}; {}]", value, length) + } else { + format!("[{}; {}]", value, length) + } + }) + } + } +} + +fn get_exprs_global_value(interner: &NodeInterner, exprs: &[ExprId]) -> Option { + let strings: Vec = + exprs.iter().filter_map(|value| get_global_value(interner, *value)).collect(); + if strings.len() == exprs.len() { + Some(strings.join(", ")) + } else { + None + } +} + fn format_function(id: FuncId, args: &ProcessRequestCallbackArgs) -> String { let func_meta = args.interner.function_meta(&id); let func_name_definition_id = args.interner.definition(func_meta.name.id); @@ -263,6 +347,10 @@ fn format_alias(id: TypeAliasId, args: &ProcessRequestCallbackArgs) -> String { fn format_local(id: DefinitionId, args: &ProcessRequestCallbackArgs) -> String { let definition_info = args.interner.definition(id); + if let DefinitionKind::Global(global_id) = &definition_info.kind { + return format_global(*global_id, args); + } + let DefinitionKind::Local(expr_id) = definition_info.kind else { panic!("Expected a local reference to reference a local definition") }; @@ -629,7 +717,7 @@ mod hover_tests { "two/src/lib.nr", Position { line: 15, character: 25 }, r#" one::subone - global some_global: Field"#, + global some_global: Field = 2"#, ) .await; }