add source to diagnostics

[vemoo]

This will help with #17, by making it easier to know for which files the reference parser should emit an error.

We could also complete the expected diagnostics in tests to include the source, but I'm not sure if that is worth it. At the moment we only distinguish between runtime and compile-time diagnostics.

[netlify]

✅ Deploy Preview for dada-lang ready!

Name	Link
🔨 Latest commit	b068441
🔍 Latest deploy log	https://app.netlify.com/sites/dada-lang/deploys/62ee9ff348a87000090be261
😎 Deploy Preview	https://deploy-preview-204--dada-lang.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

---

To edit notification comments on pull requests, go to your Netlify site settings.

[nikomatsakis]

@vemoo -- I'm not sure how this helps, can you explain a bit more?

(Also, it needs a rebase, if nothing else, sorry about that...)

[vemoo]

To verify the tree-sitter grammar I added this: https://github.com/vemoo/dada/blob/af9cdc0c61cb6de421c733fd73fa167ed6ac716b/components/dada-lang/src/test_harness.rs#L179-L199

But I don't think the grammar should catch validation errors, so we need to know which ones are from lexing or parsing.

I see that brson here: https://github.com/brson/dada/blob/e2861cc6b0c00af2a4e52aa7d6ad93813ad58818/components/dada-lang/src/test_harness.rs#L519-L549 is comparing against the expected diagnostics. So maybe another option is to juste add something like #! PARSE ERROR?

[nikomatsakis]

Ah, I see. So the idea is that we (internally) track the phase of the compiler the error comes from, for the purpose of our testing harness? In particular, we want to know whether it got a parse error, so that we can compare that against the reference grammar?

[nikomatsakis]

What if instead we just distinguish parse error vs other error, and we make a separate macro parse_error or something like that? This patch seems kind of noisy and makes reporting errors more confusing. I feel like sometimes errors may change whether they are detected at validation or not and it'd be annoying to wonder if this will affect any tooling (same is true of parse errors, but they seem less likely to change). It'd also be good to document clearly the reason for the is_parse_error: bool flag on diagnostics and its role in testing.

[vemoo]

Maybe instead of parse_error it makes more sense to call it grammar_error? Because it will also include lexing errors.

[vemoo]

With this changes maybe it doesn't make sense to merge it as a separate PR and it should be part of the reference grammar PR?

[nikomatsakis]

@vemoo I was just thinking -- another option is, rather than modifying the Diagnostic struct, it's possible to ask for only the errors that resulted from parsing a given file.

The code for getting diagnostics today is:

dada/components/dada-db/src/lib.rs

Lines 55 to 60 in 0199d1b

	/// Checks `input_file` for compilation errors and returns all relevant diagnostics.
	pub fn diagnostics(&self, input_file: InputFile) -> Vec<Diagnostic> {
	dada_check::check_input_file::accumulated::<dada_ir::diagnostic::Diagnostics>(
	self, input_file,
	)
	}

but we could add another method:

    /// Checks `input_file` for parsing errors and returns relevant diagnostics.
    pub fn grammar_diagnostics(&self, input_file: InputFile) -> Vec<Diagnostic> {
        dada_parse::file_parser::parse_file::<dada_ir::diagnostic::Diagnostics>(
            self, input_file,
        )
    }

and it would just the diagnostics emitted by the parser. Maybe that'd be easier still?

[vemoo]

Yeah, I like that. It ended up being a little more complicated: https://github.com/vemoo/dada/blob/29b3718387fe2ae3d4e5498455165d24ac1087b2/components/dada-parse/src/lib.rs#L26-L50 because parse_file leaves some parts unparsed as TokenTrees.

I'm going to close this PR because this change will make more sense in the context of adding a reference grammar.