From 8be957b6814ab4c662209bdd95404f20280d550c Mon Sep 17 00:00:00 2001 From: matt rice Date: Tue, 12 Dec 2023 08:45:16 -0800 Subject: [PATCH 1/2] fix clippy lints --- cfgrammar/src/lib/yacc/grammar.rs | 15 +++++---------- lrpar/cttests/src/lib.rs | 2 +- lrpar/cttests_macro/src/lib.rs | 2 +- lrtable/src/lib/statetable.rs | 4 ++-- 4 files changed, 9 insertions(+), 14 deletions(-) diff --git a/cfgrammar/src/lib/yacc/grammar.rs b/cfgrammar/src/lib/yacc/grammar.rs index 900917ae6..7f3e0870c 100644 --- a/cfgrammar/src/lib/yacc/grammar.rs +++ b/cfgrammar/src/lib/yacc/grammar.rs @@ -833,8 +833,7 @@ where // needed). If the first column spills, then we're done. This is basically normal // arithmetic but with each digit having an arbitrary base. - let mut todo = Vec::new(); - todo.resize(prod.len(), 0); + let mut todo = vec![0; prod.len()]; let mut cur = Vec::new(); 'b: loop { for i in 0..todo.len() { @@ -893,10 +892,8 @@ where // means that we can iteratively improve our knowledge of a token's minimum cost: // eventually we will reach a point where we can determine it definitively. - let mut costs = vec![]; - costs.resize(usize::from(grm.rules_len()), 0); - let mut done = vec![]; - done.resize(usize::from(grm.rules_len()), false); + let mut costs = vec![0; usize::from(grm.rules_len())]; + let mut done = vec![false; usize::from(grm.rules_len())]; loop { let mut all_done = true; for i in 0..done.len() { @@ -960,10 +957,8 @@ fn rule_max_costs( where usize: AsPrimitive, { - let mut done = vec![]; - done.resize(usize::from(grm.rules_len()), false); - let mut costs = vec![]; - costs.resize(usize::from(grm.rules_len()), 0); + let mut done = vec![false; usize::from(grm.rules_len())]; + let mut costs = vec![0; usize::from(grm.rules_len())]; // First mark all recursive rules. for ridx in grm.iter_rules() { diff --git a/lrpar/cttests/src/lib.rs b/lrpar/cttests/src/lib.rs index e448c8837..aabd61d3d 100644 --- a/lrpar/cttests/src/lib.rs +++ b/lrpar/cttests/src/lib.rs @@ -242,7 +242,7 @@ fn test_parseparam() { let lexerdef = parseparam_l::lexerdef(); let lexer = lexerdef.lexer("101"); match parseparam_y::parse(&lexer, &3) { - (Some(i), _) if i == 104 => (), + (Some(104), _) => (), _ => unreachable!(), } } diff --git a/lrpar/cttests_macro/src/lib.rs b/lrpar/cttests_macro/src/lib.rs index a5fee5fa1..4c3e39071 100644 --- a/lrpar/cttests_macro/src/lib.rs +++ b/lrpar/cttests_macro/src/lib.rs @@ -17,7 +17,7 @@ pub fn generate_codegen_fail_tests(item: TokenStream) -> TokenStream { let manifest_dir = std::path::Path::new(&manifest_dir) .strip_prefix(cwd) .unwrap(); - let test_glob_path = manifest_dir.join(&test_glob_str.value()); + let test_glob_path = manifest_dir.join(test_glob_str.value()); let test_glob_str = test_glob_path.into_os_string().into_string().unwrap(); let test_files = glob(&test_glob_str).unwrap(); for file in test_files { diff --git a/lrtable/src/lib/statetable.rs b/lrtable/src/lib/statetable.rs index e6142deea..437adbe85 100644 --- a/lrtable/src/lib/statetable.rs +++ b/lrtable/src/lib/statetable.rs @@ -988,8 +988,8 @@ D : D; Ok(_) => panic!("Infinitely recursive rule let through"), Err(StateTableError { kind: StateTableErrorKind::AcceptReduceConflict(_), - pidx, - }) if pidx == PIdx(1) => (), + pidx: PIdx(1), + }) => (), Err(e) => panic!("Incorrect error returned {:?}", e), } } From bbc634cf48e1ff776df2c62868ff13f6e684134a Mon Sep 17 00:00:00 2001 From: matt rice Date: Tue, 12 Dec 2023 08:46:10 -0800 Subject: [PATCH 2/2] Move generated parse function and friends to their own module. The unsafe code test was triggering a clippy lint, in fixing that it became apparent that unsafe code in the program section was being denied inadvertently. Reorganize the codegen so the generated parse tables and parse functions are in their own `mod` separate from user actions. That module now denies unsafe. User actions are kept in the parent module where unsafe is still allowed. The user provided program section is now placed at the top of the module so it may contain it's own `#![inner_attributes]`. This is a desired feature see #364 for one instance. While this can be used it also applies the attribute to the generated code in the submodule in ways that could compilation failure. Since there is not currently a way to isolate the attributes of the parser module from the user code section, it isn't currently a documented or well supported feature. --- lrpar/cttests/src/calc_unsafeaction.test | 11 +++++++--- lrpar/src/lib/ctbuilder.rs | 27 +++++++++++++++++------- 2 files changed, 27 insertions(+), 11 deletions(-) diff --git a/lrpar/cttests/src/calc_unsafeaction.test b/lrpar/cttests/src/calc_unsafeaction.test index a30918cc8..fed359e6f 100644 --- a/lrpar/cttests/src/calc_unsafeaction.test +++ b/lrpar/cttests/src/calc_unsafeaction.test @@ -5,11 +5,11 @@ grammar: | %actiontype Result %avoid_insert 'INT' %% - Expr: Expr '+' Term { unsafe { Ok(Ok::($1? + $3?).unwrap_unchecked()) } } + Expr: Expr '+' Term { unsafe { unsafe_ok($1? + $3?) } } | Term { $1 } ; - Term: Term '*' Factor { unsafe { Ok(Ok::($1? * $3?).unwrap_unchecked()) } } + Term: Term '*' Factor { unsafe { unsafe_ok($1? * $3?) } } | Factor { $1 } ; @@ -17,7 +17,7 @@ grammar: | | 'INT' { let l = $1.map_err(|_| ())?; match $lexer.span_str(l.span()).parse::() { - Ok(v) => unsafe { Ok(Ok::(v).unwrap_unchecked()) }, + Ok(v) => unsafe { unsafe_ok(v) }, Err(_) => { let ((_, col), _) = $lexer.line_col(l.span()); eprintln!("Error at column {}: '{}' cannot be represented as a u64", @@ -28,6 +28,11 @@ grammar: | } } ; + %% + // Just check that unsafe blocks work in actions. + unsafe fn unsafe_ok(x:T) -> Result { + Ok(x) + } lexer: | %% diff --git a/lrpar/src/lib/ctbuilder.rs b/lrpar/src/lib/ctbuilder.rs index 76d53d7ed..410c76d3e 100644 --- a/lrpar/src/lib/ctbuilder.rs +++ b/lrpar/src/lib/ctbuilder.rs @@ -676,12 +676,19 @@ where ) -> Result<(), Box> { let mut outs = String::new(); writeln!(outs, "{} mod {} {{", self.visibility.cow_str(), mod_name).ok(); + // Emit user program section, and actions at the top so they may specify inner attributes. + if let Some(YaccKind::Original(YaccOriginalActionKind::UserAction) | YaccKind::Grmtools) = + self.yacckind + { + outs.push_str(&self.gen_user_actions(grm)); + } + outs.push_str(" mod _parser_ {\n"); outs.push_str( - " #![allow(clippy::type_complexity)] - #![allow(clippy::unnecessary_wraps)] - #![deny(unsafe_code)] - #[allow(unused_imports)] - use ::lrpar::Lexeme; + " #![allow(clippy::type_complexity)] + #![allow(clippy::unnecessary_wraps)] + #![deny(unsafe_code)] + #[allow(unused_imports)] + use super::*; ", ); @@ -691,13 +698,17 @@ where match self.yacckind.unwrap() { YaccKind::Original(YaccOriginalActionKind::UserAction) | YaccKind::Grmtools => { outs.push_str(&self.gen_wrappers(grm)); - outs.push_str(&self.gen_user_actions(grm)); } YaccKind::Original(YaccOriginalActionKind::NoAction) | YaccKind::Original(YaccOriginalActionKind::GenericParseTree) => (), _ => unreachable!(), } - outs.push_str("}\n\n"); + outs.push_str(" } // End of `mod _parser_`\n\n"); + outs.push_str(" #[allow(unused_imports)]\n"); + outs.push_str(" pub use _parser_::*;\n"); + outs.push_str(" #[allow(unused_imports)]\n"); + outs.push_str(" use ::lrpar::Lexeme;\n"); + outs.push_str("} // End of `mod {mod_name}` \n\n"); // Output the cache so that we can check whether the IDs map is stable. outs.push_str(cache); @@ -1103,6 +1114,7 @@ where if let Some(s) = grm.programs() { outs.push_str("\n// User code from the program section\n\n"); outs.push_str(s); + outs.push_str("\n// End of user code from the program section\n\n"); } // Convert actions to functions @@ -1144,7 +1156,6 @@ where outs, " // {rulename} #[allow(clippy::too_many_arguments)] - #[allow(unsafe_code)] // Allow an action to embed unsafe blocks within it. fn {prefix}action_{}<'lexer, 'input: 'lexer>({prefix}ridx: ::cfgrammar::RIdx<{storaget}>, {prefix}lexer: &'lexer dyn ::lrpar::NonStreamingLexer<'input, {lexertypest}>, {prefix}span: ::cfgrammar::Span,