This is an archive of the discontinued LLVM Phabricator instance.

Add Documentation for Execution Results Handling in Clang-REPL
ClosedPublic

Authored by Krishna-13-cyber on Aug 2 2023, 12:31 AM.

Details

Summary

This patch adds documentation for execution results handling in Clang-REPL with the below features:

  • Automatic Printf feature
  • Value Synthesis feature
  • Pretty Printing feature

I am issuing this patch on behalf of @QuillPusher

Diff Detail

Event Timeline

Herald added a project: Restricted Project. · View Herald TranscriptAug 2 2023, 12:31 AM
Herald added a subscriber: arphaman. · View Herald Transcript
Krishna-13-cyber requested review of this revision.Aug 2 2023, 12:31 AM
Herald added a project: Restricted Project. · View Herald TranscriptAug 2 2023, 12:31 AM
Herald added a subscriber: cfe-commits. · View Herald Transcript
Krishna-13-cyber edited the summary of this revision. (Show Details)Aug 2 2023, 12:34 AM
QuillPusher requested changes to this revision.Aug 3 2023, 9:56 AM

added some comments for minor changes. Two sections to be removed since they are not yet merged to upstream code:

  • Complex Data Types
  • Users can create their own types
clang/docs/ExecutionResultsHandling.rst
51–53

@Krishna-13-cyber This paragraph is exceeding 80 columns. Please reformat to limit to 80 columns

56

Please remove the hyphen:

right-away -> right away

