[PATCH] D156858: Add Documentation for Execution Results Handling in Clang-REPL

[QuillPusher via Phabricator via cfe-commits]

QuillPusher accepted this revision.
QuillPusher added a comment.
This revision is now accepted and ready to land.

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


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D156858/new/

https://reviews.llvm.org/D156858

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D156858: Add Documentation for Execution Results Handling in Clang-REPL

[QuillPusher via Phabricator via cfe-commits]

QuillPusher added a comment.

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




Comment at: clang/docs/ClangRepl.rst:248-250
+.. image:: valuesynth.png
+   :align: center
+   :alt: valuesynth design

v.g.vassilev wrote:
> Can we replace that with its graphviz version?
.. 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="#ff", 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="#ff", 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;
}



Comment at: clang/docs/ClangRepl.rst:424-427
+
+.. image:: prettyprint.png
+   :align: center
+   :alt: prettyprint design

@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="#ff", 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;
}


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D156858/new/

https://reviews.llvm.org/D156858

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D156858: Add Documentation for Execution Results Handling in Clang-REPL

[QuillPusher via Phabricator via cfe-commits]

QuillPusher added inline comments.



Comment at: clang/docs/ClangRepl.rst:393-396
+
+.. image:: autoprint.png
+   :align: center
+   :alt: autoprint design

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);"];
 }



Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D156858/new/

https://reviews.llvm.org/D156858

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D156858: Add Documentation for Execution Results Handling in Clang-REPL

[QuillPusher via Phabricator via cfe-commits]

QuillPusher requested changes to this revision.
QuillPusher added a comment.
This revision now requires changes to proceed.

Added minor changes requested by Vassil




Comment at: clang/docs/ClangRepl.rst:296
+
+For example, the CPPYY code makes use of this feature to enable running 
+C++ within Python. It enables transporting values/information between C++ 

@Krishna-13-cyber 

Please change CPPYY to cppyy

I just saw they use small caps on their official website



Comment at: clang/docs/ClangRepl.rst:298
+C++ within Python. It enables transporting values/information between C++ 
+and Python.
+

@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/


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D156858/new/

https://reviews.llvm.org/D156858

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D156858: Add Documentation for Execution Results Handling in Clang-REPL

[QuillPusher via Phabricator via cfe-commits]

QuillPusher accepted this revision.
QuillPusher added a comment.
This revision is now accepted and ready to land.

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


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D156858/new/

https://reviews.llvm.org/D156858

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D156858: Add Documentation for Execution Results Handling in Clang-REPL

[QuillPusher via Phabricator via cfe-commits]

QuillPusher requested changes to this revision.
QuillPusher added a comment.
This revision now requires changes to proceed.

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




Comment at: clang/docs/ClangRepl.rst:220-230
+
+: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:
+
+- Capture Execution Results: This feature helps capture the execution results
+  of a program and bring them back to the compiled program.

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



Comment at: clang/docs/ExecutionResultsHandling.rst:119-127
+
+.. code-block:: console
+
+for (Decl *D : DGR)
+  if (auto *TSD = llvm::dyn_cast(D);
+  TSD && TSD->isSemiMissing())
+TSD->setStmt(Interp.SynthesizeExpr(cast(TSD->getStmt(;

Add note before code snippet

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



Comment at: clang/docs/ExecutionResultsHandling.rst:237-256
+.. code-block:: console
+
+void Value::print(llvm::raw_ostream ) const {
+assert(OpaqueType != nullptr && "Can't print default Value");
+
+if (getType()->isVoidType() || !isValid())
+return;

please remove this code block, it is not needed



Comment at: clang/docs/index.rst:96
ClangRepl
+   ExecutionResultsHandling
 

v.g.vassilev wrote:
> We should probably move that under `ClangRepl`.
> 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)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D156858/new/

https://reviews.llvm.org/D156858

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D156858: Add Documentation for Execution Results Handling in Clang-REPL

[QuillPusher via Phabricator via cfe-commits]

QuillPusher accepted this revision.
QuillPusher added a comment.
This revision is now accepted and ready to land.

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


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D156858/new/

https://reviews.llvm.org/D156858

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D156858: Add Documentation for Execution Results Handling in Clang-REPL

[QuillPusher via Phabricator via cfe-commits]

QuillPusher requested changes to this revision.
QuillPusher added a comment.
This revision now requires changes to proceed.

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

1. Capture Execution Results
  - How the Feature works
  - more details
  - Implementation Details

2. 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.

F28589747: ExecutionResultsHandling.rst 




Comment at: clang/docs/ClangRepl.rst:217-232
 
+Execution Results Handling in Clang-Repl
+
+
+:doc:`ExecutionResultsHandling` features discussed below help extend the 
Clang-REPL 
+functionality by creating an interface between the execution results of a 
+program and the compiled program.

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.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D156858/new/

https://reviews.llvm.org/D156858

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D156858: Add Documentation for Execution Results Handling in Clang-REPL

[QuillPusher via Phabricator via cfe-commits]

QuillPusher accepted this revision.
QuillPusher added a comment.
This revision is now accepted and ready to land.

Thanks for the updates, look OK to me


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D156858/new/

https://reviews.llvm.org/D156858

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D156858: Add Documentation for Execution Results Handling in Clang-REPL

[QuillPusher via Phabricator via cfe-commits]

QuillPusher requested changes to this revision.
QuillPusher added a comment.
This revision now requires changes to proceed.

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




Comment at: clang/docs/ExecutionResultsHandling.rst:50-52
+Inspired by a similar implementation in `Cling 
`_,
+this feature added to upstream Clang repo has essentially extended the syntax 
of C++,
+so that it can be more helpful for people that are writing code for data 
science applications.

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



Comment at: clang/docs/ExecutionResultsHandling.rst:55
+This is useful, for example, when you want to experiment with a set of values 
+against a set of functions, and you'd like to know the results right-away. 
+This is similar to how Python works (hence its popularity in data science 

Please remove the hyphen: 

right-away -> right away

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



Comment at: clang/docs/ExecutionResultsHandling.rst:257-259
+`Above is an example of interoperability between the compiled code and the 
+interpreted code. Interoperability between languages (e.g., C++ and Python) 
+works similarly.`

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



Comment at: clang/docs/ExecutionResultsHandling.rst:268
+
+How it works?
+---

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.



Comment at: clang/docs/ExecutionResultsHandling.rst:283
+
+.. code-block:: console
+

@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.



Comment at: clang/docs/ExecutionResultsHandling.rst:329-356
+
+Complex Data Types:
+---
+
+This feature can print out primitive types (int, char, bool, etc.) easily. 
+For more complex types (e.g., `std::vector`), it falls back to a runtime 
+function call using the following helper function.

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



Comment at: clang/docs/ExecutionResultsHandling.rst:357-371
+
+Users can create their own types:
+-
+
+All overloads live in a header, which are included at runtime. So **print a 
+std::vector** is equivalent to `PrintValueRuntime();`.
+

@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.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D156858/new/

https://reviews.llvm.org/D156858

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits