From 2c2c8ae80e6f2ce20da303f6982eb79e076072f3 Mon Sep 17 00:00:00 2001 From: "Maxim [maxirmx] Samsonov" Date: Wed, 4 May 2022 11:58:34 +0300 Subject: [PATCH] Antlr upgrade to version 4.9.2, some inline comments --- .github/workflows/rake.yml | 3 +-- ext/express-parser/antlr4-upstream | 2 +- ext/express-parser/antlr4.patch | 21 --------------------- ext/express-parser/express_parser.cpp | 20 +++++--------------- ext/express-parser/extconf.rb | 4 +--- lib/expressir/express/parser.rb | 5 ++++- 6 files changed, 12 insertions(+), 43 deletions(-) delete mode 100644 ext/express-parser/antlr4.patch diff --git a/.github/workflows/rake.yml b/.github/workflows/rake.yml index 853353ba..ccfc1df1 100644 --- a/.github/workflows/rake.yml +++ b/.github/workflows/rake.yml @@ -8,8 +8,7 @@ on: jobs: rake: name: test on ruby-${{ matrix.ruby }} ${{ matrix.os }} - runs-on: ${{ matrix.os }} - + runs-on: ${{ matrix.os }} continue-on-error: ${{ matrix.experimental }} strategy: fail-fast: false diff --git a/ext/express-parser/antlr4-upstream b/ext/express-parser/antlr4-upstream index 7a3f40bc..5e5b6d35 160000 --- a/ext/express-parser/antlr4-upstream +++ b/ext/express-parser/antlr4-upstream @@ -1 +1 @@ -Subproject commit 7a3f40bc341ddfb463d6e0aa1a6265064d020cb6 +Subproject commit 5e5b6d35b4183fd330102c40947b95c4b5c6abb5 diff --git a/ext/express-parser/antlr4.patch b/ext/express-parser/antlr4.patch deleted file mode 100644 index 798164ed..00000000 --- a/ext/express-parser/antlr4.patch +++ /dev/null @@ -1,21 +0,0 @@ -diff --git a/runtime/Cpp/runtime/src/atn/ATNDeserializationOptions.cpp b/runtime/Cpp/runtime/src/atn/ATNDeserializationOptions.cpp -index a406c4e13..73397107d 100755 ---- a/runtime/Cpp/runtime/src/atn/ATNDeserializationOptions.cpp -+++ b/runtime/Cpp/runtime/src/atn/ATNDeserializationOptions.cpp -@@ -7,8 +7,6 @@ - - using namespace antlr4::atn; - --ATNDeserializationOptions ATNDeserializationOptions::defaultOptions; -- - ATNDeserializationOptions::ATNDeserializationOptions() { - InitializeInstanceFields(); - } -@@ -22,6 +20,7 @@ ATNDeserializationOptions::~ATNDeserializationOptions() { - } - - const ATNDeserializationOptions& ATNDeserializationOptions::getDefaultOptions() { -+ static ATNDeserializationOptions defaultOptions; - return defaultOptions; - } - diff --git a/ext/express-parser/express_parser.cpp b/ext/express-parser/express_parser.cpp index b4d43e3b..e054d083 100644 --- a/ext/express-parser/express_parser.cpp +++ b/ext/express-parser/express_parser.cpp @@ -265,14 +265,6 @@ class ContextProxy { public: ContextProxy(tree::ParseTree* orig) { this -> orig = orig; -// if (orig != nullptr) { -// for (auto it = orig -> children.begin(); it != orig -> children.end(); it ++) { -// Object parseTree = ContextProxy::wrapParseTree(*it); -// if (parseTree != Nil) { -// children.push(parseTree); -// } -// } -// } } tree::ParseTree* getOriginal() { @@ -294,6 +286,11 @@ class ContextProxy { } Array getChildren() { +// Note re memory management +// It looks like it is critical to 'fill' children array each time when this method is called +// When Ruby GC collects the array it also unmarks all its elements so it is not possible to reuse them +// (without custom mark function ????) +// This comment also applies to all methods returning Ruby Arrays below Array children; if (orig != nullptr) { for (auto it = orig -> children.begin(); it != orig -> children.end(); it ++) { @@ -307,11 +304,6 @@ class ContextProxy { } Object getParent() { -// if (parent == Nil) { -// if (orig != nullptr) { -// parent = ContextProxy::wrapParseTree(orig -> parent); -// } -// } return orig == nullptr ? Nil: wrapParseTree(orig -> parent) ; } @@ -334,8 +326,6 @@ class ContextProxy { protected: tree::ParseTree* orig = nullptr; -// Array children; -// Object parent = Nil; }; class TerminalNodeProxy : public ContextProxy { diff --git a/ext/express-parser/extconf.rb b/ext/express-parser/extconf.rb index 92a36583..f8f7a875 100644 --- a/ext/express-parser/extconf.rb +++ b/ext/express-parser/extconf.rb @@ -45,12 +45,10 @@ end else require 'mkmf-rice' - dir_config(extension_name) end -$CPPFLAGS << " -std=c++17 -DANTLR4CPP_STATIC -DHAVE_CXX11 -g -fno-common -fno-omit-frame-pointer" -#-fsanitize=address -fsanitize-address-use-after-scope $DLDFLAGS << " -fsanitize=address" +$CPPFLAGS << " -std=c++17 -DANTLR4CPP_STATIC -DHAVE_CXX11 -fno-omit-frame-pointer" $INCFLAGS << " -I#{__dir__}/#{antlr4_src}" $srcs = [] diff --git a/lib/expressir/express/parser.rb b/lib/expressir/express/parser.rb index 4b9d22c5..ca045bb2 100644 --- a/lib/expressir/express/parser.rb +++ b/lib/expressir/express/parser.rb @@ -33,8 +33,11 @@ class Parser def self.from_file(file, skip_references: nil, include_source: nil) input = File.read(file) + # An important note re memory management + # parse, syntax, visitor methods return complex tree structures created in netive (C++) extension + # visit method references nodes and leaves of this structures but it is totally untransparent for Ruby GarbageCllector + # so in this class we keep those C++ structure marked for GC so theu are not freed @parser = ::ExpressParser::Parser.parse(input) - @parse_tree = @parser.syntax() @visitor = Visitor.new(@parser.tokens, include_source: include_source)