From 71174f6626cbf100a8428ddc334681e4edfb45e6 Mon Sep 17 00:00:00 2001 From: Vincent Ambo Date: Tue, 20 Dec 2022 17:22:56 +0300 Subject: fix(tvix/eval): fix current clippy warnings It's been a while since the last time, so quite a lot of stuff has accumulated here. Change-Id: I0762827c197b30a917ff470fd8ae8f220f6ba247 Reviewed-on: https://cl.tvl.fyi/c/depot/+/7597 Reviewed-by: grfn Autosubmit: tazjin Tested-by: BuildkiteCI --- tvix/eval/src/builtins/mod.rs | 17 +++--- tvix/eval/src/compiler/bindings.rs | 3 +- tvix/eval/src/compiler/mod.rs | 6 +-- tvix/eval/src/errors.rs | 13 +++-- tvix/eval/src/lib.rs | 2 +- tvix/eval/src/nix_search_path.rs | 1 - tvix/eval/src/pretty_ast.rs | 62 +++++++--------------- tvix/eval/src/systems.rs | 7 +-- tvix/eval/src/tests/mod.rs | 14 +++-- .../nix_tests/notyetpassing/eval-okay-readDir.nix | 1 - .../notyetpassing/eval-okay-readDir.nix.disabled | 1 + tvix/eval/src/value/attrs.rs | 1 - tvix/eval/src/value/function.rs | 2 +- tvix/eval/src/value/mod.rs | 4 +- tvix/eval/src/value/string.rs | 2 +- tvix/eval/src/value/thunk.rs | 10 ++-- tvix/eval/src/vm.rs | 14 +++-- 17 files changed, 59 insertions(+), 101 deletions(-) delete mode 100644 tvix/eval/src/tests/nix_tests/notyetpassing/eval-okay-readDir.nix create mode 100644 tvix/eval/src/tests/nix_tests/notyetpassing/eval-okay-readDir.nix.disabled (limited to 'tvix/eval/src') diff --git a/tvix/eval/src/builtins/mod.rs b/tvix/eval/src/builtins/mod.rs index e4e9c14df6b8..3c635eb364ca 100644 --- a/tvix/eval/src/builtins/mod.rs +++ b/tvix/eval/src/builtins/mod.rs @@ -414,7 +414,7 @@ mod pure_builtins { let mut res: BTreeMap> = BTreeMap::new(); for val in list.to_list()? { let key = vm.call_with(&f, [val.clone()])?.force(vm)?.to_str()?; - res.entry(key).or_insert_with(|| vec![]).push(val); + res.entry(key).or_insert_with(std::vec::Vec::new).push(val); } Ok(Value::attrs(NixAttrs::from_iter( res.into_iter() @@ -688,7 +688,7 @@ mod pure_builtins { // We already applied a from->to with an empty from // transformation. // Let's skip it so that we don't loop infinitely - if empty_string_replace && from.as_str().len() == 0 { + if empty_string_replace && from.as_str().is_empty() { continue; } @@ -698,7 +698,7 @@ mod pure_builtins { i += from.len(); // remember if we applied the empty from->to - empty_string_replace = from.as_str().len() == 0; + empty_string_replace = from.as_str().is_empty(); continue 'outer; } @@ -719,7 +719,7 @@ mod pure_builtins { let from = elem.0.to_str()?; let to = elem.1.to_str()?; - if from.as_str().len() == 0 { + if from.as_str().is_empty() { res += &to; break; } @@ -866,9 +866,7 @@ mod pure_builtins { let len = len as usize; let end = cmp::min(beg + len, x.as_str().len()); - Ok(Value::String( - x.as_str()[(beg as usize)..(end as usize)].into(), - )) + Ok(Value::String(x.as_str()[beg..end].into())) } #[builtin("tail")] @@ -1070,10 +1068,7 @@ pub fn global_builtins(source: SourceCode) -> GlobalsMapFunc { // We need to insert import into the builtins, but the // builtins passed to import must have import *in it*. - let import = Value::Builtin(crate::builtins::impure::builtins_import( - globals, - source.clone(), - )); + let import = Value::Builtin(crate::builtins::impure::builtins_import(globals, source)); map.insert("import", import); }; diff --git a/tvix/eval/src/compiler/bindings.rs b/tvix/eval/src/compiler/bindings.rs index 31ab76aee815..12eaae5c191c 100644 --- a/tvix/eval/src/compiler/bindings.rs +++ b/tvix/eval/src/compiler/bindings.rs @@ -804,7 +804,7 @@ impl Compiler<'_> { } if let Some(ast::InterpolPart::Literal(lit)) = parts.pop() { - return Some(SmolStr::new(&lit)); + return Some(SmolStr::new(lit)); } None @@ -812,7 +812,6 @@ impl Compiler<'_> { /// Convert the provided `ast::Attr` into a statically known string if /// possible. - // TODO(tazjin): these should probably be SmolStr fn expr_static_attr_str(&self, node: &ast::Attr) -> Option { match node { ast::Attr::Ident(ident) => Some(ident.ident_token().unwrap().text().into()), diff --git a/tvix/eval/src/compiler/mod.rs b/tvix/eval/src/compiler/mod.rs index ae31d2714513..12fd269c2f9b 100644 --- a/tvix/eval/src/compiler/mod.rs +++ b/tvix/eval/src/compiler/mod.rs @@ -558,7 +558,7 @@ impl Compiler<'_> { self.emit_force(s); } - ast::Attr::Ident(ident) => self.emit_literal_ident(&ident), + ast::Attr::Ident(ident) => self.emit_literal_ident(ident), } } @@ -1232,7 +1232,7 @@ pub fn compile( let root_span = c.span_for(expr); let root_slot = c.scope_mut().declare_phantom(root_span, false); - c.compile(root_slot, &expr); + c.compile(root_slot, expr); // The final operation of any top-level Nix program must always be // `OpForce`. A thunk should not be returned to the user in an @@ -1247,6 +1247,6 @@ pub fn compile( lambda, warnings: c.warnings, errors: c.errors, - globals: globals, + globals, }) } diff --git a/tvix/eval/src/errors.rs b/tvix/eval/src/errors.rs index 92ec7b079941..0d12f259a5b7 100644 --- a/tvix/eval/src/errors.rs +++ b/tvix/eval/src/errors.rs @@ -539,8 +539,8 @@ fn spans_for_parse_errors(file: &File, errors: &[rnix::parser::ParseError]) -> V span, format!( "found '{}', but expected {}", - name_for_syntax(&found), - expected_syntax(&wanted), + name_for_syntax(found), + expected_syntax(wanted), ), ) } @@ -565,7 +565,7 @@ fn spans_for_parse_errors(file: &File, errors: &[rnix::parser::ParseError]) -> V file.span, format!( "code ended unexpectedly, but wanted {}", - expected_syntax(&wanted) + expected_syntax(wanted) ), ) } @@ -580,9 +580,8 @@ fn spans_for_parse_errors(file: &File, errors: &[rnix::parser::ParseError]) -> V rnix::parser::ParseError::RecursionLimitExceeded => ( file.span, - format!( - "this code exceeds the parser's recursion limit, please report a Tvix bug" - ), + "this code exceeds the parser's recursion limit, please report a Tvix bug" + .to_string(), ), // TODO: can rnix even still throw this? it's semantic! @@ -727,7 +726,7 @@ impl Error { fn spans(&self, source: &SourceCode) -> Vec { match &self.kind { ErrorKind::ImportParseError { errors, file, .. } => { - spans_for_parse_errors(&file, errors) + spans_for_parse_errors(file, errors) } ErrorKind::ParseErrors(errors) => { diff --git a/tvix/eval/src/lib.rs b/tvix/eval/src/lib.rs index bf6c01426289..32624b2918a2 100644 --- a/tvix/eval/src/lib.rs +++ b/tvix/eval/src/lib.rs @@ -223,7 +223,7 @@ impl<'code, 'co, 'ro> Evaluation<'code, 'co, 'ro> { None } }) - .unwrap_or_else(|| Default::default()); + .unwrap_or_default(); let runtime_observer = self.runtime_observer.take().unwrap_or(&mut noop_observer); let vm_result = run_lambda( diff --git a/tvix/eval/src/nix_search_path.rs b/tvix/eval/src/nix_search_path.rs index 8295aaa0ef91..3de773bb22a2 100644 --- a/tvix/eval/src/nix_search_path.rs +++ b/tvix/eval/src/nix_search_path.rs @@ -92,7 +92,6 @@ pub struct NixSearchPath { impl NixSearchPath { /// Attempt to resolve the given `path` within this [`NixSearchPath`] using the /// path resolution rules for `<...>`-style paths - #[allow(dead_code)] // TODO(grfn) pub fn resolve

(&self, path: P) -> Result where P: AsRef, diff --git a/tvix/eval/src/pretty_ast.rs b/tvix/eval/src/pretty_ast.rs index a829be26d814..5ac115e21c89 100644 --- a/tvix/eval/src/pretty_ast.rs +++ b/tvix/eval/src/pretty_ast.rs @@ -74,7 +74,7 @@ impl<'a> Serialize for SerializeAST<&'a ast::Select> { } } -impl<'a> Serialize for SerializeAST> { +impl Serialize for SerializeAST> { fn serialize(&self, serializer: S) -> Result { match &self.0 { ast::InterpolPart::Literal(s) => Serialize::serialize(s, serializer), @@ -96,7 +96,7 @@ impl<'a> Serialize for SerializeAST<&'a ast::Str> { .0 .normalized_parts() .into_iter() - .map(|part| SerializeAST(part)) + .map(SerializeAST) .collect::>(), )?; @@ -104,7 +104,7 @@ impl<'a> Serialize for SerializeAST<&'a ast::Str> { } } -impl<'a> Serialize for SerializeAST> { +impl Serialize for SerializeAST> { fn serialize(&self, serializer: S) -> Result { match &self.0 { ast::InterpolPart::Literal(p) => Serialize::serialize(p.syntax().text(), serializer), @@ -122,11 +122,7 @@ impl<'a> Serialize for SerializeAST<&'a ast::Path> { map.serialize_entry( "parts", - &self - .0 - .parts() - .map(|part| SerializeAST(part)) - .collect::>(), + &self.0.parts().map(SerializeAST).collect::>(), )?; map.end() @@ -148,7 +144,7 @@ impl<'a> Serialize for SerializeAST<&'a ast::Literal> { } } -impl<'a> Serialize for SerializeAST { +impl Serialize for SerializeAST { fn serialize(&self, serializer: S) -> Result { let mut map = serializer.serialize_map(None)?; map.serialize_entry("ident", &SerializeAST(&self.0.ident().unwrap()))?; @@ -161,7 +157,7 @@ impl<'a> Serialize for SerializeAST { } } -impl<'a> Serialize for SerializeAST { +impl Serialize for SerializeAST { fn serialize(&self, serializer: S) -> Result { match &self.0 { ast::Param::Pattern(pat) => { @@ -170,9 +166,7 @@ impl<'a> Serialize for SerializeAST { map.serialize_entry( "entries", - &pat.pat_entries() - .map(|entry| SerializeAST(entry)) - .collect::>(), + &pat.pat_entries().map(SerializeAST).collect::>(), )?; if let Some(bind) = pat.pat_bind() { @@ -211,17 +205,13 @@ impl<'a> Serialize for SerializeAST<&'a ast::LegacyLet> { &self .0 .attrpath_values() - .map(|val| SerializeAST(val)) + .map(SerializeAST) .collect::>(), )?; map.serialize_entry( "inherits", - &self - .0 - .inherits() - .map(|val| SerializeAST(val)) - .collect::>(), + &self.0.inherits().map(SerializeAST).collect::>(), )?; map.end() @@ -238,17 +228,13 @@ impl<'a> Serialize for SerializeAST<&'a ast::LetIn> { &self .0 .attrpath_values() - .map(|val| SerializeAST(val)) + .map(SerializeAST) .collect::>(), )?; map.serialize_entry( "inherits", - &self - .0 - .inherits() - .map(|val| SerializeAST(val)) - .collect::>(), + &self.0.inherits().map(SerializeAST).collect::>(), )?; map.serialize_entry("body", &SerializeAST(&self.0.body().unwrap()))?; @@ -258,11 +244,7 @@ impl<'a> Serialize for SerializeAST<&'a ast::LetIn> { impl<'a> Serialize for SerializeAST<&'a ast::List> { fn serialize(&self, serializer: S) -> Result { - let list = self - .0 - .items() - .map(|elem| SerializeAST(elem)) - .collect::>(); + let list = self.0.items().map(SerializeAST).collect::>(); let mut map = serializer.serialize_map(Some(2))?; map.serialize_entry("kind", "list")?; @@ -322,7 +304,7 @@ impl<'a> Serialize for SerializeAST<&'a ast::Root> { } } -impl<'a> Serialize for SerializeAST { +impl Serialize for SerializeAST { fn serialize(&self, serializer: S) -> Result { let mut map = serializer.serialize_map(Some(2))?; map.serialize_entry("name", &SerializeAST(self.0.attrpath().unwrap()))?; @@ -331,7 +313,7 @@ impl<'a> Serialize for SerializeAST { } } -impl<'a> Serialize for SerializeAST { +impl Serialize for SerializeAST { fn serialize(&self, serializer: S) -> Result { let mut map = serializer.serialize_map(None)?; @@ -341,7 +323,7 @@ impl<'a> Serialize for SerializeAST { map.serialize_entry( "names", - &self.0.attrs().map(|a| SerializeAST(a)).collect::>(), + &self.0.attrs().map(SerializeAST).collect::>(), )?; map.end() @@ -359,17 +341,13 @@ impl<'a> Serialize for SerializeAST<&'a ast::AttrSet> { &self .0 .attrpath_values() - .map(|val| SerializeAST(val)) + .map(SerializeAST) .collect::>(), )?; map.serialize_entry( "inherits", - &self - .0 - .inherits() - .map(|val| SerializeAST(val)) - .collect::>(), + &self.0.inherits().map(SerializeAST).collect::>(), )?; map.end() @@ -439,11 +417,7 @@ impl Serialize for SerializeAST { map.serialize_entry( "path", - &self - .0 - .attrs() - .map(|attr| SerializeAST(attr)) - .collect::>(), + &self.0.attrs().map(SerializeAST).collect::>(), )?; map.end() diff --git a/tvix/eval/src/systems.rs b/tvix/eval/src/systems.rs index cfdbe2eed505..16386cb9e0ad 100644 --- a/tvix/eval/src/systems.rs +++ b/tvix/eval/src/systems.rs @@ -1,10 +1,7 @@ /// true iff the argument is recognized by cppnix as the second /// coordinate of a "nix double" fn is_second_coordinate(x: &str) -> bool { - match x { - "linux" | "darwin" | "netbsd" | "openbsd" | "freebsd" => true, - _ => false, - } + matches!(x, "linux" | "darwin" | "netbsd" | "openbsd" | "freebsd") } /// This function takes an llvm triple (which may have three or four @@ -16,7 +13,7 @@ pub fn llvm_triple_to_nix_double(llvm_triple: &str) -> String { let cpu = match parts[0] { "armv6" => "armv6l", // cppnix appends an "l" to armv6 "armv7" => "armv7l", // cppnix appends an "l" to armv7 - x => match x.as_bytes().as_ref() { + x => match x.as_bytes() { [b'i', _, b'8', b'6'] => "i686", // cppnix glob-matches against i*86 _ => x, }, diff --git a/tvix/eval/src/tests/mod.rs b/tvix/eval/src/tests/mod.rs index 1c872f67d2b0..bb46cf2b79b0 100644 --- a/tvix/eval/src/tests/mod.rs +++ b/tvix/eval/src/tests/mod.rs @@ -49,15 +49,13 @@ fn eval_test(code_path: &str, expect_success: bool) { "{code_path}: test passed unexpectedly! consider moving it out of notyetpassing" ); } + } else if expect_success { + panic!("{code_path}: should be able to read test expectation"); } else { - if expect_success { - panic!("{code_path}: should be able to read test expectation"); - } else { - panic!( - "{code_path}: test should have failed, but succeeded with output {}", - result_str - ); - } + panic!( + "{code_path}: test should have failed, but succeeded with output {}", + result_str + ); } } diff --git a/tvix/eval/src/tests/nix_tests/notyetpassing/eval-okay-readDir.nix b/tvix/eval/src/tests/nix_tests/notyetpassing/eval-okay-readDir.nix deleted file mode 100644 index a7ec9292aae2..000000000000 --- a/tvix/eval/src/tests/nix_tests/notyetpassing/eval-okay-readDir.nix +++ /dev/null @@ -1 +0,0 @@ -builtins.readDir ./readDir diff --git a/tvix/eval/src/tests/nix_tests/notyetpassing/eval-okay-readDir.nix.disabled b/tvix/eval/src/tests/nix_tests/notyetpassing/eval-okay-readDir.nix.disabled new file mode 100644 index 000000000000..a7ec9292aae2 --- /dev/null +++ b/tvix/eval/src/tests/nix_tests/notyetpassing/eval-okay-readDir.nix.disabled @@ -0,0 +1 @@ +builtins.readDir ./readDir diff --git a/tvix/eval/src/value/attrs.rs b/tvix/eval/src/value/attrs.rs index ecce34fb4af4..833cb2fd6045 100644 --- a/tvix/eval/src/value/attrs.rs +++ b/tvix/eval/src/value/attrs.rs @@ -321,7 +321,6 @@ impl NixAttrs { } } - // TODO(tazjin): extend_reserve(count) (rust#72631) let mut attrs = NixAttrs(AttrsRep::Map(BTreeMap::new())); for _ in 0..count { diff --git a/tvix/eval/src/value/function.rs b/tvix/eval/src/value/function.rs index e31bb92b7931..1a32795393bf 100644 --- a/tvix/eval/src/value/function.rs +++ b/tvix/eval/src/value/function.rs @@ -30,7 +30,7 @@ impl Formals { Q: ?Sized + Hash + Eq, NixString: std::borrow::Borrow, { - self.ellipsis || self.arguments.contains_key(&arg) + self.ellipsis || self.arguments.contains_key(arg) } } diff --git a/tvix/eval/src/value/mod.rs b/tvix/eval/src/value/mod.rs index 124ce881d9a1..34dbf06231d4 100644 --- a/tvix/eval/src/value/mod.rs +++ b/tvix/eval/src/value/mod.rs @@ -217,7 +217,7 @@ impl Value { if let Value::Thunk(t) = f { t.force(vm)?; let guard = t.value(); - call_to_string(&*guard, vm) + call_to_string(&guard, vm) } else { call_to_string(f, vm) } @@ -451,7 +451,7 @@ impl Value { if let Some(name) = &f.lambda.name { format!("the user-defined Nix function '{}'", name) } else { - format!("a user-defined Nix function") + "a user-defined Nix function".to_string() } } diff --git a/tvix/eval/src/value/string.rs b/tvix/eval/src/value/string.rs index 66697a7f2f4f..5b2cb83a17de 100644 --- a/tvix/eval/src/value/string.rs +++ b/tvix/eval/src/value/string.rs @@ -172,7 +172,7 @@ fn is_valid_nix_identifier(s: &str) -> bool { _ => return false, } } - return true; + true } /// Escape a Nix string for display, as most user-visible representation diff --git a/tvix/eval/src/value/thunk.rs b/tvix/eval/src/value/thunk.rs index c7cdfa244183..8813b0039888 100644 --- a/tvix/eval/src/value/thunk.rs +++ b/tvix/eval/src/value/thunk.rs @@ -143,7 +143,7 @@ impl Thunk { }) => vm.enter_frame(lambda, upvalues, arg_count)?, } match trampoline.continuation { - None => break (), + None => break, Some(cont) => { trampoline = cont(vm)?; continue; @@ -203,7 +203,7 @@ impl Thunk { return Ok(Trampoline { action: Some(TrampolineAction::EnterFrame { lambda, - upvalues: upvalues.clone(), + upvalues, arg_count: 0, light_span: light_span.clone(), }), @@ -212,10 +212,10 @@ impl Thunk { self_clone.0.replace(ThunkRepr::Evaluated(vm.pop())); assert!(matches!(should_be_blackhole, ThunkRepr::Blackhole)); vm.push(Value::Thunk(self_clone)); - return Self::force_trampoline(vm).map_err(|kind| Error { + Self::force_trampoline(vm).map_err(|kind| Error { kind, span: light_span.span(), - }); + }) })), }); } @@ -268,7 +268,7 @@ impl Thunk { panic!("Thunk::value called on an unfinalised closure"); } */ - return value; + value } ThunkRepr::Blackhole => panic!("Thunk::value called on a black-holed thunk"), ThunkRepr::Suspended { .. } => panic!("Thunk::value called on a suspended thunk"), diff --git a/tvix/eval/src/vm.rs b/tvix/eval/src/vm.rs index 6c0d1157ec8d..f1dc99439638 100644 --- a/tvix/eval/src/vm.rs +++ b/tvix/eval/src/vm.rs @@ -275,8 +275,8 @@ impl<'o> VM<'o> { } /// Access the I/O handle used for filesystem access in this VM. - pub(crate) fn io(&self) -> &Box { - &self.io_handle + pub(crate) fn io(&self) -> &dyn EvalIO { + &*self.io_handle } /// Construct an error from the given ErrorKind and the source @@ -385,7 +385,7 @@ impl<'o> VM<'o> { // that of the tail-called closure. let mut frame = self.frame_mut(); frame.lambda = lambda; - frame.upvalues = closure.upvalues().clone(); + frame.upvalues = closure.upvalues(); frame.ip = CodeIdx(0); // reset instruction pointer to beginning Ok(()) } @@ -584,10 +584,8 @@ impl<'o> VM<'o> { (Value::List(_), _) => break false, (Value::Attrs(a1), Value::Attrs(a2)) => { - if allow_pointer_equality_on_functions_and_thunks { - if Rc::ptr_eq(&a1, &a2) { - continue; - } + if allow_pointer_equality_on_functions_and_thunks && Rc::ptr_eq(&a1, &a2) { + continue; } allow_pointer_equality_on_functions_and_thunks = true; match (a1.select("type"), a2.select("type")) { @@ -708,7 +706,7 @@ impl<'o> VM<'o> { match b { Value::Integer(0) => return Err(self.error(ErrorKind::DivisionByZero)), Value::Float(b) => { - if *b == (0.0 as f64) { + if *b == 0.0_f64 { return Err(self.error(ErrorKind::DivisionByZero)); } arithmetic_op!(self, /) -- cgit 1.4.1