2
0
Эх сурвалжийг харах

Improve code generation of choice! macro

This improves the code generation for when we need to try a bunch of parsers
and see what sticks. I don't know how it took me so long to think of doing
this, but it helps a huge amount. Compile times are noticeably reduced on
my machine, and we no longer need to manage the type limit when choice!ing
a large number of parsers together. The code generation at the macro
callsite is arguably worse, but overall it's a big win.
Isaac Woods 4 жил өмнө
parent
commit
47c4aec17e
2 өөрчлөгдсөн 41 нэмэгдсэн , 29 устгасан
  1. 27 11
      aml/src/parser.rs
  2. 14 18
      aml/src/type2.rs

+ 27 - 11
aml/src/parser.rs

@@ -436,17 +436,33 @@ pub(crate) macro choice {
         id().map(|()| Err(AmlError::WrongParser))
     },
 
-    ($first_parser: expr) => {
-        $first_parser
-        .or(id().map(|()| Err(AmlError::WrongParser)))
-    },
-
-    ($first_parser: expr, $($other_parser: expr),*) => {
-        $first_parser
-        $(
-            .or($other_parser)
-         )*
-        .or(id().map(|()| Err(AmlError::WrongParser)))
+    /*
+     * The nice way of writing this would generate something like:
+     * ```
+     * $first_parser
+     * $(
+     *     .or($other_parser)
+     *  )*
+     * .or(id().map(|()| Err(AmlError::WrongParser)))
+     * ```
+     * This problem with this is that it generates enormous types that very easily break `rustc`'s type
+     * limit, so writing large parsers with choice required some gymnastics, which sucks for everyone involved.
+     *
+     * Instead, we manually call each parser sequentially, checking its result to see if we should return, or try
+     * the next parser. This generates worse code at the macro callsite, but is much easier for the compiler to
+     * type-check (and so reduces the cost of pulling us in as a dependency as well as improving ergonomics).
+     */
+    ($($parser: expr),+) => {
+        move |input, mut context| {
+            $(
+                match ($parser).parse(input, context) {
+                    Ok(parse_result) => return Ok(parse_result),
+                    Err((_, new_context, AmlError::WrongParser)) => { context = new_context; },
+                    Err((_, context, err)) => return Err((input, context, err)),
+                }
+             )+
+            Err((input, context, AmlError::WrongParser))
+        }
     }
 }
 

+ 14 - 18
aml/src/type2.rs

@@ -35,29 +35,25 @@ where
      *                DefVarPackage | DefRefOf | DefShiftLeft | DefShitRight | DefSizeOf | DefStore |
      *                DefSubtract | DefTimer | DefToBCD | DefToBuffer | DefToDecimalString |
      *                DefToHexString | DefToInteger | DefToString | DefWait | DefXOr | MethodInvocation
-     *
-     * NOTE: MethodInvocation should always appear last in the choice.
      */
-    // TODO: we're struggling a little with the type limit here, is there a better way than making everything
-    // concrete?
     make_parser_concrete!(comment_scope(
         DebugVerbosity::AllScopes,
         "Type2Opcode",
         choice!(
-            make_parser_concrete!(def_and()),
-            make_parser_concrete!(def_buffer()),
-            make_parser_concrete!(def_l_equal()),
-            make_parser_concrete!(def_l_greater()),
-            make_parser_concrete!(def_l_greater_equal()),
-            make_parser_concrete!(def_l_less()),
-            make_parser_concrete!(def_l_less_equal()),
-            make_parser_concrete!(def_l_not_equal()),
-            make_parser_concrete!(def_l_or()),
-            make_parser_concrete!(def_package()),
-            make_parser_concrete!(def_shift_left()),
-            make_parser_concrete!(def_shift_right()),
-            make_parser_concrete!(def_store()),
-            make_parser_concrete!(method_invocation())
+            def_and(),
+            def_buffer(),
+            def_l_equal(),
+            def_l_greater(),
+            def_l_greater_equal(),
+            def_l_less(),
+            def_l_less_equal(),
+            def_l_not_equal(),
+            def_l_or(),
+            def_package(),
+            def_shift_left(),
+            def_shift_right(),
+            def_store(),
+            method_invocation() // XXX: this must always appear last. See how we have to parse it to see why.
         ),
     ))
 }