mdebray/tvl-depot
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

fix(tvix/eval): fix current clippy warnings

Browse source 
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 <grfn@gws.fyi>
Autosubmit: tazjin <tazjin@tvl.su>
Tested-by: BuildkiteCI
This commit is contained in:
Vincent Ambo 2022-12-20 17:22:56 +03:00 committed by clbot
parent 67d508f2ec
commit 71174f6626
17 changed files with 69 additions and 108 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

19
tvix/eval/benches/eval.rs
View file

@ -2,23 +2,23 @@ use criterion::{black_box, criterion_group, criterion_main, Criterion};
use itertools::Itertools;
fn interpret(code: &str) {
tvix_eval::Evaluation::new(code).evaluate()
tvix_eval::Evaluation::new(code, None).evaluate();
}
fn eval_literals(c: &mut Criterion) {
c.bench_function("int", |b| {
b.iter(|| black_box(interpret("42", None, Default::default())))
b.iter(|| {
interpret("42");
black_box(())
})
});
}
fn eval_merge_attrs(c: &mut Criterion) {
c.bench_function("merge small attrs", |b| {
b.iter(|| {
black_box(interpret(
"{ a = 1; b = 2; } // { c = 3; }",
None,
Default::default(),
))
interpret("{ a = 1; b = 2; } // { c = 3; }");
black_box(())
})
});
@ -28,7 +28,10 @@ fn eval_merge_attrs(c: &mut Criterion) {
(0..10000).map(|n| format!("a{n} = {n};")).join(" ")
);
let expr = format!("{large_attrs} // {{ c = 3; }}");
b.iter(move || black_box(interpret(&expr, None, Default::default())))
b.iter(move || {
interpret(&expr);
black_box(())
})
});
}

17
tvix/eval/src/builtins/mod.rs
View file

@ -414,7 +414,7 @@ mod pure_builtins {
let mut res: BTreeMap<NixString, Vec<Value>> = 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);
};

3
tvix/eval/src/compiler/bindings.rs
View file

@ -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<SmolStr> {
match node {
ast::Attr::Ident(ident) => Some(ident.ident_token().unwrap().text().into()),

6
tvix/eval/src/compiler/mod.rs
View file

@ -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,
})
}

13
tvix/eval/src/errors.rs
View file

