Close #8 #33

elevankoff · 2020-12-04T15:11:53Z

Added UAST builder and related functions.

…read_graphviz_new>

aravij

As get there a lot of files from @AlexanderMishutkin commits, but you can both take it into account.
According to the design of a builder, as we were talking before, we better separate functionalities of storing paths to virtual machines, calling them and parsing dot file to our structure. You better do it within a single class.

aravij · 2020-12-30T12:34:24Z

source/CMakeLists.txt

@@ -1,10 +1,12 @@
 project(uast_core)

-set(SOURCE_LIB uast.h uast_stub.cpp)
+set(CMAKE_CXX_FLAGS -lboost_graph)


You already link boost_graph with target_link_library. Do you need to add this flag?

aravij · 2020-12-30T12:34:59Z

source/CMakeLists.txt


 add_library(uast_core STATIC ${SOURCE_LIB})

-target_link_libraries(uast_core LINK_PUBLIC cpp_parser)
+target_link_libraries(uast_core cpp_parser)


Why have you removed LINK_PUBLIC?

Somehow there was double including of this file...

aravij · 2020-12-30T12:36:47Z

source/CMakeLists.txt


 add_library(uast_core STATIC ${SOURCE_LIB})

-target_link_libraries(uast_core LINK_PUBLIC cpp_parser)
+target_link_libraries(uast_core cpp_parser)
+target_link_libraries(uast_core boost_graph boost_regex)


I think you can link all needed libraries in one target_link_libraries call.

aravij · 2020-12-30T12:47:12Z

source/node.h

+#include <vector>
+
+namespace uast {
+    class Node {


We better dont indent code inside namespace block, because usually all files declare some namespace, but extra indentation increase code readability.

We can change the style, but it would be har, cause all the IDLE's use such indentation by default, but they can be customized, of course. Should we do it anyway?

aravij · 2020-12-30T12:48:16Z

source/node.h

+
+namespace uast {
+    class Node {
+        std::unordered_multimap<std::string, std::shared_ptr<Node>> nodes_;


Why do you use shared pointer? Who else sharing ownership of a node. except its parent?

One main link is stored in parent's Node. But pointers are also used in iterators and can be customly done by user to reduct tree. This logic could be realized via weak_pointers, but they are too complicated.

aravij · 2021-01-20T12:27:23Z

source/uast_stub.cpp

+
+    return *rootP->first;
+}
+
 std::string Stub() {


We can delete stub for now.

aravij · 2021-01-20T12:49:54Z

source/uast_stub.cpp

+                      const std::string& pythonVmPath) {
+
+    const std::string runCppParser = "cpp_parser/cpp_parser.exe",
+                      runJavaParser = javaVmPath + "java -jar java_parser/java_parser.jar",


As I see here, you expect user to provide a path to directory. Here I see few problems. First, user may not provide a separator /, making usual concatenation error prone. Second, the name of the virtual machine may not be java, but java11 or something like this. I think we have to make user specify full path to virtual machine, to avoid such errors.

aravij · 2021-01-20T13:04:32Z

source/uast_stub.cpp

+
+    system(runParserCommand.c_str());
+
+    const std::string gvFilePath = "graph.dot";  // Where is parser result?


You better specify output file name on your own. You might create a string template and substitute into it output file name, making it each time different. Check for some functions, how to generate unique file name.

aravij · 2021-01-20T13:07:26Z

source/uast_stub.cpp

+    boost::graph_traits<graph_t>::out_edge_iterator eIt, edge_end;
+
+    // Parse boost::graph to uast
+    for (boost::tie(vIt, end) = boost::vertices(graph); vIt != end; vIt++) {


What is first statement in for cycle?
boost::tie(vIt, end) = boost::vertices(graph) does it declare any cycle index?

aravij · 2021-01-20T13:36:12Z

source/uast_stub.cpp

+        vertex_t v = *vIt;
+        if (nodes.find(v) == nodes.end()) {
+            nodes[v] = std::make_shared<Node>(typeMap[v]);
+            degrees[nodes[v]] = 0;
+        }
+
+        for (boost::tie(eIt, edge_end) = boost::out_edges(*vIt, graph); eIt != edge_end; eIt++) {
+            vertex_t to = boost::target(*eIt, graph);
+            if (nodes.find(to) == nodes.end()) {
+                nodes[to] = std::make_shared<Node>(typeMap[to]);
+            }
+            nodes[v]->AddChild(nameMap[*eIt], nodes[to]);
+            degrees[nodes[to]]++;
+        }
+    }
+
+    auto rootP =
+        std::find_if(degrees.begin(), degrees.end(), [](const auto& p) { return p.second == 0; });


It is really hard for me to understand this code, it might need some explanation, like comments, ecomposition to several functions with proper names, etc.

AlexanderMishutkin · 2021-01-21T15:37:14Z

@elevankoff, please, can you move the biggest part of your code from uast.h, wich is used just for including other headers and defining namespaces, to some specific files 🙏

elevankoff · 2022-12-24T13:38:59Z

@elevankoff

А, собственно, почему вдруг?))

Да просто увидел открытый ПР неактуальный -- решил закопать
Зачистка перед Новым годом)

elevankoff · 2022-12-24T13:46:58Z

И то верно. Потомуто

а) никогда не вредно убраться

