Repository: tweag/nickel
Pull request #1227 (2023-04-04 14:14:52)

Export a parser for `Types` and refactor a bit `parser`
This PR has been done as part of fixing #1187, and split as a stand-alone change. For some reason I needed to parse types. This PR exposes the rule for parsing type in the LALRPOP grammar.

I also noticed that the code already present had duplication, and that I was going to add one more copy of the `parse_term_tolerant` and `parse_term` methods. Those methods are now implemented once and for all for any parser generated by Lalrpop, and have been renamed to be more consistent with the rest of the codebase.


Patch:
diff --git a/benches/arrays/fold.ncl b/benches/arrays/fold.ncl
index 4bb934ed..fd1b7bd9 100644
--- a/benches/arrays/fold.ncl
+++ b/benches/arrays/fold.ncl
@@ -1,6 +1,7 @@
-let letter | Number -> string.CharLiteral = fun n =>
-  string.code "a" + (n % 26)
-  |> string.from_code in
+let letter | Number -> string.Character = fun n =>
+  let letters = string.characters "abcdefghijklmnopqrstuvwxyz" in
+  array.at (n % 26) letters
+in
 
 {
   right = {
diff --git a/benches/mantis/deploy.ncl b/benches/mantis/deploy.ncl
index ee47bb9f..aa6dd9ce 100644
--- a/benches/mantis/deploy.ncl
+++ b/benches/mantis/deploy.ncl
@@ -67,6 +67,7 @@ fun vars =>
       loggers = defaultLoggers,
       job = vars.job,
       role | [| `passive, `miner, `backup |] = vars.role,
+      reschedule = {},
     }
   in
   let jobDefs =
diff --git a/src/cache.rs b/src/cache.rs
index de18af31..7cc8d052 100644
--- a/src/cache.rs
+++ b/src/cache.rs
@@ -2,7 +2,7 @@
 
 use crate::error::{Error, ImportError, ParseError, ParseErrors, TypecheckError};
 use crate::eval::cache::Cache as EvalCache;
-use crate::parser::lexer::Lexer;
+use crate::parser::{lexer::Lexer, ErrorTolerantParser};
 use crate::position::TermPos;
 use crate::stdlib::{self as nickel_stdlib, StdlibModule};
 use crate::term::record::{Field, RecordData};
@@ -452,7 +452,7 @@ impl Cache {
             InputFormat::Nickel => {
                 let (t, parse_errs) =
                     // TODO: Should this really be parse_term if self.error_tolerant = false?
-                    parser::grammar::TermParser::new().parse_term_lax(file_id, Lexer::new(buf))?;
+                    parser::grammar::TermParser::new().parse_tolerant(file_id, Lexer::new(buf))?;
 
                 Ok((t, parse_errs))
             }
@@ -1248,7 +1248,7 @@ pub mod resolvers {
             if let hash_map::Entry::Vacant(e) = self.term_cache.entry(file_id) {
                 let buf = self.files.source(file_id);
                 let term = parser::grammar::TermParser::new()
-                    .parse_term(file_id, Lexer::new(buf))
+                    .parse_strict(file_id, Lexer::new(buf))
                     .map_err(|e| ImportError::ParseErrors(e, *pos))?;
                 e.insert(term);
                 Ok((
diff --git a/src/parser/grammar.lalrpop b/src/parser/grammar.lalrpop
index 3ada1888..ab0c0fe6 100644
--- a/src/parser/grammar.lalrpop
+++ b/src/parser/grammar.lalrpop
@@ -197,7 +197,10 @@ Types: Types = {
 };
 
 // A type with type variables fixed. See `parser::utils::fix_type_vars`.
-FixedType: Types = {
+//
+// This rule is public and can be used from external modules to parse an input
+// directly as a type.
+pub FixedType: Types = {
     <l: @L> <mut ty: Types> <r: @R> =>? {
         ty.fix_type_vars(mk_span(src_id, l, r))?;
         Ok(ty)
diff --git a/src/parser/mod.rs b/src/parser/mod.rs
index 72b8b6ed..b172f6b5 100644
--- a/src/parser/mod.rs
+++ b/src/parser/mod.rs
@@ -1,6 +1,7 @@
 use crate::error::{ParseError, ParseErrors};
 use crate::identifier::Ident;
 use crate::term::RichTerm;
+use crate::types::Types;
 use codespan::FileId;
 use lalrpop_util::lalrpop_mod;
 
@@ -10,6 +11,8 @@ lalrpop_mod!(
     #[allow(unused_imports)]
     pub grammar, "/parser/grammar.rs");
 
+use grammar::__ToTriple;
+
 pub mod error;
 pub mod lexer;
 pub mod uniterm;
@@ -31,51 +34,92 @@ pub enum ExtendedTerm {
     ToplevelLet(Ident, RichTerm),
 }
 
-impl grammar::ExtendedTermParser {
-    pub fn parse_term_tolerant(
+// The interface of LALRPOP-generated parsers, for each public rule. This trait is used as a facade
+// to implement parser-independent features (such as error tolerance helpers), which don't have to
+// be reimplemented for each and every parser. It's LALRPOP-specific and shouldn't be used outside
+// of this module, if we don't want our implementation to be coupled to LALRPOP details.
+//
+// The type of `parse` was just copy-pasted from the generated code of LALRPOP.
+trait LalrpopParser<T> {
+    fn parse<'input, 'err, 'wcard, __TOKEN, __TOKENS>(
         &self,
-        file_id: FileId,
-        lexer: lexer::Lexer,
-    ) -> Result<(ExtendedTerm, ParseErrors), ParseError> {
-        let mut parse_errors = Vec::new();
-        let mut next_wildcard_id = 0;
-        let result = self
-            .parse(file_id, &mut parse_errors, &mut next_wildcard_id, lexer)
-            .map_err(|err| ParseError::from_lalrpop(err, file_id));
+        src_id: FileId,
+        errors: &'err mut Vec<
+            lalrpop_util::ErrorRecovery<usize, lexer::Token<'input>, self::error::ParseError>,
+        >,
+        next_wildcard_id: &'wcard mut usize,
+        __tokens0: __TOKENS,
+    ) -> Result<T, lalrpop_util::ParseError<usize, lexer::Token<'input>, self::error::ParseError>>
+    where
+        __TOKEN: __ToTriple<'input, 'err, 'wcard>,
+        __TOKENS: IntoIterator<Item = __TOKEN>;
+}
 
-        let parse_errors = ParseErrors::from_recoverable(parse_errors, file_id);
-        match result {
-            Ok(t) => Ok((t, parse_errors)),
-            Err(e) => Err(e),
+/// Generate boiler-plate code to implement the trait [`LalrpopParser`] for a parser generated by
+/// LALRPOP.
+macro_rules! generate_lalrpop_parser_impl {
+    ($parser:ty, $output:ty) => {
+        impl LalrpopParser<$output> for $parser {
+            fn parse<'input, 'err, 'wcard, __TOKEN, __TOKENS>(
+                &self,
+                src_id: FileId,
+                errors: &'err mut Vec<
+                    lalrpop_util::ErrorRecovery<
+                        usize,
+                        lexer::Token<'input>,
+                        self::error::ParseError,
+                    >,
+                >,
+                next_wildcard_id: &'wcard mut usize,
+                __tokens0: __TOKENS,
+            ) -> Result<
+                $output,
+                lalrpop_util::ParseError<usize, lexer::Token<'input>, self::error::ParseError>,
+            >
+            where
+                __TOKEN: __ToTriple<'input, 'err, 'wcard>,
+                __TOKENS: IntoIterator<Item = __TOKEN>,
+            {
+                Self::parse(self, src_id, errors, next_wildcard_id, __tokens0)
+            }
         }
-    }
+    };
+}
 
-    pub fn parse_term(
+generate_lalrpop_parser_impl!(grammar::ExtendedTermParser, ExtendedTerm);
+generate_lalrpop_parser_impl!(grammar::TermParser, RichTerm);
+generate_lalrpop_parser_impl!(grammar::FixedTypeParser, Types);
+
+/// Generic interface of the various specialized Nickel parsers.
+///
+/// `T` is the product of the parser (a term, a type, etc.).
+pub trait ErrorTolerantParser<T> {
+    /// Parse a value from a lexer with the given `file_id` in an error-tolerant way. This methods
+    /// can still fail for non-recoverable errors.
+    fn parse_tolerant(
         &self,
         file_id: FileId,
         lexer: lexer::Lexer,
-    ) -> Result<ExtendedTerm, ParseErrors> {
-        match self.parse_term_tolerant(file_id, lexer) {
-            Ok((t, e)) if e.no_errors() => Ok(t),
-            Ok((_, e)) => Err(e),
-            Err(e) => Err(e.into()),
-        }
-    }
-}
+    ) -> Result<(T, ParseErrors), ParseError>;
 
-impl grammar::TermParser {
-    /// Parse a term from a token stream in an error tolerant way: parse errors are accumulated and
-    /// returned, instead of returning an error. The `Err` variant corresponds to a non-recoverable
+    /// Parse a value from a lexer with the given `file_id`, failing at the first encountered
     /// error.
-    pub fn parse_term_lax(
+    fn parse_strict(&self, file_id: FileId, lexer: lexer::Lexer) -> Result<T, ParseErrors>;
+}
+
+impl<T, P> ErrorTolerantParser<T> for P
+where
+    P: LalrpopParser<T>,
+{
+    fn parse_tolerant(
         &self,
         file_id: FileId,
         lexer: lexer::Lexer,
-    ) -> Result<(RichTerm, ParseErrors), ParseError> {
+    ) -> Result<(T, ParseErrors), ParseError> {
         let mut parse_errors = Vec::new();
-        let mut wildcard_id = 0;
+        let mut next_wildcard_id = 0;
         let result = self
-            .parse(file_id, &mut parse_errors, &mut wildcard_id, lexer)
+            .parse(file_id, &mut parse_errors, &mut next_wildcard_id, lexer)
             .map_err(|err| ParseError::from_lalrpop(err, file_id));
 
         let parse_errors = ParseErrors::from_recoverable(parse_errors, file_id);
@@ -85,12 +129,8 @@ impl grammar::TermParser {
         }
     }
 
-    pub fn parse_term(
-        &self,
-        file_id: FileId,
-        lexer: lexer::Lexer,
-    ) -> Result<RichTerm, ParseErrors> {
-        match self.parse_term_lax(file_id, lexer) {
+    fn parse_strict(&self, file_id: FileId, lexer: lexer::Lexer) -> Result<T, ParseErrors> {
+        match self.parse_tolerant(file_id, lexer) {
             Ok((t, e)) if e.no_errors() => Ok(t),
             Ok((_, e)) => Err(e),
             Err(e) => Err(e.into()),
diff --git a/src/repl/mod.rs b/src/repl/mod.rs
index 25d17c36..fbca2417 100644
--- a/src/repl/mod.rs
+++ b/src/repl/mod.rs
@@ -11,7 +11,7 @@ use crate::error::{Error, EvalError, IOError, ParseError, ParseErrors, ReplError
 use crate::eval::cache::Cache as EvalCache;
 use crate::eval::VirtualMachine;
 use crate::identifier::Ident;
-use crate::parser::{grammar, lexer, ExtendedTerm};
+use crate::parser::{grammar, lexer, ErrorTolerantParser, ExtendedTerm};
 use crate::program::QueryPath;
 use crate::term::{record::Field, RichTerm, Term, Traverse};
 use crate::transform::import_resolution;
@@ -116,7 +116,7 @@ impl<EC: EvalCache> ReplImpl<EC> {
 
         let (term, parse_errs) = self
             .parser
-            .parse_term_tolerant(file_id, lexer::Lexer::new(exp))?;
+            .parse_tolerant(file_id, lexer::Lexer::new(exp))?;
 
         if !parse_errs.no_errors() {
             return Err(parse_errs.into());
@@ -372,7 +372,7 @@ impl InputParser {
 
         let result = self
             .parser
-            .parse_term_tolerant(self.file_id, lexer::Lexer::new(input));
+            .parse_tolerant(self.file_id, lexer::Lexer::new(input));
 
         let partial = |pe| {
             matches!(
diff --git a/src/types.rs b/src/types.rs
index e5c2bad3..e1df23cf 100644
--- a/src/types.rs
+++ b/src/types.rs
@@ -1105,30 +1105,17 @@ impl Display for Types {
 #[cfg(test)]
 mod test {
     use super::Types;
-    use crate::parser::grammar::TermParser;
     use crate::parser::lexer::Lexer;
-    use crate::term::Term;
+    use crate::parser::{grammar::FixedTypeParser, ErrorTolerantParser};
     use codespan::Files;
 
     /// Parse a type represented as a string.
     fn parse_type(s: &str) -> Types {
-        use crate::term::TypeAnnotation;
+        let id = Files::new().add("<test>", s);
 
-        // Wrap the type in a contract to have it accepted by the parser.
-        let wrapper = format!("null | {s}");
-        println!("{wrapper}");
-        let id = Files::new().add("<test>", wrapper.clone());
-
-        let rt = TermParser::new()
-            .parse_term(id, Lexer::new(&wrapper))
-            .unwrap();
-
-        match rt.term.into_owned() {
-            Term::Annotated(TypeAnnotation { mut contracts, .. }, _) if contracts.len() == 1 => {
-                contracts.remove(0).types
-            }
-            _ => panic!("types::test::parse_type(): expected contract"),
-        }
+        FixedTypeParser::new()
+            .parse_strict(id, Lexer::new(s))
+            .unwrap()
     }
 
     /// Take a string representation of a type, parse it, and assert that formatting it gives the
diff --git a/stdlib/array.ncl b/stdlib/array.ncl
index 18d962a7..609f0990 100644
--- a/stdlib/array.ncl
+++ b/stdlib/array.ncl
@@ -355,8 +355,20 @@
         "%
       = fun pred l => fold_right (fun x acc => if pred x then true else acc) false l,
 
+    # **Warning**: unless you know what you're doing, please don't change the
+    # type of `elem` to be polymorphic.
+    # 
+    # `elem` must operate on elements of type `Dyn` only, because `elem` performs
+    # equality tests, and those aren't allowed on generic variables (as it
+    # breaks parametricity).
+    #
+    # However, as a current work-around to use equality easily in typed code,
+    # `(==)` has type `forall a. a -> a -> Bool`. We could
+    # be tempted to assign the type `elem : forall a. a -> Array a -> Bool`, but
+    # that would be lying (as `==` is currently lying as well), and more
+    # importantly, the contract will fail on any non-trivial call at run-time.
     elem
-      : forall a. a -> Array a -> Bool
+      : Dyn -> Array Dyn -> Bool
       | doc m%"
           Returns true if the given value appears in the array, false otherwise.
 
diff --git a/utilities/src/lib.rs b/utilities/src/lib.rs
index 2a6ffac6..ef55567d 100644
--- a/utilities/src/lib.rs
+++ b/utilities/src/lib.rs
@@ -8,7 +8,7 @@ use nickel_lang::{
         cache::{Cache as EvalCache, CacheImpl},
         VirtualMachine,
     },
-    parser::{grammar, lexer},
+    parser::{grammar, lexer, ErrorTolerantParser},
     program::Program,
     term::{RichTerm, Term},
     transform::import_resolution,
@@ -36,7 +36,7 @@ pub fn parse(s: &str) -> Result<RichTerm, ParseError> {
     let id = Files::new().add("<test>", String::from(s));
 
     grammar::TermParser::new()
-        .parse_term(id, lexer::Lexer::new(s))
+        .parse_strict(id, lexer::Lexer::new(s))
         .map_err(|errs| errs.errors.first().unwrap().clone())
 }
 


Test patch:
diff --git a/src/eval/tests.rs b/src/eval/tests.rs
index 87ba1043..4c88d6bf 100644
--- a/src/eval/tests.rs
+++ b/src/eval/tests.rs
@@ -3,7 +3,7 @@ use super::*;
 use crate::cache::resolvers::{DummyResolver, SimpleResolver};
 use crate::error::ImportError;
 use crate::label::Label;
-use crate::parser::{grammar, lexer};
+use crate::parser::{grammar, lexer, ErrorTolerantParser};
 use crate::term::make as mk_term;
 use crate::term::Number;
 use crate::term::{BinaryOp, StrChunk, UnaryOp};
@@ -22,7 +22,7 @@ fn parse(s: &str) -> Option<RichTerm> {
     let id = Files::new().add("<test>", String::from(s));
 
     grammar::TermParser::new()
-        .parse_term(id, lexer::Lexer::new(s))
+        .parse_strict(id, lexer::Lexer::new(s))
         .map(RichTerm::without_pos)
         .map_err(|err| println!("{err:?}"))
         .ok()
diff --git a/src/parser/tests.rs b/src/parser/tests.rs
index e8021bff..4ba5c8d9 100644
--- a/src/parser/tests.rs
+++ b/src/parser/tests.rs
@@ -4,7 +4,7 @@ use super::lexer::{Lexer, MultiStringToken, NormalToken, StringToken, SymbolicSt
 use super::utils::{build_record, FieldPathElem};
 use crate::error::ParseError;
 use crate::identifier::Ident;
-use crate::parser::error::ParseError as InternalParseError;
+use crate::parser::{error::ParseError as InternalParseError, ErrorTolerantParser};
 use crate::term::array::Array;
 use crate::term::Number;
 use crate::term::Term::*;
@@ -19,7 +19,7 @@ fn parse(s: &str) -> Result<RichTerm, ParseError> {
     let id = Files::new().add("<test>", String::from(s));
 
     super::grammar::TermParser::new()
-        .parse_term(id, Lexer::new(s))
+        .parse_strict(id, Lexer::new(s))
         .map_err(|errs| errs.errors.first().unwrap().clone())
 }
 
diff --git a/tests/integration/typecheck_fail.rs b/tests/integration/typecheck_fail.rs
index 10c3f432..74442fc5 100644
--- a/tests/integration/typecheck_fail.rs
+++ b/tests/integration/typecheck_fail.rs
@@ -2,7 +2,7 @@ use assert_matches::assert_matches;
 use codespan::Files;
 use nickel_lang::cache::resolvers::DummyResolver;
 use nickel_lang::error::TypecheckError;
-use nickel_lang::parser::{grammar, lexer};
+use nickel_lang::parser::{grammar, lexer, ErrorTolerantParser};
 use nickel_lang::term::RichTerm;
 use nickel_lang::types::{TypeF, Types};
 use nickel_lang::{typecheck, typecheck::Context};
@@ -17,7 +17,7 @@ fn type_check_expr(s: impl std::string::ToString) -> Result<(), TypecheckError>
     let id = Files::new().add("<test>", s.clone());
     type_check(
         &grammar::TermParser::new()
-            .parse_term(id, lexer::Lexer::new(&s))
+            .parse_strict(id, lexer::Lexer::new(&s))
             .unwrap(),
     )
 }