@ -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<SpanLabel> {
match &self.kind {
ErrorKind::ImportParseError { errors, file, .. } => {
spans_for_parse_errors(&file, errors)
spans_for_parse_errors(file, errors)
}
ErrorKind::ParseErrors(errors) => {

2
tvix/eval/src/lib.rs
View file

@ -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(

1
tvix/eval/src/nix_search_path.rs
View file

@ -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<P>(&self, path: P) -> Result<PathBuf, ErrorKind>
where
P: AsRef<Path>,

62
tvix/eval/src/pretty_ast.rs
View file

@ -74,7 +74,7 @@ impl<'a> Serialize for SerializeAST<&'a ast::Select> {
}
}
impl<'a> Serialize for SerializeAST<ast::InterpolPart<String>> {
impl Serialize for SerializeAST<ast::InterpolPart<String>> {
fn serialize<S: Serializer>(&self, serializer: S) -> Result<S::Ok, S::Error> {
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::<Vec<_>>(),
)?;
@ -104,7 +104,7 @@ impl<'a> Serialize for SerializeAST<&'a ast::Str> {
}
}
impl<'a> Serialize for SerializeAST<ast::InterpolPart<ast::PathContent>> {
impl Serialize for SerializeAST<ast::InterpolPart<ast::PathContent>> {
fn serialize<S: Serializer>(&self, serializer: S) -> Result<S::Ok, S::Error> {
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::<Vec<_>>(),
&self.0.parts().map(SerializeAST).collect::<Vec<_>>(),
)?;
map.end()
@ -148,7 +144,7 @@ impl<'a> Serialize for SerializeAST<&'a ast::Literal> {
}
}
impl<'a> Serialize for SerializeAST<ast::PatEntry> {
impl Serialize for SerializeAST<ast::PatEntry> {
fn serialize<S: Serializer>(&self, serializer: S) -> Result<S::Ok, S::Error> {
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<ast::PatEntry> {
}
}
impl<'a> Serialize for SerializeAST<ast::Param> {
impl Serialize for SerializeAST<ast::Param> {
fn serialize<S: Serializer>(&self, serializer: S) -> Result<S::Ok, S::Error> {
match &self.0 {
ast::Param::Pattern(pat) => {
@ -170,9 +166,7 @@ impl<'a> Serialize for SerializeAST<ast::Param> {
map.serialize_entry(
"entries",
&pat.pat_entries()
.map(|entry| SerializeAST(entry))
.collect::<Vec<_>>(),
&pat.pat_entries().map(SerializeAST).collect::<Vec<_>>(),
)?;
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::<Vec<_>>(),
)?;
map.serialize_entry(
"inherits",
&self
.0
.inherits()
.map(|val| SerializeAST(val))
.collect::<Vec<_>>(),
&self.0.inherits().map(SerializeAST).collect::<Vec<_>>(),
)?;
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::<Vec<_>>(),
)?;
map.serialize_entry(
"inherits",
&self
.0
.inherits()
.map(|val| SerializeAST(val))
.collect::<Vec<_>>(),
&self.0.inherits().map(SerializeAST).collect::<Vec<_>>(),
)?;
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<S: Serializer>(&self, serializer: S) -> Result<S::Ok, S::Error> {
let list = self
.0
.items()
.map(|elem| SerializeAST(elem))
.collect::<Vec<_>>();
let list = self.0.items().map(SerializeAST).collect::<Vec<_>>();
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<ast::AttrpathValue> {
impl Serialize for SerializeAST<ast::AttrpathValue> {
fn serialize<S: Serializer>(&self, serializer: S) -> Result<S::Ok, S::Error> {
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<ast::AttrpathValue> {
}
}
impl<'a> Serialize for SerializeAST<ast::Inherit> {
impl Serialize for SerializeAST<ast::Inherit> {
fn serialize<S: Serializer>(&self, serializer: S) -> Result<S::Ok, S::Error> {
let mut map = serializer.serialize_map(None)?;
@ -341,7 +323,7 @@ impl<'a> Serialize for SerializeAST<ast::Inherit> {
map.serialize_entry(
"names",
&self.0.attrs().map(|a| SerializeAST(a)).collect::<Vec<_>>(),
&self.0.attrs().map(SerializeAST).collect::<Vec<_>>(),
)?;
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::<Vec<_>>(),
)?;
map.serialize_entry(
"inherits",
&self
.0
.inherits()
.map(|val| SerializeAST(val))
.collect::<Vec<_>>(),
&self.0.inherits().map(SerializeAST).collect::<Vec<_>>(),
)?;
map.end()
@ -439,11 +417,7 @@ impl Serialize for SerializeAST<ast::Attrpath> {
map.serialize_entry(
"path",
&self
.0
.attrs()
.map(|attr| SerializeAST(attr))
.collect::<Vec<_>>(),
&self.0.attrs().map(SerializeAST).collect::<Vec<_>>(),
)?;
map.end()

7
tvix/eval/src/systems.rs
View file

@ -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,
},

14
tvix/eval/src/tests/mod.rs
View file

@ -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
);
}
}

0
tvix/eval/src/tests/nix_tests/notyetpassing/eval-okay-readDir.nix → tvix/eval/src/tests/nix_tests/notyetpassing/eval-okay-readDir.nix.disabled
View file

1
tvix/eval/src/value/attrs.rs
View file

@ -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 {

2
tvix/eval/src/value/function.rs
View file

@ -30,7 +30,7 @@ impl Formals {
Q: ?Sized + Hash + Eq,
NixString: std::borrow::Borrow<Q>,
{
self.ellipsis || self.arguments.contains_key(&arg)
self.ellipsis || self.arguments.contains_key(arg)
}
}

4
tvix/eval/src/value/mod.rs
View file

@ -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()
}
}

2
tvix/eval/src/value/string.rs
View file

@ -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

10
tvix/eval/src/value/thunk.rs
View file

@ -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"),

14
tvix/eval/src/vm.rs
View file

@ -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<dyn EvalIO> {
&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, /)