Merge pull request #1083 from hash-org/fix-parser-bug

Fix several parser bugs

compiler/hash-parser/src/parser/expr.rs

@@ -357,11 +357,18 @@ impl AstGen<'_> {
     pub(crate) fn parse_singular_expr(
         &mut self,
         mut subject: AstNode<Expr>,
-        subject_span: ByteRange,
+        mut subject_span: ByteRange,
     ) -> ParseResult<AstNode<Expr>> {
         // so here we need to peek to see if this is either a index_access, field access
         // or a function call...
         while let Some(token) = self.peek() {
+            // if there exists a space between the `subject` and the
+            // next fragment of the expression, we treat them as explicitly
+            // non-singular expressions.
+            if !token.span.is_right_before(subject_span) {
+                break;
+            }
+
             subject = match token.kind {
                 // Property access or method call
                 TokenKind::Dot => self.parse_property_access(subject, subject_span)?,
@@ -382,7 +389,11 @@ impl AstGen<'_> {
                 // Function call
                 TokenKind::Tree(Delimiter::Paren, _) => self.parse_call(subject, subject_span)?,
                 _ => break,
-            }
+            };
+
+            // We need to adjust the subject_span so we can compute whether
+            // we're still "physically" connected to the end of the expression.
+            subject_span = subject_span.join(self.previous_pos())
         }
 
         Ok(subject)
@@ -752,7 +763,10 @@ impl AstGen<'_> {
 
                 // We want to emit a redundant parentheses warning if it is not a binary-like
                 // expression since it does not affect the precedence...
-                if !matches!(expr.body(), Expr::BinaryExpr(_) | Expr::Cast(_) | Expr::FnDef(_)) {
+                if !matches!(
+                    expr.body(),
+                    Expr::BinaryExpr(_) | Expr::Cast(_) | Expr::FnDef(_) | Expr::Deref(_)
+                ) {
                     gen.add_warning(ParseWarning::new(
                         WarningKind::RedundantParenthesis(expr.body().into()),
                         gen.make_span(gen.range()),

compiler/hash-parser/src/parser/mod.rs

@@ -32,7 +32,7 @@ use crate::{
     import_resolver::ImportResolver,
 };
 
-#[derive(Deref)]
+#[derive(Deref, Debug)]
 pub(crate) struct AstGenFrame<'s> {
     /// The token cursor.
     #[deref]
@@ -108,6 +108,12 @@ impl<'s> AstGenFrame<'s> {
         let pos = self.previous_pos().end() + 1;
         ByteRange::new(pos, pos)
     }
+
+    /// Check whether the frame has encountered an error.
+    #[inline(always)]
+    pub(crate) fn has_error(&self) -> bool {
+        self.error.get()
+    }
 }
 
 /// The [AstGen] struct it the primary parser for the Hash compiler. It

compiler/hash-parser/src/parser/ty.rs

@@ -328,6 +328,14 @@ impl AstGen<'_> {
                 ),
             };
 
+            // Abort early if we encountered some kind of error along the way,
+            // although I would think when the `gen` is consumed then we can
+            // send up all of the errors to the parent generator?
+            if gen.has_error() {
+                let err = gen.diagnostics.errors.pop().unwrap();
+                return Err(err);
+            }
+
             // Here we check that the token tree has a comma at the end to later determine
             // if this is a `TupleTy`...
             let gen_has_comma = !gen.is_empty() && gen.previous_token().has_kind(TokenKind::Comma);

compiler/hash-reporting/src/macros.rs

@@ -1,6 +1,6 @@
 //! Utility macros for performing various operations when it comes to
 //! working with reports
-use hash_source::location::Span;
+use hash_source::{location::Span, ModuleId, ModuleKind};
 #[allow(unused_imports)]
 use hash_utils::stream_less_ewriteln;
 
@@ -28,12 +28,59 @@ pub macro panic_on_span {
     }
 }
 
+/// A guard on when to print a particular message using `note_on_span`
+/// and `panic_on_span` family functions.
+#[derive(Clone, Copy)]
+pub enum SpanGuard {
+    /// No guard, we're always printing for any given module.
+    Always,
+
+    /// Guarded by a specific module.
+    Module(ModuleId),
+
+    /// Non-prelude print, we'll only print something when
+    /// we're in a non-prelude context.
+    NonPrelude,
+
+    /// Only print on the specified entry point of the given
+    /// workspace, handy for when debugging simple things.
+    EntryPoint,
+}
+
 /// This macro will produce a [crate::report::Report] and then print it to the
 /// standard output, this does not panic, it is intended as a debugging utility
 /// to quickly print the `span` of something and the `message` associated with
 /// it.
