Introduce expression/type formatting functor#2192
Conversation
|
So what about the language-specific output? Is it a sound assumption that all symbols in the symbol table will always belong to a single language? |
martin-cs
left a comment
There was a problem hiding this comment.
The changes in "my" part look OK but I have a bigger question -- how is this going to work with expr2c expr2java, expr2whatever, etc. Without some link to that it seems like just extra wrapping.
|
An option is to build expr2c (or other languages) by extending this one; the obvious, low-effort step is to add a wrapper. |
|
Language-specific output: The idea is that we use the language of the application: cbmc prints C syntax, jbmc prints java syntax, adabmc prints, ADA, etc. pp |
|
|
|
If the debug formatter weren't a static object then front-ends could also configure it to produce, e.g., json-compatible output. Thus I'd still like to see this static object go away. |
|
debug formatter: but then pass down a formatter basically everywhere that might want to debug? |
No, you can safely leave the |
|
|
The debug_formatter is now configurable. |
I agree with that in principle, but that requires any such language-independent output to be properly tested and effectively become as good as |
db211d6 to
d46e8e5
Compare
tautschnig
left a comment
There was a problem hiding this comment.
I'm sure this will eventually be valuable, but right now it still has a few issues:
- It is introducing even more objects of static lifetime, which seemed something that was to be gotten rid of.
- There is no sensitivity towards plain text/JSON/XML output, with no apparent way of making this possible.
- Maybe one of the commits could clarify the "why?"
| return formatter->format(os, o); | ||
| } | ||
|
|
||
| formattert *formatter; |
There was a problem hiding this comment.
A raw public pointer seems very dangerous. There should be an API around it - in particular as this is, as far as I can tell, how it is made "configurable"?!
| std::ostream &format(std::ostream &, const source_locationt &) override; | ||
| }; | ||
|
|
||
| extern default_debug_formattert default_debug_formatter; |
There was a problem hiding this comment.
This should not be a global object. Create it when creating the debug_formattert. Actually I'd move the entire declaration to the .cpp file.
There was a problem hiding this comment.
The key benefit is that it can then be a default argument. I understand overloading is an alternative, but that also creates many functions.
|
Can you clarify what sensitivity towards XML/JSON output would mean? |
|
How will we supply language-specific formatters to be used in the structured JSON/XML output (!= messages)? |
|
|
A couple of thoughts as the overall design goals and plans aren't very clear to me. Just my opinion, I would very much welcome comments/feedback/push-back:
|
|
@tautschnig, fully agree. If we want to be really radical, we could go even one step further and define any non-debugging output (I'm talking about goto-programs, traces, etc) in a (structured) way that is independent of plain/JSON/XML and then provide a generic converter from the structured output specification to the respective formats. There is a massive potential for reducing code duplication in the various show_* classes... |
Yes, I have thought along those lines as well, with the following result:
|
|
The 'radical' idea certainly has appeal; however, it's not trivial. As simplest example, JSON prefers camlCase, whereas XML uses a different convention. Plain text has further human preferences attached, which requires further special-casing. I'd be tempted to remove that redundancy by killing XML eventually; fewer and fewer people care. |
|
Although I'm far from a fan of XML, I think there were a number of commercial users for it last time I checked. |
|
Closing this PR as the discussion implies that a different solution is desirable and this PR is not going to be completed/approved in current form. Unless the discussion/opinions change significantly, recommend a new PR for the new approach rather than reopen this one to continue this approach. If you believe this has been closed erroneously please reopen. |
The interface comes with an exemplar of an application (show_symbol_table).
Note that this can replace util/format_expr.h and util/format_type.h.
This is extensible, i.e., it is possible to make the debug_formatter treat particular cases differently; it is possible to pass further data along with the formatter, say a symbol table, a wide space policy, etc., if needed. The formatter can be stateful (say for indents, parentheses, etc.).