This is an archive of the discontinued LLVM Phabricator instance.

[analyzer] exploded-graph-rewriter: Improve user-friendliness.
ClosedPublic

Authored by NoQ on Jul 24 2019, 4:03 PM.

Details

Summary

Change the default behavior: the tool no longer dumps the rewritten .dot file to stdout, but instead it automatically converts it into an .html file (which essentially wraps an .svg file) and immediately opens it with the default web browser.

This means that the tool should be fairly easy to use:

$ exploded-graph-rewriter.py /tmp/ExprEngine.dot

The benefits of wrapping the .svg file into an .html file are:

  • It'll open in a web browser, which is the intended behavior. An .svg file would be open with an image viewer/editor instead.
  • It avoids the white background around the otherwise dark svg area in dark mode.

I also added more help text so that it was easier to figure out how to use the tool (suggestions are welcome!).

The new feature bypasses tests because i don't expect everybody (esp. buildbots) to have python-graphviz installed. The LIT substitution is specifically tweaked to enforce the old mode.

TODO: Add a -o flag to specify the output file.

Diff Detail

Repository
rL LLVM

Event Timeline

NoQ created this revision.Jul 24 2019, 4:03 PM
Herald added a project: Restricted Project. · View Herald TranscriptJul 24 2019, 4:03 PM
NoQ updated this revision to Diff 211639.Jul 24 2019, 4:50 PM

Make it a soft failure when graphviz is not installed. Display a friendly warning.

NoQ updated this revision to Diff 211640.Jul 24 2019, 4:51 PM

Polish wording a bit.

There is no need to wrap SVG in HTML if you want to display it in a web-browser, you can just open the web-browser explicitly, they already support opening SVG, use something like

webbrowser.open_new(url)

i.e. https://docs.python.org/2/library/webbrowser.html

NoQ added a comment.Jul 25 2019, 8:00 AM

There is no need to wrap SVG in HTML if you want to display it in a web-browser, you can just open the web-browser explicitly, they already support opening SVG, use something like

webbrowser.open_new(url)

i.e. https://docs.python.org/2/library/webbrowser.html

Mmm, interesting, maybe, but i don't see any obvious reasons to keep it in svg either. With html you can also save it and open up later without messing up your MIME associations, or even send it to others without having them mess with their MIME associations. With html you can also avoid the dark background glitch (the svg has limited size and would otherwise be displayed as a dark rectangle over a white background). Also it opens up opportunities for scripting, if we ever go that far (even if this gets more and more hacky). I guess i could do your suggestion under an --svg flag, but i'll prefer to have it off by default, because html is slightly better for most purposes. I guess it's worth it when the recipient doesn't trust html attachments (eg., for sending to the mailing lists).

In D65250#1601187, @NoQ wrote:

There is no need to wrap SVG in HTML if you want to display it in a web-browser

I guess it's worth it when the recipient doesn't trust html attachments (eg., for sending to the mailing lists).

You could put HTML inside the SVG (https://developer.mozilla.org/en-US/docs/Web/SVG/Element/foreignObject). That is a cool feature just I have not managed to use it yet, but I would use it for better performance in loading the SVG, and that definitely needs an HTML wrapper.

Charusso accepted this revision.Jul 26 2019, 1:03 PM

I like the HTML output, thanks!

clang/utils/analyzer/exploded-graph-rewriter.py
846 ↗(On Diff #211640)

Why we need multiple prints to print one message? According to the Stack Overflow:

print("""
Line1
Line2
""")

Link: https://stackoverflow.com/questions/34980251/how-to-print-multiple-lines-of-text-with-python

953 ↗(On Diff #211640)

This flag is a little-bit too long and does not really emphasize what it suppose to do. What about just --dump or --stdout?

This revision is now accepted and ready to land.Jul 26 2019, 1:03 PM
NoQ marked 2 inline comments as done.Aug 13 2019, 3:58 PM
NoQ added inline comments.
clang/utils/analyzer/exploded-graph-rewriter.py
846 ↗(On Diff #211640)

I dislike what it does to my formatting. We're in 4 nested scopes and it suddenly requires me to make an un-indented block.

953 ↗(On Diff #211640)

Renamed to --dump-dot-only.

This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptAug 13 2019, 4:05 PM