(must have carried through from previous tool's preview)

258–260

Formatting for this note seems different, it is enclosed in single quotes. Did you mean Note: or " " ?

269

Please remove the question mark, this is an expression, not a question (common mistake)

How it works? -> How it works

Note that another heading has a question mark (Where is the captured result stored?). That question mark is OK, since that is actually mimicking a user's question. So leave that one as it is.

284

@Krishna-13-cyber please add a note/comment above the code block:

Note: Following is a sample code snippet. Actual code may vary over time.

330–357

@Krishna-13-cyber please remove the "Complex Data Types" section, since this is not merged into the upstream LLVM code yet.
@junaire can confirm.

358–372

@Krishna-13-cyber please remove the "Users can create their own types" section, since this is not merged into the upstream LLVM code yet.
@junaire can confirm.

This revision now requires changes to proceed.Aug 3 2023, 9:56 AM
v.g.vassilev added inline comments.Aug 3 2023, 12:13 PM
clang/docs/index.rst
96

We should probably move that under ClangRepl.

  • Addressed the comments
QuillPusher accepted this revision.EditedAug 5 2023, 7:28 AM

Thanks @Krishna-13-cyber for the updates, look OK to me

Thanks @junaire for sharing feature knowledge to help create this document

This revision is now accepted and ready to land.Aug 5 2023, 7:28 AM

I want to clarify: We offer two features: 1. capture the execution results and bring it back to the compiled program. 2. dump the captured value (value printing/automatic printf) and all the parsing, ast transform and balabala, these are all implementation details. The doc looks fine but may need a bit of restructuring. I'd suggest you split them into:

  1. what do we offer? what are the new features? why are they important? The two I mentioned above
  2. how these are implemented? what does the underhood look like? annotation token, code synthesis, etc...
  3. further information, RFC and etc...
QuillPusher requested changes to this revision.Aug 5 2023, 11:55 AM

Thanks @junaire for the clarification, following is the updated document structure:

  1. Capture Execution Results
    • How the Feature works
    • more details
    • Implementation Details
  1. Dump Captured Execution Results¶
    • How- the Feature works
    • more details
    • Implementation Details

@Krishna-13-cyber Please use the attached file named ExecutionResultsHandling.rst as the latest doc, so Jun can review the updated document.

clang/docs/ClangRepl.rst
217–233

Execution Results Handling in Clang-Repl

:doc:ExecutionResultsHandling helps extend the Clang-REPL functionality by
creating an interface between the execution results of a program and the compiled
program. Following are its main components:

1 - Capture Execution Results: This feature helps capture the execution results
of a program and bring them back to the compiled program.

2 - Dump Captured Execution Results: This feature helps create a temporary dump
for Value Printing/Automatic Printf, that is, to display the value and type of
the captured data.

This revision now requires changes to proceed.Aug 5 2023, 11:55 AM
  • Address the comments
  • Update section of Execution Results Handling in Clang-REPL
QuillPusher accepted this revision.Aug 6 2023, 8:16 AM

Thanks @Krishna-13-cyber for the prompt changes.

@v.g.vassilev If no further changes are required by @junaire , then this should be ready to merge

This revision is now accepted and ready to land.Aug 6 2023, 8:16 AM
v.g.vassilev added inline comments.Aug 9 2023, 1:27 AM
clang/docs/ExecutionResultsHandling.rst
81

cppyy seems undefined here.

QuillPusher requested changes to this revision.Aug 9 2023, 1:53 AM

added minor changes based on Vassil's review during docs meeting

clang/docs/ClangRepl.rst
220–230

As per Vassil's comment below, please move all the Execution Results handling text in the main Clang-Repl doc instead of a separate ExecutionResultsHandling document

So this paragraph can be removed and all the content from the ExecutionResultsHandling.rst can be included here in this ClangRepl.rst document

clang/docs/ExecutionResultsHandling.rst
120–128

Add note before code snippet

Note: Following is a sample code snippet. Actual code may vary over time.

238–257

please remove this code block, it is not needed

clang/docs/index.rst
96

We should probably move that under ClangRepl.

@Krishna-13-cyber I have commented above, where the excerpt paragraph in CalngRepl.rst can be removed and all content from ExecutionResultsHandling.rst can be included (and remove file ExecutionResultsHandling.rst after that)

This revision now requires changes to proceed.Aug 9 2023, 1:53 AM
  • Address the comments
QuillPusher accepted this revision.Aug 11 2023, 3:35 AM

accepting current revision, one minor change is still required, adding that separately

This revision is now accepted and ready to land.Aug 11 2023, 3:35 AM
QuillPusher requested changes to this revision.Aug 11 2023, 3:39 AM

Added minor changes requested by Vassil

clang/docs/ClangRepl.rst
296

@Krishna-13-cyber

Please change CPPYY to cppyy

I just saw they use small caps on their official website

298

@Krishna-13-cyber

Please add the following note under this paragraph, as requested by Vassil in previous comment:

Note: cppyy_ is an automatic, run-time, Python-to-C++ bindings
generator, for calling C++ from Python and Python from C++. It uses LLVM along
with a C++ interpreter (e.g., Cling) to enable features like run-time
instantiation of C++ templates, cross-inheritance, callbacks, auto-casting,
transparent use of smart pointers, etc.

.. _cppyy: https://github.com/wlav/cppyy/

This revision now requires changes to proceed.Aug 11 2023, 3:39 AM
QuillPusher added inline comments.Aug 13 2023, 6:46 AM
clang/docs/ClangRepl.rst
393–396

Please replace the image reference with the following Graphviz code (do not delete the image .png for now, we need to commit and see if graphviz works out of the box).

.. graphviz::

:name: automaticprintf
:caption: Automatic PrintF
:alt: Shows how Automatic PrintF can be used
:align: center

 digraph "AutomaticPrintF" {
     size="6,4";
     rankdir="LR";
     graph [fontname="Verdana", fontsize="12"];
     node [fontname="Verdana", fontsize="12"];
     edge [fontname="Sans", fontsize="9"];

     manual [label=" Manual PrintF ", shape="box"];
     int1 [label=" int ( &) 42 ", shape="box"]
     auto [label=" Automatic PrintF ", shape="box"];
     int2 [label=" int ( &) 42 ", shape="box"]

     auto -> int2 [label="int x = 42; \n x"];
     manual -> int1 [label="int x = 42; \n printf("(int &) %d \\n", x);"];
 }
v.g.vassilev added inline comments.Aug 13 2023, 7:34 AM
clang/docs/ClangRepl.rst
221

We should probably spell consistently Clang-Repl.

248

Can we replace that with its graphviz version?

264
265
266–267
269–274
276–278
284
296–298
352
404
419
461

@Krishna-13-cyber added Graphviz code snippets to replace all .png images

clang/docs/ClangRepl.rst
248–250

.. graphviz::

:name: valuesynthesis
:caption: Value Synthesis
:alt: Shows how an object of type 'Value' is synthesized
:align: center

 digraph "valuesynthesis" {
     rankdir="LR";
     graph [fontname="Verdana", fontsize="12"];
     node [fontname="Verdana", fontsize="12"];
     edge [fontname="Sans", fontsize="9"];

     start [label=" Create an Object \n 'Last Value' \n of type 'Value' ", shape="note", fontcolor=white, fillcolor="#3333ff", style=filled];
     assign [label=" Assign the result \n to the 'LastValue' \n (based on respective \n Memory Allocation \n scenario) ", shape="box"]
     print [label=" Pretty Print \n the Value Object ", shape="Msquare", fillcolor="yellow", style=filled];
     start -> assign;
     assign -> print;

       subgraph SynthesizeExpression {
         synth [label=" SynthesizeExpr() ", shape="note", fontcolor=white, fillcolor="#3333ff", style=filled];
         mem [label=" New Memory \n Allocation? ", shape="diamond"];
         withaloc [label=" SetValueWithAlloc() ", shape="box"];
         noaloc [label=" SetValueNoAlloc() ", shape="box"];
         right [label=" 1. RValue Structure \n (a temporary value)", shape="box"];
         left2 [label=" 2. LValue Structure \n (a variable with \n an address)", shape="box"];
         left3 [label=" 3. Built-In Type \n (int, float, etc.)", shape="box"];
         output [label=" move to 'Assign' step ", shape="box"];

         synth -> mem;
         mem -> withaloc [label="Yes"];
         mem -> noaloc [label="No"];
         withaloc -> right;
         noaloc -> left2;
         noaloc -> left3;
         right -> output;
         left2 -> output;
         left3 -> output;
 }
     output -> assign;

}

424–427

@Krishna-13-cyber please replace the image reference with the following code:

.. graphviz::

:name: parsing
:caption: Parsing Mechanism
:alt: Shows the Parsing Mechanism for Pretty Printing
:align: center

 digraph "prettyprint" {
     rankdir="LR";
     graph [fontname="Verdana", fontsize="12"];
     node [fontname="Verdana", fontsize="12"];
     edge [fontname="Verdana", fontsize="9"];

     parse [label=" ParseAndExecute() \n in Clang ", shape="box"];
     capture [label=" Capture 'Value' parameter \n for processing? ", shape="diamond"];
     use [label="  Use for processing  ", shape="box"];
     dump [label="  Validate and push  \n to dump()", shape="box"];
     callp [label="  call print() function ", shape="box"];
     type [label="  Print the Type \n ReplPrintTypeImpl()", shape="box"];
     data [label="  Print the Data \n ReplPrintDataImpl() ", shape="box"];
     output [label="  Output Pretty Print \n to the user  ", shape="box", fontcolor=white, fillcolor="#3333ff", style=filled];

     parse -> capture [label="Optional 'Value' Parameter"];
     capture -> use [label="Yes"];
     use -> End;
     capture -> dump [label="No"];
     dump -> callp;
     callp -> type;
     callp -> data;
     type -> output;
     data -> output;

}

  • Address the comments
QuillPusher accepted this revision.Aug 13 2023, 1:26 PM

Thanks @Krishna-13-cyber for the prompt efforts
@v.g.vassilev it is ready to commit as far as I can see

This revision is now accepted and ready to land.Aug 13 2023, 1:26 PM
v.g.vassilev accepted this revision.Aug 23 2023, 7:09 AM

The pre-merge check seems to fail at clang-format for some reason. This patch does not change anything that should be formatted by clang-format.

We will try to add the images as editable graphviz content. We need to see if the infrastructure allows for this as suggested here: https://discourse.llvm.org/t/any-tool-for-creating-editable-diagrams-in-llvm-clang-repl-documentation-similar-to-mermaid-ingithub/72729/2

This revision was landed with ongoing or failed builds.Aug 23 2023, 7:12 AM
This revision was automatically updated to reflect the committed changes.