Stack alloc: option to print in JSON format #656
Conversation
eponier
force-pushed
the
stack-alloc-json
branch
from
07fa397 to
5c77bd6
Compare
There was a problem hiding this comment.
I find it weird to output this JSON data to the error output, mixed with other debug information not meant for machine consumption.
Wouldn’t it be more convenient for the user to specify a file name?
Also, there is no test of this functionality (therefore it will break).
|
Ok, I will print to a dedicated file. What do you have in mind for the test? Testing this is valid JSON, that it prints the same information that the option we already have, something else? |
|
I don’t have anything in mind beyond the following invariant of software engineering: untested features eventually break. |
eponier
force-pushed
the
stack-alloc-json
branch
2 times, most recently
from
494ba19 to
80c0dcc
Compare
The command line option is -json-stack-alloc filename
eponier
force-pushed
the
stack-alloc-json
branch
from
80c0dcc to
b720328
Compare
We check that -print-stack-alloc and -json-stack-alloc stay in sync. The tests are not run by default by "make check".
|
I implemented what you suggested. And I added a test that checks that |
|
Are we happy with this one ? |
|
I now regret to have suggested to test that feature… |
|
If you find the tests too ugly, I can just drop them. Another option, suggested by Benjamin last week, is to test the output like we do for the SCT checker. |
|
I'd like to close this one, I think the consensus is rejection. @bgregoir, you mentioned that you wanted to discuss with Tiago about this PR, did you do it? |
|
@bgregoir ping |
|
I forgot, I should do it. |
No description provided.