Improve LSP error reporting (#1950)

- [x] Actually run papercut, well-formed, and synthesis-papercut passes
- [x] support multiple errors in a single file
- [x] figure out what to do about primitive files that have no main
component
- [x] Report parsing errors at reported location
- [x] handle errors that don't report location
- [x] add infrastructure for reporting warnings

calyx-frontend/src/ast.rs

@@ -2,18 +2,18 @@
 use super::parser;
 use crate::{Attributes, PortDef, Primitive};
 use atty::Stream;
-use calyx_utils::{CalyxResult, Error, GPosIdx, Id};
+use calyx_utils::{CalyxResult, Error, GPosIdx, Id, PosString};
 use std::{num::NonZeroU64, path::PathBuf};
 
 /// Corresponds to an individual Calyx file.
 #[derive(Debug)]
 pub struct NamespaceDef {
     /// Path to extern files.
-    pub imports: Vec<String>,
+    pub imports: Vec<PosString>,
     /// List of component definitions.
     pub components: Vec<ComponentDef>,
     /// Extern statements and any primitive declarations in them.
-    pub externs: Vec<(Option<String>, Vec<Primitive>)>,
+    pub externs: Vec<(Option<PosString>, Vec<Primitive>)>,
     /// Optional opaque metadata
     pub metadata: Option<String>,
 }

calyx-frontend/src/parser.rs

@@ -6,7 +6,7 @@ use super::ast::{
 };
 use super::Attributes;
 use crate::{Attribute, Direction, PortDef, Primitive, Width};
-use calyx_utils::{self, CalyxResult, Id};
+use calyx_utils::{self, CalyxResult, Id, PosString};
 use calyx_utils::{FileIdx, GPosIdx, GlobalPositionTable};
 use pest::pratt_parser::{Assoc, Op, PrattParser};
 use pest_consume::{match_nodes, Error, Parser};
@@ -66,25 +66,16 @@ impl CalyxParser {
             CalyxParser::parse_with_userdata(Rule::file, content, user_data)
                 .map_err(|e| e.with_path(&path.to_string_lossy()))
                 .map_err(|e| {
-                    calyx_utils::Error::misc(format!(
-                        "Failed to parse `{}`: {err}",
-                        path.to_string_lossy(),
-                        err = e
-                    ))
+                    calyx_utils::Error::parse_error(e.variant.message())
+                        .with_pos(&Self::error_span(&e, file))
                 })?;
         let input = inputs.single().map_err(|e| {
-            calyx_utils::Error::misc(format!(
-                "Failed to parse `{}`: {err}",
-                path.to_string_lossy(),
-                err = e
-            ))
+            calyx_utils::Error::parse_error(e.variant.message())
+                .with_pos(&Self::error_span(&e, file))
         })?;
         let out = CalyxParser::file(input).map_err(|e| {
-            calyx_utils::Error::misc(format!(
-                "Failed to parse `{}`: {err}",
-                path.to_string_lossy(),
-                err = e
-            ))
+            calyx_utils::Error::parse_error(e.variant.message())
+                .with_pos(&Self::error_span(&e, file))
         })?;
         log::info!(
             "Parsed `{}` in {}ms",
@@ -110,15 +101,16 @@ impl CalyxParser {
         let inputs =
             CalyxParser::parse_with_userdata(Rule::file, contents, user_data)
                 .map_err(|e| {
-                calyx_utils::Error::misc(
-                    format!("Failed to parse buffer: {e}",),
-                )
+                calyx_utils::Error::parse_error(e.variant.message())
+                    .with_pos(&Self::error_span(&e, file))
             })?;
         let input = inputs.single().map_err(|e| {
-            calyx_utils::Error::misc(format!("Failed to parse buffer: {e}",))
+            calyx_utils::Error::parse_error(e.variant.message())
+                .with_pos(&Self::error_span(&e, file))
         })?;
         let out = CalyxParser::file(input).map_err(|e| {
-            calyx_utils::Error::misc(format!("Failed to parse buffer: {e}",))
+            calyx_utils::Error::parse_error(e.variant.message())
+                .with_pos(&Self::error_span(&e, file))
         })?;
         Ok(out)
     }
@@ -134,6 +126,15 @@ impl CalyxParser {
         GPosIdx(pos)
     }
 
+    fn error_span(error: &pest::error::Error<Rule>, file: FileIdx) -> GPosIdx {
+        let (start, end) = match error.location {
+            pest::error::InputLocation::Pos(off) => (off, off + 1),
+            pest::error::InputLocation::Span((start, end)) => (start, end),
+        };
+        let pos = GlobalPositionTable::as_mut().add_pos(file, start, end);
+        GPosIdx(pos)
+    }
+
     #[allow(clippy::result_large_err)]
     fn guard_expr_helper(
         ud: UserData,
@@ -188,7 +189,7 @@ impl CalyxParser {
 
 #[allow(clippy::large_enum_variant)]
 enum ExtOrComp {
-    Ext((Option<String>, Vec<Primitive>)),
+    Ext((Option<PosString>, Vec<Primitive>)),
     Comp(ComponentDef),
     PrimInline(Primitive),
 }
@@ -350,18 +351,19 @@ impl CalyxParser {
         Ok(input.as_str())
     }
 
-    fn string_lit(input: Node) -> ParseResult<String> {
+    fn string_lit(input: Node) -> ParseResult<PosString> {
+        let span = Self::get_span(&input);
         Ok(match_nodes!(
             input.into_children();
-            [char(c)..] => c.collect::<Vec<_>>().join("")
+            [char(c)..] => PosString::new(c.collect::<Vec<_>>().join(""), span)
         ))
     }
 
     // ================ Attributes =====================
     fn attribute(input: Node) -> ParseResult<(Attribute, u64)> {
         match_nodes!(
             input.clone().into_children();
-            [string_lit(key), bitwidth(num)] => Attribute::from_str(&key).map(|attr| (attr, num)).map_err(|e| input.error(format!("{:?}", e)))
+            [string_lit(key), bitwidth(num)] => Attribute::from_str(key.as_ref()).map(|attr| (attr, num)).map_err(|e| input.error(format!("{:?}", e)))
         )
     }
     fn attributes(input: Node) -> ParseResult<Attributes> {
@@ -1199,14 +1201,14 @@ impl CalyxParser {
         )
     }
 
-    fn imports(input: Node) -> ParseResult<Vec<String>> {
+    fn imports(input: Node) -> ParseResult<Vec<PosString>> {
         Ok(match_nodes!(
             input.into_children();
             [string_lit(path)..] => path.collect()
         ))
     }
 
-    fn ext(input: Node) -> ParseResult<(Option<String>, Vec<Primitive>)> {
+    fn ext(input: Node) -> ParseResult<(Option<PosString>, Vec<Primitive>)> {
         Ok(match_nodes!(
             input.into_children();
             [string_lit(file), primitive(prims)..] => (Some(file), prims.collect())

calyx-frontend/src/workspace.rs

@@ -3,7 +3,7 @@ use super::{
     parser,
 };
 use crate::LibrarySignatures;
-use calyx_utils::{CalyxResult, Error};
+use calyx_utils::{CalyxResult, Error, WithPos};
 use std::{
     collections::HashSet,
     path::{Path, PathBuf},
@@ -65,7 +65,7 @@ impl Workspace {
         lib_path: &Path,
     ) -> CalyxResult<PathBuf>
     where
-        S: AsRef<Path> + Clone,
+        S: AsRef<Path> + Clone + WithPos,
     {
         let absolute_import = import.as_ref();
         if absolute_import.is_absolute() && absolute_import.exists() {
@@ -87,7 +87,7 @@ impl Workspace {
             import.as_ref().to_string_lossy(),
             parent.to_string_lossy(),
             lib_path.to_string_lossy()
-        )))
+        )).with_pos(&import))
     }
 
     // Get the absolute path to an extern. Extern can only exist on paths
@@ -98,17 +98,19 @@ impl Workspace {
         parent: &Path,
     ) -> CalyxResult<PathBuf>
     where
-        S: AsRef<Path> + Clone,
+        S: AsRef<Path> + Clone + WithPos,
     {
-        let parent_path = parent.join(extern_path.clone()).canonicalize()?;
-        if parent_path.exists() {
-            return Ok(parent_path);
-        }
-        Err(Error::invalid_file(format!(
-            "Extern path `{}` not found in parent directory ({})",
-            extern_path.as_ref().to_string_lossy(),
-            parent.to_string_lossy(),
-        )))
+        parent
+            .join(extern_path.clone())
+            .canonicalize()
+            .map_err(|_| {
+                Error::invalid_file(format!(
+                    "Extern path `{}` not found in parent directory ({})",
+                    extern_path.as_ref().to_string_lossy(),
+                    parent.to_string_lossy(),
+                ))
+                .with_pos(&extern_path)
+            })
     }
 
     /// Construct a new workspace using the `compile.futil` library which
@@ -273,7 +275,8 @@ impl Workspace {
         })?;
 
         // Add original imports to workspace
-        ws.original_imports = ns.imports.clone();
+        ws.original_imports =
+            ns.imports.iter().map(|imp| imp.to_string()).collect();
 
         // TODO (griffin): Probably not a great idea to clone the metadata
         // string but it works for now

calyx-ir/src/from_ast.rs

@@ -182,16 +182,20 @@ pub fn ast_to_ir(mut workspace: Workspace) -> CalyxResult<Context> {
     let mut all_names: HashSet<&Id> =
         HashSet::with_capacity(workspace.components.len() + prims.len());
 
-    let prim_names = prims.iter().map(|p| &p.name);
+    let prim_names = prims.iter().map(|p| (&p.name, p.attributes.copy_span()));
 
-    let comp_names = workspace.components.iter().map(|comp| &comp.name);
+    let comp_names = workspace
+        .components
+        .iter()
+        .map(|comp| (&comp.name, comp.attributes.copy_span()));
 
-    for bound in prim_names.chain(comp_names) {
+    for (bound, span) in prim_names.chain(comp_names) {
         if all_names.contains(bound) {
             return Err(Error::already_bound(
                 *bound,
                 "component or primitive".to_string(),
-            ));
+            )
+            .with_pos(&span));
         }
         all_names.insert(bound);
     }

calyx-lsp/Cargo.toml

@@ -27,6 +27,7 @@ default = ["diagnostics"]
 calyx-frontend.workspace = true
 calyx-ir.workspace = true
 calyx-utils.workspace = true
+calyx-opt.workspace = true
 chrono = "0.4.33"
 itertools.workspace = true
 regex = "1.10.3"