qsharp-compiler icon indicating copy to clipboard operation
qsharp-compiler copied to clipboard

Refactor compiler diagnostic data structures

Open bamarsha opened this issue 4 years ago • 1 comments

  • Diagnostics need additional information to display a diagnostic message, but each diagnostic is an enum item, which doesn't allow additional data. The information is stored as a string sequence which is error-prone and causes confusion since each string is referred to by its index instead of a name.
  • Diagnostic enums are grouped by severity, but severity should be a separate configurable property of each diagnostic, not hard-coded into its name. This also means that diagnostic codes are duplicated across severities, which is confusing.
  • Diagnostic items require a range, but not all diagnostics have a range, which causes a default range to be used as a fallback throughout the compiler whenever a diagnostic is created. Diagnostics should make the range optional and use a standard default value to reduce boilerplate/code duplication.

bamarsha avatar May 10 '21 17:05 bamarsha

An example of where the code for diagnostic messages starts to get really messy and hard to maintain is when we want to make the diagnostic message a bit smarter/adaptable to the context:

https://github.com/microsoft/qsharp-compiler/blob/29d10588c2b85a5ffe8e4243eb94cd96f1a3adbc/src/QsCompiler/DataStructures/Diagnostics.fs#L474-L481

(I don't think it's right to create separate diagnostics for the two messages, because they really are the same diagnostic, just displayed differently depending on context.)

bamarsha avatar Apr 04 '22 16:04 bamarsha