diff --git a/Cargo.toml b/Cargo.toml index 53c0abc..e1b0054 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -38,3 +38,132 @@ getrandom = { version = "0.2", features = ["js"] } [workspace] members = ["codegen"] exclude = ["fuzz", "bitcoind-tests"] + +[lints.clippy] +# Exclude lints we don't think are valuable. +large_enum_variant = "allow" # docs say "measure before paying attention to this"; why is it on by default?? +similar_names = "allow" # Too many (subjectively) false positives. +uninlined_format_args = "allow" # This is a subjective style choice. +match_bool = "allow" # Adds extra indentation and LOC. +match_same_arms = "allow" # Collapses things that are conceptually unrelated to each other. +must_use_candidate = "allow" # Useful for audit but many false positives. +# Whitelist the cast lints because sometimes casts are unavoidable. But +# every cast should contain a code comment! +cast_possible_truncation = "allow" +cast_possible_wrap = "allow" +cast_sign_loss = "allow" +# Exhaustive list of pedantic clippy lints +assigning_clones = "warn" +bool_to_int_with_if = "warn" +borrow_as_ptr = "warn" +case_sensitive_file_extension_comparisons = "warn" +cast_lossless = "warn" +cast_precision_loss = "warn" +cast_ptr_alignment = "warn" +checked_conversions = "warn" +cloned_instead_of_copied = "warn" +copy_iterator = "warn" +default_trait_access = "warn" +doc_link_with_quotes = "warn" +doc_markdown = "allow" +empty_enum = "warn" +enum_glob_use = "allow" +expl_impl_clone_on_copy = "warn" +explicit_deref_methods = "warn" +explicit_into_iter_loop = "warn" +explicit_iter_loop = "warn" +filter_map_next = "warn" +flat_map_option = "warn" +float_cmp = "warn" +fn_params_excessive_bools = "warn" +from_iter_instead_of_collect = "warn" +if_not_else = "warn" +ignored_unit_patterns = "allow" +implicit_clone = "warn" +implicit_hasher = "warn" +inconsistent_struct_constructor = "warn" +index_refutable_slice = "warn" +inefficient_to_string = "allow" +inline_always = "warn" +into_iter_without_iter = "warn" +invalid_upcast_comparisons = "warn" +items_after_statements = "allow" +iter_filter_is_ok = "warn" +iter_filter_is_some = "warn" +iter_not_returning_iterator = "warn" +iter_without_into_iter = "warn" +large_digit_groups = "warn" +large_futures = "warn" +large_stack_arrays = "warn" +large_types_passed_by_value = "warn" +linkedlist = "warn" +macro_use_imports = "warn" +manual_assert = "allow" +manual_instant_elapsed = "warn" +manual_is_power_of_two = "warn" +manual_is_variant_and = "warn" +manual_let_else = "allow" +manual_ok_or = "warn" +manual_string_new = "warn" +many_single_char_names = "warn" +map_unwrap_or = "allow" +match_on_vec_items = "warn" +match_wild_err_arm = "warn" +match_wildcard_for_single_variants = "allow" +maybe_infinite_iter = "warn" +mismatching_type_param_order = "warn" +missing_errors_doc = "allow" +missing_fields_in_debug = "warn" +missing_panics_doc = "allow" +mut_mut = "warn" +naive_bytecount = "warn" +needless_bitwise_bool = "warn" +needless_continue = "allow" +needless_for_each = "warn" +needless_pass_by_value = "allow" +needless_raw_string_hashes = "allow" +no_effect_underscore_binding = "warn" +no_mangle_with_rust_abi = "warn" +option_as_ref_cloned = "warn" +option_option = "allow" +ptr_as_ptr = "warn" +ptr_cast_constness = "warn" +pub_underscore_fields = "warn" +range_minus_one = "warn" +range_plus_one = "warn" +redundant_closure_for_method_calls = "allow" +redundant_else = "warn" +ref_as_ptr = "warn" +ref_binding_to_reference = "warn" +ref_option = "warn" +ref_option_ref = "warn" +return_self_not_must_use = "allow" +same_functions_in_if_condition = "warn" +semicolon_if_nothing_returned = "allow" +should_panic_without_expect = "warn" +single_char_pattern = "warn" +single_match_else = "allow" +stable_sort_primitive = "warn" +str_split_at_newline = "warn" +string_add_assign = "warn" +struct_excessive_bools = "warn" +struct_field_names = "warn" +too_many_lines = "allow" +transmute_ptr_to_ptr = "warn" +trivially_copy_pass_by_ref = "warn" +unchecked_duration_subtraction = "warn" +unicode_not_nfc = "warn" +unnecessary_box_returns = "warn" +unnecessary_join = "warn" +unnecessary_literal_bound = "warn" +unnecessary_wraps = "warn" +unnested_or_patterns = "allow" +unreadable_literal = "allow" +unsafe_derive_deserialize = "warn" +unused_async = "warn" +unused_self = "warn" +used_underscore_binding = "warn" +used_underscore_items = "warn" +verbose_bit_mask = "warn" +wildcard_imports = "allow" +zero_sized_map_values = "warn" diff --git a/bitcoind-tests/tests/common/test.rs b/bitcoind-tests/tests/common/test.rs index 0e63842..5dd151f 100644 --- a/bitcoind-tests/tests/common/test.rs +++ b/bitcoind-tests/tests/common/test.rs @@ -53,7 +53,7 @@ impl<'a> TestCase<'a> { pub fn program_path>(mut self, path: P) -> Self { let text = std::fs::read_to_string(path).expect("path should be readable"); - let compiled = simfony::CompiledProgram::new(text.as_str(), simfony::Arguments::default()) + let compiled = simfony::CompiledProgram::new(text.as_str(), simfony::Arguments::default(), false) .expect("program should compile"); self.compiled = Some(compiled); self @@ -72,7 +72,7 @@ impl<'a> TestCase<'a> { .template .as_ref() .expect("template should exist") - .instantiate(arguments) + .instantiate(arguments, false) .expect("arguments should be consistent with the program"); self.compiled = Some(compiled); self diff --git a/clippy.toml b/clippy.toml index a93e948..8d622bc 100644 --- a/clippy.toml +++ b/clippy.toml @@ -1 +1,3 @@ msrv = "1.78.0" +# We have an error type, `RichError`, of size 144. This is pushing it but probably fine. +large-error-threshold = 145 diff --git a/fuzz/fuzz_targets/compile_parse_tree.rs b/fuzz/fuzz_targets/compile_parse_tree.rs index fd7319d..718d055 100644 --- a/fuzz/fuzz_targets/compile_parse_tree.rs +++ b/fuzz/fuzz_targets/compile_parse_tree.rs @@ -21,7 +21,7 @@ fuzz_target!(|data: &[u8]| { Err(..) => return, }; let simplicity_named_construct = ast_program - .compile(arguments) + .compile(arguments, false) .with_file("") .expect("AST should compile with given arguments"); let _simplicity_commit = named::to_commit_node(&simplicity_named_construct) diff --git a/fuzz/fuzz_targets/compile_text.rs b/fuzz/fuzz_targets/compile_text.rs index 2d8ed0f..5c0776d 100644 --- a/fuzz/fuzz_targets/compile_text.rs +++ b/fuzz/fuzz_targets/compile_text.rs @@ -46,7 +46,7 @@ fuzz_target!(|data: &[u8]| -> Corpus { Ok(arguments) => arguments, Err(..) => return Corpus::Reject, }; - let _ = template.instantiate(arguments); + let _ = template.instantiate(arguments, false); Corpus::Keep }); diff --git a/src/ast.rs b/src/ast.rs index dfc2615..8c08722 100644 --- a/src/ast.rs +++ b/src/ast.rs @@ -1039,13 +1039,13 @@ impl AbstractSyntaxTree for Call { parse_args: &[parse::Expression], expected_tys: &[ResolvedType], ) -> Result<(), Error> { - if parse_args.len() != expected_tys.len() { + if parse_args.len() == expected_tys.len() { + Ok(()) + } else { Err(Error::InvalidNumberOfArguments( expected_tys.len(), parse_args.len(), )) - } else { - Ok(()) } } @@ -1053,13 +1053,13 @@ impl AbstractSyntaxTree for Call { observed_ty: &ResolvedType, expected_ty: &ResolvedType, ) -> Result<(), Error> { - if observed_ty != expected_ty { + if observed_ty == expected_ty { + Ok(()) + } else { Err(Error::ExpressionTypeMismatch( expected_ty.clone(), observed_ty.clone(), )) - } else { - Ok(()) } } diff --git a/src/compile.rs b/src/compile.rs index 36d8351..41f01bb 100644 --- a/src/compile.rs +++ b/src/compile.rs @@ -66,6 +66,7 @@ struct Scope { call_tracker: Arc, /// Values for parameters inside the Simfony program. arguments: Arguments, + include_debug_symbols: bool, } impl Scope { @@ -77,12 +78,17 @@ impl Scope { /// /// The supplied `arguments` are consistent with the program's parameters. /// Call [`Arguments::is_consistent`] before calling this method! - pub fn new(call_tracker: Arc, arguments: Arguments) -> Self { + pub fn new( + call_tracker: Arc, + arguments: Arguments, + include_debug_symbols: bool, + ) -> Self { Self { variables: vec![vec![Pattern::Ignore]], ctx: simplicity::types::Context::new(), call_tracker, arguments, + include_debug_symbols, } } @@ -93,6 +99,7 @@ impl Scope { ctx: self.ctx.shallow_clone(), call_tracker: Arc::clone(&self.call_tracker), arguments: self.arguments.clone(), + include_debug_symbols: self.include_debug_symbols, } } @@ -193,12 +200,12 @@ impl Scope { span: &S, ) -> Result, RichError> { match self.call_tracker.get_cmr(span.as_ref()) { - Some(cmr) => { + Some(cmr) if self.include_debug_symbols => { let false_and_args = ProgNode::bit(self.ctx(), false).pair(args); let nop_assert = ProgNode::assertl_drop(body, cmr); false_and_args.comp(&nop_assert).with_span(span) } - None => args.comp(body).with_span(span), + _ => args.comp(body).with_span(span), } } @@ -246,8 +253,16 @@ impl Program { /// /// The supplied `arguments` are consistent with the program's parameters. /// Call [`Arguments::is_consistent`] before calling this method! - pub fn compile(&self, arguments: Arguments) -> Result { - let mut scope = Scope::new(Arc::clone(self.call_tracker()), arguments); + pub fn compile( + &self, + arguments: Arguments, + include_debug_symbols: bool, + ) -> Result { + let mut scope = Scope::new( + Arc::clone(self.call_tracker()), + arguments, + include_debug_symbols, + ); self.main().compile(&mut scope).map(PairBuilder::build) } } diff --git a/src/lib.rs b/src/lib.rs index 53d4197..b16e31b 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -72,7 +72,11 @@ impl TemplateProgram { /// /// The arguments are not consistent with the parameters of the program. /// Use [`TemplateProgram::parameters`] to see which parameters the program has. - pub fn instantiate(&self, arguments: Arguments) -> Result { + pub fn instantiate( + &self, + arguments: Arguments, + include_debug_symbols: bool, + ) -> Result { arguments .is_consistent(self.simfony.parameters()) .map_err(|error| error.to_string())?; @@ -80,7 +84,7 @@ impl TemplateProgram { debug_symbols: self.simfony.debug_symbols(self.file.as_ref()), simplicity: self .simfony - .compile(arguments) + .compile(arguments, include_debug_symbols) .with_file(Arc::clone(&self.file))?, witness_types: self.simfony.witness_types().shallow_clone(), }) @@ -113,8 +117,13 @@ impl CompiledProgram { /// /// - [`TemplateProgram::new`] /// - [`TemplateProgram::instantiate`] - pub fn new>>(s: Str, arguments: Arguments) -> Result { - TemplateProgram::new(s).and_then(|template| template.instantiate(arguments)) + pub fn new>>( + s: Str, + arguments: Arguments, + include_debug_symbols: bool, + ) -> Result { + TemplateProgram::new(s) + .and_then(|template| template.instantiate(arguments, include_debug_symbols)) } /// Access the debug symbols for the Simplicity target code. @@ -183,8 +192,9 @@ impl SatisfiedProgram { s: Str, arguments: Arguments, witness_values: WitnessValues, + include_debug_symbols: bool, ) -> Result { - let compiled = CompiledProgram::new(s, arguments)?; + let compiled = CompiledProgram::new(s, arguments, include_debug_symbols)?; compiled.satisfy(witness_values) } @@ -312,7 +322,7 @@ mod tests { } pub fn with_arguments(self, arguments: Arguments) -> TestCase { - let program = match self.program.instantiate(arguments) { + let program = match self.program.instantiate(arguments, true) { Ok(x) => x, Err(error) => panic!("{error}"), }; @@ -602,7 +612,12 @@ fn main() { assert!(my_true()); } "#; - match SatisfiedProgram::new(prog_text, Arguments::default(), WitnessValues::default()) { + match SatisfiedProgram::new( + prog_text, + Arguments::default(), + WitnessValues::default(), + false, + ) { Ok(_) => panic!("Accepted faulty program"), Err(error) => { if !error.contains("Expected expression of type `bool`, found type `()`") { diff --git a/src/main.rs b/src/main.rs index 04894a5..644f736 100644 --- a/src/main.rs +++ b/src/main.rs @@ -29,7 +29,7 @@ fn run() -> Result<(), String> { let prog_file = &args[1]; let prog_path = std::path::Path::new(prog_file); let prog_text = std::fs::read_to_string(prog_path).map_err(|e| e.to_string())?; - let compiled = CompiledProgram::new(prog_text, Arguments::default())?; + let compiled = CompiledProgram::new(prog_text, Arguments::default(), false)?; if args.len() >= 3 { let wit_file = &args[2]; @@ -73,7 +73,7 @@ fn run() -> Result<(), String> { let prog_file = &args[1]; let prog_path = std::path::Path::new(prog_file); let prog_text = std::fs::read_to_string(prog_path).map_err(|e| e.to_string())?; - let compiled = CompiledProgram::new(prog_text, Arguments::default())?; + let compiled = CompiledProgram::new(prog_text, Arguments::default(), false)?; let program_bytes = compiled.commit().encode_to_vec(); println!( diff --git a/src/num.rs b/src/num.rs index 350eeae..7df5790 100644 --- a/src/num.rs +++ b/src/num.rs @@ -298,8 +298,8 @@ impl fmt::Display for U256 { is_zero = true; // Divide by 10, starting at the most significant bytes - for byte in bytes.iter_mut() { - let value = carry * 256 + *byte as u32; + for byte in &mut bytes { + let value = carry * 256 + u32::from(*byte); *byte = (value / 10) as u8; carry = value % 10; @@ -334,7 +334,7 @@ impl FromStr for U256 { // Add to the least significant bytes first for byte in bytes.iter_mut().rev() { - let value = *byte as u32 * 10 + carry; + let value = u32::from(*byte) * 10 + carry; *byte = (value % 256) as u8; carry = value / 256; } diff --git a/src/pattern.rs b/src/pattern.rs index 0f80da9..d88e4df 100644 --- a/src/pattern.rs +++ b/src/pattern.rs @@ -259,7 +259,7 @@ impl BasePattern { /// Get an iterator over all identifiers inside the pattern. pub fn identifiers(&self) -> impl Iterator { - self.pre_order_iter().flat_map(BasePattern::as_identifier) + self.pre_order_iter().filter_map(BasePattern::as_identifier) } /// Check if all `identifiers` are contained inside the pattern. diff --git a/src/serde.rs b/src/serde.rs index 7c4ed14..2a12550 100644 --- a/src/serde.rs +++ b/src/serde.rs @@ -144,7 +144,7 @@ impl<'a> Serialize for WitnessMapSerializer<'a> { S: Serializer, { let mut map = serializer.serialize_map(Some(self.0.len()))?; - for (name, value) in self.0.iter() { + for (name, value) in self.0 { map.serialize_entry(name.as_inner(), &ValueMapSerializer(value))?; } map.end() diff --git a/src/types.rs b/src/types.rs index 30c8505..22f5bf0 100644 --- a/src/types.rs +++ b/src/types.rs @@ -55,7 +55,7 @@ impl TypeInner { TypeInner::Tuple(elements) => match n_children_yielded { 0 => { f.write_str("(")?; - if 0 == elements.len() { + if elements.is_empty() { f.write_str(")")?; } Ok(()) diff --git a/src/value.rs b/src/value.rs index 276002e..ccedc8a 100644 --- a/src/value.rs +++ b/src/value.rs @@ -145,7 +145,7 @@ impl UIntValue { return Err(Error::ExpressionTypeMismatch(ty.into(), bit_ty.into())); } - let byte_len = (bit_len.get() + 7) / 8; + let byte_len = bit_len.get().div_ceil(8); let mut bytes = Vec::with_capacity(byte_len); let padding_len = 8usize.saturating_sub(bit_len.get()); let padding = std::iter::repeat('0').take(padding_len); @@ -155,7 +155,7 @@ impl UIntValue { let mut byte = 0u8; for _ in 0..8 { let bit = padded_bits.next().unwrap(); - byte = (byte << 1) | if bit == '1' { 1 } else { 0 }; + byte = (byte << 1) | u8::from(bit == '1'); } bytes.push(byte); } @@ -934,7 +934,7 @@ impl ValueConstructible for StructuralValue { fn array>(elements: I, ty: Self::Type) -> Self { let elements: Vec = elements.into_iter().collect(); - for element in elements.iter() { + for element in &elements { assert!( element.is_of_type(&ty), "Element {element} is not of expected type {ty}" @@ -954,7 +954,7 @@ impl ValueConstructible for StructuralValue { elements.len() < bound.get(), "There must be fewer list elements than the bound {bound}" ); - for element in elements.iter() { + for element in &elements { assert!( element.is_of_type(&ty), "Element {element} is not of expected type {ty}" diff --git a/src/witness.rs b/src/witness.rs index d47a190..7aa17ec 100644 --- a/src/witness.rs +++ b/src/witness.rs @@ -236,7 +236,7 @@ mod tests { WitnessName::from_str_unchecked("A"), Value::u16(42), )])); - match SatisfiedProgram::new(s, Arguments::default(), witness) { + match SatisfiedProgram::new(s, Arguments::default(), witness, false) { Ok(_) => panic!("Ill-typed witness assignment was falsely accepted"), Err(error) => assert_eq!( "Witness `A` was declared with type `u32` but its assigned value is of type `u16`", @@ -255,7 +255,7 @@ fn main() { assert!(jet::is_zero_32(f())); }"#; - match CompiledProgram::new(s, Arguments::default()) { + match CompiledProgram::new(s, Arguments::default(), false) { Ok(_) => panic!("Witness outside main was falsely accepted"), Err(error) => { assert!(error