You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Copying a comment from JuliaLang/julia#36547 (comment) to avoid forgetting it, here's some thoughts about how we might integrate diagnostics such as warnings better with Base.
Diagnostics design sketch
Ok. How about the following sketch for how diagnostics could flow through the parser and runtime:
Change Core._parse API such that, instead of returning svec(ex, last_offset), it returns a svec(ex, last_offset, diagnostics). Note that in general, compiler diagnostics do not need to align with the tree structure returned in ex. So having them in a separate list is more flexible. See also the Expr(:diagnostics) design alternative below
Figure out the API for the diagnostics data structure. Probably show and some kind of other summary function (Meta.diagnostic_summary? - see discussion below)
Modify the Meta.parse() machinery for the new Core._parse, in some way that it can (optionally I guess, for compatibility?) return diagnostics to the caller. In JuliaSyntax.parse() we have ignore_warnings
Change jlfrontend.scm and julia-parser.scm to use a dynamically scoped list to collect warnings during parsing, as mentioned in a previous comment. Change the runtime C function fl_parse to deal with the diagnostics list returned.
Change the use of Meta.parse within Base and the runtime to request diagnostics and report them in some way. For the use in loading.jl in include/include_string, presumably they would go via the logging system, as we're already running Julia code there. Same for the use of parseall in client.jl and parse_input_line in the REPL. (side note - parse_input_line() predates parseall() but in principle should just be replaced with parseall())
Users may still call into the C runtime functions jl_eval_string / jl_parse_string / jl_parse_all from arbitrary C code. We could report warnings arising from those to stderr? Or just ignore them? It seems like an edge case we could ignore.
Design alternative with Expr(:diagnostics)
A possible alternative to returning the diagnostics separately from Core._parse is to encode them into the expression tree, somewhat similarly to how Expr(:error) is done. However, I think it best to encode them in the root of the tree as Expr(:diagnostics, ex, diagnostics), as they're not part of the tree structure per se. This is different from how Expr(:error) currently gets tacked onto the end of the Expr(:toplevel) args list.
Then report any diagnostics which exist at eval() time. This might be neater and more easy to make compatible than threading a separate diagnostics data structure around everywhere - implementing will tell. The advantages of this approach is that it could allow us to have warnings on in Meta.parse() by default. And also that there's a chance that they could survive flowing through user code (including macros) and back into eval(). (But tbh I'm not sure these features are entirely necessary... needs thought.)
Source Diagnostics data structures / API
One straightforward concrete option would be a Vector of (level, first_byte, last_byte, message) where level can be :error, :warn (or maybe :info) - first_byte:last_byte, refers to a source range of the input text, and message is a string. This is basically just the JuliaSyntax.Diagnostic data structure.
A challenge here is I don't think JuliaSyntax.Diagnostic is "finished" and I'd like flexibility to expand it in the future. For example, adding
Sub-ranges for highlighting and additional message structure
Continue to work on pretty printing. Including
Printing for mime types other than text
Formatting multiple diagnostics on a single line in a way which only prints the source line once.
So a better option than requiring a particular data structure for diagnostics is probably to rely on generic functions - minimal, just enough for the runtime to query the diagnostics data structure as necessary. show(io, diagnostics) being an obvious one for pretty printing. But perhaps also an analogue of JuliaSyntax.any_error() to summarize the diagnostics. Maybe it could be Meta.diagnostic_summary or some such so we can distinguish between errors and warnings. (Not sure if it needs to be in Core? Needs investigation.)
The text was updated successfully, but these errors were encountered:
Copying a comment from JuliaLang/julia#36547 (comment) to avoid forgetting it, here's some thoughts about how we might integrate diagnostics such as warnings better with Base.
Diagnostics design sketch
Ok. How about the following sketch for how diagnostics could flow through the parser and runtime:
Core._parse
API such that, instead of returningsvec(ex, last_offset)
, it returns asvec(ex, last_offset, diagnostics)
. Note that in general, compiler diagnostics do not need to align with the tree structure returned inex
. So having them in a separate list is more flexible. See also theExpr(:diagnostics)
design alternative belowdiagnostics
data structure. Probablyshow
and some kind of other summary function (Meta.diagnostic_summary
? - see discussion below)Meta.parse()
machinery for the newCore._parse
, in some way that it can (optionally I guess, for compatibility?) return diagnostics to the caller. InJuliaSyntax.parse()
we haveignore_warnings
jlfrontend.scm
andjulia-parser.scm
to use a dynamically scoped list to collect warnings during parsing, as mentioned in a previous comment. Change the runtime C functionfl_parse
to deal with the diagnostics list returned.Meta.parse
withinBase
and the runtime to request diagnostics and report them in some way. For the use inloading.jl
ininclude/include_string
, presumably they would go via the logging system, as we're already running Julia code there. Same for the use ofparseall
inclient.jl
andparse_input_line
in the REPL. (side note -parse_input_line()
predatesparseall()
but in principle should just be replaced withparseall()
)jl_eval_string
/jl_parse_string
/jl_parse_all
from arbitrary C code. We could report warnings arising from those tostderr
? Or just ignore them? It seems like an edge case we could ignore.Design alternative with
Expr(:diagnostics)
A possible alternative to returning the diagnostics separately from
Core._parse
is to encode them into the expression tree, somewhat similarly to howExpr(:error)
is done. However, I think it best to encode them in the root of the tree asExpr(:diagnostics, ex, diagnostics)
, as they're not part of the tree structure per se. This is different from howExpr(:error)
currently gets tacked onto the end of theExpr(:toplevel)
args list.Then report any diagnostics which exist at
eval()
time. This might be neater and more easy to make compatible than threading a separate diagnostics data structure around everywhere - implementing will tell. The advantages of this approach is that it could allow us to have warnings on inMeta.parse()
by default. And also that there's a chance that they could survive flowing through user code (including macros) and back intoeval()
. (But tbh I'm not sure these features are entirely necessary... needs thought.)Source Diagnostics data structures / API
One straightforward concrete option would be a
Vector
of(level, first_byte, last_byte, message)
wherelevel
can be:error
,:warn
(or maybe:info
) -first_byte:last_byte
, refers to a source range of the input text, andmessage
is a string. This is basically just theJuliaSyntax.Diagnostic
data structure.A challenge here is I don't think
JuliaSyntax.Diagnostic
is "finished" and I'd like flexibility to expand it in the future. For example, addingSo a better option than requiring a particular data structure for diagnostics is probably to rely on generic functions - minimal, just enough for the runtime to query the diagnostics data structure as necessary.
show(io, diagnostics)
being an obvious one for pretty printing. But perhaps also an analogue ofJuliaSyntax.any_error()
to summarize the diagnostics. Maybe it could beMeta.diagnostic_summary
or some such so we can distinguish between errors and warnings. (Not sure if it needs to be inCore
? Needs investigation.)The text was updated successfully, but these errors were encountered: