Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add cypher AST #250

Open
wants to merge 9 commits into
base: master
Choose a base branch
from
Open

add cypher AST #250

wants to merge 9 commits into from

Conversation

thamindumk
Copy link

No description provided.

@@ -1,6 +1,6 @@
cmake_minimum_required(VERSION 3.10)
project(JasmineGraph)
set(CMAKE_CXX_STANDARD 11)
set(CMAKE_CXX_STANDARD 17)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We use C++ 11 standard currently in JasmineGraph. Is there special reason to change the C++ standard to 17?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sir, I have to use "any_cast" and "std::any" in the cypher ast, because I have to override the methods of generated parse by antlr4 and those methods use that "std::any". it is a utility which is provided by c++ 17 standards. that's why I used C++ 17 standards.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not a simple change. We have to evaluate the effect of this for the entire project. We should not do standard upgrade unless we assess the essential requirement of doing so, what would be the complications, and the development workload attached to that. So what you are saying is Cypher is dependent on C++ 17?

RUN apt-get update && apt-get install --no-install-recommends -y default-jre
RUN curl -O https://s3.amazonaws.com/artifacts.opencypher.org/M23/Cypher.g4
RUN curl -O https://www.antlr.org/download/antlr-4.13.2-complete.jar
RUN java -jar antlr-4.13.2-complete.jar -Dlanguage=Cpp -visitor Cypher.g4
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we move this entire process of downloading, installing, running, and removing the JVM to pre-requisites image? I do not see a point of doing that here.

@@ -45,6 +45,12 @@ limitations under the License.
#include "JasmineGraphFrontEndProtocol.h"
#include "core/CoreConstants.h"
#include "core/scheduler/JobScheduler.h"
#include "antlr4-runtime.h"
#include "/home/ubuntu/software/jasminegraph/code_generated/antlr/CypherLexer.h"
#include "/home/ubuntu/software/jasminegraph/code_generated/antlr/CypherParser.h"
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rather than specifying absolute path here, we should use relative path instead.

@@ -0,0 +1,1668 @@
#include "ASTBuilder.h"
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lets add license header to this file.

@@ -0,0 +1,219 @@
#include <any>
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lets add license header to this file.

@@ -0,0 +1,6 @@
#include "ASTInternalNode.h"
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lets add license header to this file.

@@ -0,0 +1,19 @@
#include "ASTNode.h"
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lets add license header to this file.

@@ -0,0 +1 @@
#include "ASTLeafNoValue.h"
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lets add license header to this file.

@@ -0,0 +1,14 @@
#include "ASTNode.h"
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lets add license header to this file.

@@ -0,0 +1 @@
#include "ASTLeafValue.h"
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lets add license header to this file.

@@ -0,0 +1,16 @@
#include "ASTNode.h"
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lets add license header to this file.

@@ -0,0 +1,25 @@
#include "ASTNode.h"
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lets add license header to this file.


using namespace std;

string ASTNode::print(int depth, string prefix, bool isLast) const
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

May be good to keep this print function as a support for debugging. However, we should use a logger instead of outputting this to console.

@@ -0,0 +1,24 @@
#include <vector>
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lets add license header to this file.

Copy link

sonarcloud bot commented Aug 28, 2024

merge from jasmineGraph master
Copy link

sonarcloud bot commented Nov 15, 2024

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants