-
Notifications
You must be signed in to change notification settings - Fork 45
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Report multiple errors #1924
Comments
So near as I can tell, this is one of those things that seems like it should be easy and is actually quite hard. I found a few blog posts that are kinda about the topic but haven't had a chance to read them yet so I won't link them here, but everything I've seen suggests a different general "shape" for programs that want to do this type of error collecting. Instead of having the return type be The biggest problem of course is that this requires the program to continue an execution partially under a lot of circumstances which is not what |
Filament used to do this by passing a mutable Another option is librarize-ing the analysis done by papercut and well-formed so we can write a different LSP analysis tool instead of relying on the passes to perform the checking for us. |
Thanks for the thought, @sgpthomas! Yes, this would be a cool thing to do now that Calyx is kinda grown up. Yep, @EclecticGriffin is right that there is no real shortcut here. Basically, grown-up compilers do what @rachitnigam alluded to: there is a "reporter" thing that gets passed around mutably one way or another, and specific passes/checks call a method on that to accumulate reports. @rachitnigam, you make a good point that we could consider avoiding disturbing the compiler too much by doing what you say. Philosophically, this would amount to a recognition that the papercut pass and well-formedness pass are different from all other passes; other passes do not wish to report interesting human-visible diagnostics and would prefer to just abort on first error for efficiency reasons. So with that in mind, it does not make sense to change all the passes in the world (and the top-level compiler entry point) to support a use case that is actually much more narrow than the compiler as a whole. |
Before I go to much further with my implementation, I'm curious to gather thoughts on what I've done so far. Work is in this PR: #1950. I've introduced a This makes "multiple error reporting" an opt-in thing for passes, and doesn't require any changes for existing passes. Does this seem like a reasonable approach? |
@sgpthomas I really like this approach! One tricky thing is making sure that the error reporting code does not assume that bail outs will occur. For example, if a particular program is malformed in some way, it might cause a cascade of errors that all boil down to the same error location. Also, we should just take advantage of this in the normal compiler flow. No need to just report just the first error. |
At the moment, we only report the first error found in a file. This is the default way that error handling works in Rust because of how
?
works. For the language server it would be super neat if we could report all the errors found in a file, so that we could display them all at once.I'm not exactly sure what the best way to implement this is. I don't know if this is possible while using the
?
operator. Something to look into.The text was updated successfully, but these errors were encountered: