Parser update #7

laurmaedje · 2020-07-27T12:12:30Z

This PR gives the parser and some associated modules a facelift, mostly due to better parsing primitives.

Better parsing primitives leading to simpler code.
More consistent and simpler error handling methods.
The complex parse_collection function has been removed in favor of separate parsers for function arguments, tuples and objects. While this leads to (very) slight duplication, the code is much easier to understand and the error handling way less complicated.
The parser test code was cleaned up a bit, especially through the introduction of the pval! macro which reduces boilerplate for testing expression parsing.
The ParseContext struct was replaced by a ParseState which owns the function scope instead of borrowing it. This will later be useful to support function definitions (as they need to update the scope). As to the name: State fits better than context with the envisioned design of the layouter.
The tokenizer's function names have been changed from parse_<thing> to read_<thing> to better distinguish tokenizing from parsing.

Some extra small things:

The order of the items in the span module was brought into an order which is more consistent with the rest of the project (general to specific / big to small instead of the other way around).
The order of the arguments to the parse function was also updated to better reflect their importance.

reknih

src/syntax/parsing.rs seems to be a bit longer now, but also less nested and the functions make more sense (also, no take!)!
So, overall, I like it! Two minor change requests attached..

reknih · 2020-07-29T14:24:07Z

src/func.rs

@@ -17,7 16,6 @@ pub mod prelude {
 pub use crate::syntax::span::{Span, Spanned};
 }

-


Maybe we should really have automatic formatting to prevent changes like these

Yes, maybe, but it makes some code so horrible!

reknih · 2020-07-29T14:29:42Z

src/lib.rs

- pub fn extend_offset(&mut self, offset: Position, other: Feedback) {
- self.problems.extend(offset_spans(offset, other.problems));
- self.decos.extend(offset_spans(offset, other.decos));
+ pub fn extend_offset(&mut self, other: Feedback, offset: Position) {


I'm aware that this was here before, but since this PR changes the argument orders and other stylicstic stuff, allow me this remark: I'm not sure whether other is a great argument name. If it remains as such, the arguments should be more clearly explained in the Doc-Comment, especially since this is a public function.

I propose changing the argument name to more.

reknih · 2020-07-29T14:32:37Z

src/lib.rs

- /// The standard library scope.
- scope: Scope,
+ /// The base parser state.
+ parse_state: ParseState,


Nice change, makes it really clear where stuff goes!

reknih · 2020-07-29T14:34:13Z

src/macros.rs

+/// ```
+/// try_or!(result, continue);
+/// ```
+macro_rules! try_or {


I really feel like this should be in the language.

reknih · 2020-07-29T14:36:47Z

src/syntax/parsing.rs

-pub struct ParseContext<'a> {
- /// The scope containing function definitions.
- pub scope: &'a Scope,
+/// The state which can influence how a string of source code is parsed.


This is a major improvement over the last Doc-Comment

reknih · 2020-07-29T14:38:28Z

src/syntax/parsing.rs

- // Only at least two newlines mean a _real_ newline indicating a
- // paragraph break.
+ // Starting from two newlines counts as a paragraph break, a single
+ // newline not.


Missing does

reknih · 2020-07-29T14:50:56Z

src/syntax/scope.rs

- .map(|x| &**x)
- .ok_or_else(|| &*self.fallback)
+ pub fn get_parser(&self, name: &str) -> Option<&Parser> {
+ self.parsers.get(name).map(AsRef::as_ref)


More rs::BlackMagic

reknih · 2020-07-29T14:51:38Z

src/syntax/span.rs

@@ -111,56 114,67 @@ impl Span {

 impl Debug for Span {
 fn fmt(&self, f: &mut Formatter) -> fmt::Result {
- write!(f, "({:?} -> {:?})", self.start, self.end)
+ write!(f, "<{:?}-{:?}>", self.start, self.end)


reknih · 2020-07-29T14:52:44Z

src/syntax/tokens.rs

@@ -367,12 368,12 @@ impl<'s> Tokens<'s> {
 (&self.src[start .. end], terminated)
 }

- fn parse_string(&mut self) -> Token<'s> {


Good riddance of parse everywhere..

laurmaedje · 2020-07-29T15:37:44Z

It think src/syntax/parsing.rs is now actually sixty lines shorter!

reknih

LGTM now

laurmaedje added 3 commits July 26, 2020 18:08

Better distinguish function names in tokenizer from parser ✂

ea64ce9

Streamline parser and tokenizer test code ✈

9f40004

Refactor parser 🚇

53ca5a7

laurmaedje requested a review from reknih July 27, 2020 12:12

laurmaedje mentioned this pull request Jul 27, 2020

Associativity of multiplication / division #6

Closed

reknih reviewed Jul 29, 2020

View reviewed changes

Improve argument naming and fix grammar in comment ♻

9672d43

reknih approved these changes Jul 29, 2020

View reviewed changes

laurmaedje merged commit f34ba3d into master Jul 29, 2020

laurmaedje deleted the parser-update branch July 29, 2020 15:47

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Parser update #7

Parser update #7

laurmaedje commented Jul 27, 2020 •

edited

Loading

reknih left a comment

reknih Jul 29, 2020

laurmaedje Jul 29, 2020

reknih Jul 29, 2020

laurmaedje Jul 29, 2020

reknih Jul 29, 2020

reknih Jul 29, 2020

reknih Jul 29, 2020

reknih Jul 29, 2020

reknih Jul 29, 2020

laurmaedje Jul 29, 2020

reknih Jul 29, 2020

laurmaedje Jul 29, 2020

reknih Jul 29, 2020

laurmaedje commented Jul 29, 2020

reknih left a comment

		@@ -17,7 16,6 @@ pub mod prelude {
		pub use crate::syntax::span::{Span, Spanned};
		}

Parser update #7

Parser update #7

Conversation

laurmaedje commented Jul 27, 2020 • edited Loading

reknih left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

laurmaedje commented Jul 29, 2020

reknih left a comment

Choose a reason for hiding this comment

laurmaedje commented Jul 27, 2020 •

edited

Loading