б) может когда-нибудь во время технологической сингулярности на руинах человеческой цивилизации какая-нибудь GPTшка найдет этот проект на гите, оценит идею и завершит его. Она будет благодарна тому, что репа в борядке

Ну так)) Все по Джордану Питерсану, если ты понимаешь о чем я ахахах

С наступающим!)

AlexanderMishutkin and others added 4 commits December 4, 2020 09:54

[7] UAST node basic fuctional. Basic unit tests

f4cadb2

[7] Merge branch 'issue-19' into issue-7

e6defa9

[8] Add uast-build and related functions

5676107

[??] Autoformat

3baca57

elevankoff self-assigned this Dec 4, 2020

elevankoff linked an issue Dec 4, 2020 that may be closed by this pull request

UAST builder #8

Open

elevankoff closed this Dec 4, 2020

elevankoff and others added 6 commits December 4, 2020 20:36

[8] Add boost/1.74.0 to conanfile | deleted including <filesystem>

713c2e8

[??] Autoformat

3543bf6

Try to include <boost/graph/detail/read_graphviz_new>

594915a

Try to add -lboost_graph flag to CMakeLists | deleted including <.../…

e16c103

…read_graphviz_new>

[8] Add linking boost_graph and boost_regex

ed6eccd

[8] Add --linelength=120 for cpplint and code refactoring

6986e3e

elevankoff reopened this Dec 5, 2020

aravij requested changes Jan 21, 2021

View reviewed changes

AlexanderMishutkin changed the title ~~[8] UAST builder~~ Close #8 Jan 26, 2021

elevankoff closed this Dec 24, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Close #8 #33

Close #8 #33

elevankoff commented Dec 4, 2020

aravij left a comment

aravij Dec 30, 2020

aravij Dec 30, 2020

AlexanderMishutkin Jan 26, 2021

aravij Dec 30, 2020

aravij Dec 30, 2020

AlexanderMishutkin Jan 26, 2021

aravij Dec 30, 2020

AlexanderMishutkin Jan 26, 2021

aravij Jan 20, 2021

aravij Jan 20, 2021

aravij Jan 20, 2021

aravij Jan 20, 2021

aravij Jan 20, 2021

AlexanderMishutkin commented Jan 21, 2021

elevankoff commented Dec 24, 2022

elevankoff commented Dec 24, 2022


		system(runParserCommand.c_str());

		const std::string gvFilePath = "graph.dot"; // Where is parser result?

Close #8 #33

Close #8 #33

Conversation

elevankoff commented Dec 4, 2020

aravij left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AlexanderMishutkin commented Jan 21, 2021

elevankoff commented Dec 24, 2022

elevankoff commented Dec 24, 2022