Skip to content

Commit

Permalink
Merge #442
Browse files Browse the repository at this point in the history
442: Remove HashMap from derive/generate r=dragostis a=nagisa

There is a lot of value in outputs produced by compilers being
reproducible. Rust makes it somewhat hard to get it right, but most of
the time rustc’s outputs are also reproducible. This allows us to enjoy
all the benefits as other more primitive toolchains. Most of the time.

Pest was, sadly, one of those examples where reproducibility was not a
concern and HashMap was used to implement some of the  code generation
portions. Replacing this `HashMap` with a `Vec` and a `O(nm)` loop is
the easy solution. With the values that `n` and `m` get substituted with
in practice, however, this loop should still be approximately as good as
the previous `HashMap` implementation.

Co-authored-by: Simonas Kazlauskas <[email protected]>
  • Loading branch information
bors[bot] and nagisa authored Feb 22, 2020
2 parents d63ef10 + dab7713 commit 1b0c9c5
Show file tree
Hide file tree
Showing 2 changed files with 13 additions and 9 deletions.
18 changes: 11 additions & 7 deletions generator/src/generator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
// option. All files in the project carrying such notice may not be copied,
// modified, or distributed except according to those terms.

use std::collections::HashMap;
use std::path::PathBuf;

use proc_macro2::{Span, TokenStream};
Expand Down Expand Up @@ -43,7 +42,13 @@ pub fn generate(
let skip = generate_skip(&rules);

let mut rules: Vec<_> = rules.into_iter().map(generate_rule).collect();
rules.extend(defaults.into_iter().map(|name| builtins[name].clone()));
rules.extend(builtins.into_iter().filter_map(|(builtin, tokens)| {
if defaults.contains(&builtin) {
Some(tokens)
} else {
None
}
}));

let (impl_generics, ty_generics, where_clause) = generics.split_for_impl();

Expand Down Expand Up @@ -89,8 +94,8 @@ pub fn generate(

// Note: All builtin rules should be validated as pest builtins in meta/src/validator.rs.
// Some should also be keywords.
fn generate_builtin_rules() -> HashMap<&'static str, TokenStream> {
let mut builtins = HashMap::new();
fn generate_builtin_rules() -> Vec<(&'static str, TokenStream)> {
let mut builtins = Vec::new();

insert_builtin!(builtins, ANY, state.skip(1));
insert_public_builtin!(
Expand Down Expand Up @@ -147,15 +152,14 @@ fn generate_builtin_rules() -> HashMap<&'static str, TokenStream> {
for property in UNICODE_PROPERTY_NAMES {
let property_ident: Ident = syn::parse_str(property).unwrap();
// insert manually for #property substitution
builtins.insert(property, quote! {
builtins.push((property, quote! {
#[inline]
#[allow(dead_code, non_snake_case, unused_variables)]
fn #property_ident(state: Box<::pest::ParserState<Rule>>) -> ::pest::ParseResult<Box<::pest::ParserState<Rule>>> {
state.match_char_by(::pest::unicode::#property_ident)
}
});
}));
}

builtins
}

Expand Down
4 changes: 2 additions & 2 deletions generator/src/macros.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,13 @@

macro_rules! insert_builtin {
($builtin: expr, $name: ident, $pattern: expr) => {
$builtin.insert(stringify!($name), generate_rule!($name, $pattern));
$builtin.push((stringify!($name), generate_rule!($name, $pattern)));
};
}

macro_rules! insert_public_builtin {
($builtin: expr, $name: ident, $pattern: expr) => {
$builtin.insert(stringify!($name), generate_public_rule!($name, $pattern));
$builtin.push((stringify!($name), generate_public_rule!($name, $pattern)));
};
}

Expand Down

0 comments on commit 1b0c9c5

Please sign in to comment.