This is an archive of the discontinued LLVM Phabricator instance.

[pseudo] Add `clang-pseudo -html-forest=<output.html>`, an HTML forest browser
ClosedPublic

Authored by sammccall on Jul 18 2022, 6:02 AM.

Details

Summary

It generates a standalone HTML file with all needed JS/CSS embedded.
This allows navigating the tree both with a tree widget and in the code,
inspecting nodes, and selecting ambiguous alternatives.

Demo: https://htmlpreview.github.io/?https://gist.githubusercontent.com/sam-mccall/03882f7499d293196594e8a50599a503/raw/ASTSignals.cpp.html

Diff Detail

Event Timeline

sammccall created this revision.Jul 18 2022, 6:02 AM
Herald added a project: Restricted Project. · View Herald TranscriptJul 18 2022, 6:02 AM
Herald added a subscriber: mgorny. · View Herald Transcript
sammccall requested review of this revision.Jul 18 2022, 6:02 AM
Herald added a project: Restricted Project. · View Herald TranscriptJul 18 2022, 6:02 AM
hokein accepted this revision.Jul 19 2022, 6:03 AM

Super cool! Can't wait to use it.

The code looks good, just nits (feel free to ignore)

clang-tools-extra/pseudo/tool/CMakeLists.txt
29

This likely introduces some pain. (An alternative is to inline the content of .js, .css, html in the cpp directly, but it is awkward).

Would be nice to prepare an internal BUILD file change when landing (if you don't have time, I'm happy to help with it).

clang-tools-extra/pseudo/tool/HTMLForest.cpp
65

nit: it is clearer to use use llvm::formatv

llvm::formatv(R"html(
...
)html", js, css, forest_json, code, html);.
116

I'd suggest renaming to Array and adding comment for the Index (it is the index in the Queue), Queue leaves an impression to me that we will do some pop_back.

171

Having derived stream is fine to me at the moment, particularly useful for debugging.

But as a regular user, I would prefer to see the original token stream (the derived stream is confusing, as a regular user doesn't know how pseudoparser work inside).

clang-tools-extra/pseudo/tool/HTMLForest.css
51

nit: just an idea we could use different background color for different forest node kind.

64

nit: can we make the tree and code view resizable? so that I can adjust the width of tree/code view.

83

nit: we could also add some style (possibly red indicating the error) for opaque node.

clang-tools-extra/pseudo/tool/HTMLForest.html
2

I think it might be better to move these .html, .css, .js files to a separate directory tool/resources rather than tool.

14

nit: maybe name it i_context_stack, I had no idea what does the context mean until I read the actual implementation.

This revision is now accepted and ready to land.Jul 19 2022, 6:03 AM
sammccall marked 7 inline comments as done.Jul 19 2022, 11:03 AM

Thanks for the review!

clang-tools-extra/pseudo/tool/CMakeLists.txt
29

This likely introduces some pain. (An alternative is to inline the content of .js, .css, html in the cpp directly, but it is awkward).

Yeah, I think this is unreasonably difficult to edit, unfortunately - no editor understands JS/CSS/html inside string literals. (We can probably get away with the HTML, but the JS is a significant amount of code)

(In fact had a second mode where the resources were read from disk without a recompile, which was faster while working on it, but I don't think it's necessary - inotifywait loop is enough)

Would be nice to prepare an internal BUILD file change when landing (if you don't have time, I'm happy to help with it).

Good point, I'll prepare that.

clang-tools-extra/pseudo/tool/HTMLForest.cpp
65

I guess, but:

  • a long formatv string means it's hard to spot which {n} corresponds to which arg
  • we still have to break up the formatv for writeForestJSON() etc, which doesn't seem like a natural place to break.

Added a small tag(name, block) function instead which I think makes the structure easier to follow (basically the clang-formatted code shows the HTML structure)

116

I agree about Queue.
Array is confusing as this isn't actually either a C++ array or a json::Array.
Chose Sequence to emphasize that it's the order we're establishing.

171

Yeah, I think we'll want this later changed to FIXME.
We will have to assume some invariants to get it (i.e. that the derived tokens are basically a same-order subset of the original ones).
Not sure if "regular users" will ever see this tool :-) I'm not sure it extends well to forests, we may want something else.

clang-tools-extra/pseudo/tool/HTMLForest.css
51

Done, and made the colors of the nodes in the tree match.
I like it, hope it's not too rainbow-y

64

resize: horizontal seems to do the trick

clang-tools-extra/pseudo/tool/HTMLForest.html
2

the tool/ directory is ~empty or I would... happy to add some more structure if it gets at all crowded

14

Renamed to i_ancestors which seems explicit without being long

sammccall marked 6 inline comments as done.

review changes

sammccall updated this revision to Diff 445920.Jul 19 2022, 1:08 PM

Make bundle_resources less sensitive to its working directory

This revision was landed with ongoing or failed builds.Jul 19 2022, 1:32 PM
This revision was automatically updated to reflect the committed changes.