-///
-/// Examples of use:
+#[track_caller]
+pub fn guarded_note_on_span(location: impl Into<Span>, message: impl ToString, guard: SpanGuard) {
+    let span: Span = location.into();
+
+    let should_execute = match (guard, span.id) {
+        (SpanGuard::Always, _) => true,
+        (SpanGuard::Module(id), span) if span.is_module() => id == ModuleId::from(span),
+        (SpanGuard::NonPrelude, span) => !span.is_prelude(),
+        (SpanGuard::EntryPoint, span) => matches!(span.module_kind(), Some(ModuleKind::EntryPoint)),
+        _ => false,
+    };
+
+    // Don't report if the span guard isn't met.
+    if !should_execute {
+        return;
+    }
+
+    let mut reporter = Reporter::new();
+    reporter
+        .info()
+        .title(message)
+        .add_labelled_span(span, "here")
+        .add_note(format!("invoked at {}", ::core::panic::Location::caller()));
+
+    stream_less_ewriteln!("{}", reporter);
+}
+
+/// This macro will produce a [crate::report::Report] and then print it to the
+/// standard output, this does not panic, it is intended as a debugging utility
+/// for use when debugging the compiler.
 ///
 /// ```rust
 /// // Don't print on `prelude` module.
@@ -42,16 +89,11 @@ pub macro panic_on_span {
 /// // Always print.
 /// note_on_span(item.span(), "compiling `item`");
 /// ```
-#[track_caller]
+///
+/// If you want to only print in certain conditions, i.e. only the entry point,
+/// then you can use [guarded_note_on_span] instead.
 pub fn note_on_span(location: impl Into<Span>, message: impl ToString) {
-    let mut reporter = Reporter::new();
-    reporter
-        .info()
-        .title(message)
-        .add_labelled_span(location.into(), "here")
-        .add_note(format!("invoked at {}", ::core::panic::Location::caller()));
-
-    stream_less_ewriteln!("{}", reporter);
+    guarded_note_on_span(location, message, SpanGuard::Always);
 }
 
 /// A macro which doesn't invoke the printing of a given format expression when

compiler/hash-source/src/location.rs

@@ -85,6 +85,20 @@ impl ByteRange {
     pub fn into_location(self, source_id: SourceId) -> Span {
         Span::new(self, source_id)
     }
+
+    /// Check if one span intersects with another one.
+    ///
+    /// We check whether the `current` token touches the other token, if it
+    /// does, then we consider it to be intersecting.
+    /// ```
+    /// 
+    ///  (current token)
+    ///                 (other token)
+    /// ```
+    #[inline(always)]
+    pub fn is_right_before(&self, other: Self) -> bool {
+        self.start() == (other.end() + 1)
+    }
 }
 
 impl Default for ByteRange {

compiler/hash-token/src/cursor.rs

@@ -51,6 +51,7 @@ pub type TokenStream<'t> = &'t [Token];
 
 /// A cursor over a token stream which handles internal traversal logic
 /// and bookkeeping.
+#[derive(Debug)]
 pub struct TokenCursor<'t> {
     /// The raw stream of tokens that is being traversed.
     stream: TokenStream<'t>,

compiler/hash-token/src/lib.rs

@@ -53,6 +53,9 @@ impl Token {
     /// order to print literals.
     ///
     /// ##Note: this does not print trees.
+    ///
+    /// @@Todo: make this in a "TokenPrinter" context so that we don't
+    /// need to explicitly make this a method on token.
     pub fn pretty_print(&self, source: SpannedSource<'_>) -> String {
         match self.kind {
             TokenKind::Int(_, _) | TokenKind::Float(_) | TokenKind::Char(_) | TokenKind::Str(_) => {

tests/cases/parser/blocks/empty_for_loop.hash

@@ -4,6 +4,6 @@ main := () => {
     // Test that the parser shouldn't get confused between a struct literal and a for block
     for x in xs {}
 
-    // Test that struct literals are allowed in parenthesees...
-    for x in (Struct ( xs )) {}
+    // Test that struct literals are allowed in parentheses...
+    for x in (Struct( xs )) {}
 }

tests/cases/parser/constructors/struct_defn.hash

@@ -7,7 +7,7 @@ Point := struct(
 
 // Test that structs can be initialised with default field value and types.
 Triangle := struct(
-    x: Point = Point (x = 0, y = 0),
+    x: Point = Point(x = 0, y = 0),
     y: Point,
     z := Point( x = 0, y = 0), // Type should be inferred from default value?
 );

tests/cases/parser/misc/while_implicit_call.hash

@@ -0,0 +1,9 @@
+// run=pass, stage=parse, warnings=compare
+
+#dump_ast 
+main := () => {
+    i := 0;
+    while i < (*arr).len {
+        i += 1;
+    }
+}

tests/cases/parser/patterns/list_patterns.hash

@@ -6,7 +6,7 @@
 [a, b] := foo();
 [] := foo();
 
-[x, Struct ( name )] = *mu();
+[x, Struct(name)] = *mu();
 
 [(a, 3) | (a, 2), (c, d), (r, m)] := foo();
 

tests/cases/typecheck/pat_binds/548.hash

@@ -3,12 +3,12 @@
 main := () => {
     // ~ERROR: variable `c` not declared in all patterns
     // ~ERROR: variable `d` not declared in all patterns
-    (c | d) := 2;    
+    (c | d) := 2;
 
     k := (1, 2)
 
     match k {
-        (a, b) | (t, a) => {}
+        (a, b) | (t, a) => {},
         (2, 3) | (a, b) => {}
     }
 }

tests/cases/typecheck/pat_binds/548.stderr

@@ -1,51 +1,77 @@
 error[0081]: variable `c` is not bound in all patterns
  --> $DIR/548.hash:6:10
 5 |       // ~ERROR: variable `d` not declared in all patterns
-6 |       (c | d) := 2;    
+6 |       (c | d) := 2;
   |            ^ pattern doesn't bind `c`
 7 |   
 
  --> $DIR/548.hash:6:6
 5 |       // ~ERROR: variable `d` not declared in all patterns
-6 |       (c | d) := 2;    
+6 |       (c | d) := 2;
   |        ^ variable not in all patterns
 7 |   
 
 error[0081]: variable `d` is not bound in all patterns
  --> $DIR/548.hash:6:6
 5 |       // ~ERROR: variable `d` not declared in all patterns
-6 |       (c | d) := 2;    
+6 |       (c | d) := 2;
   |        ^ pattern doesn't bind `d`
 7 |   
 
  --> $DIR/548.hash:6:10
 5 |       // ~ERROR: variable `d` not declared in all patterns
-6 |       (c | d) := 2;    
+6 |       (c | d) := 2;
   |            ^ variable not in all patterns
 7 |   
 
 error[0081]: variable `b` is not bound in all patterns
   --> $DIR/548.hash:11:18
 10 |       match k {
-11 |           (a, b) | (t, a) => {}
+11 |           (a, b) | (t, a) => {},
    |                    ^^^^^^ pattern doesn't bind `b`
 12 |           (2, 3) | (a, b) => {}
 
   --> $DIR/548.hash:11:13
 10 |       match k {
-11 |           (a, b) | (t, a) => {}
+11 |           (a, b) | (t, a) => {},
    |               ^ variable not in all patterns
 12 |           (2, 3) | (a, b) => {}
 
 error[0081]: variable `t` is not bound in all patterns
   --> $DIR/548.hash:11:9
 10 |       match k {
-11 |           (a, b) | (t, a) => {}
+11 |           (a, b) | (t, a) => {},
    |           ^^^^^^ pattern doesn't bind `t`
 12 |           (2, 3) | (a, b) => {}
 
   --> $DIR/548.hash:11:19
 10 |       match k {
-11 |           (a, b) | (t, a) => {}
+11 |           (a, b) | (t, a) => {},
    |                     ^ variable not in all patterns
 12 |           (2, 3) | (a, b) => {}
+
+error[0081]: variable `a` is not bound in all patterns
+  --> $DIR/548.hash:12:9
+11 |           (a, b) | (t, a) => {},
+12 |           (2, 3) | (a, b) => {}
+   |           ^^^^^^ pattern doesn't bind `a`
+13 |       }
+
+  --> $DIR/548.hash:12:19
+11 |           (a, b) | (t, a) => {},
+12 |           (2, 3) | (a, b) => {}
+   |                     ^ variable not in all patterns
+13 |       }
+
+error[0081]: variable `b` is not bound in all patterns
+  --> $DIR/548.hash:12:9
+11 |           (a, b) | (t, a) => {},
+12 |           (2, 3) | (a, b) => {}
+   |           ^^^^^^ pattern doesn't bind `b`
+13 |       }
+
+  --> $DIR/548.hash:12:22
+11 |           (a, b) | (t, a) => {},
+12 |           (2, 3) | (a, b) => {}
+   |                        ^ variable not in all patterns
+13 |       }