[C++] Fix bugs in ParserATNSimulator and PredictionContextMergeCache (#…

…3644) * [C++] Fix bug in ParserATNSimulator Signed-off-by: Justin King <[email protected]> * [C++] Fix bug in PredictionContextMergeCache Signed-off-by: Justin King <[email protected]> * [C++] Enable asserts during tests and use parallelism based on available resources Signed-off-by: Justin King <[email protected]>
antlr · Apr 11, 2022 · 1684238 · 1684238
1 parent 20abdb5
commit 1684238
Show file tree

Hide file tree

Showing 4 changed files with 20 additions and 12 deletions.
diff --git a/runtime-testsuite/test/org/antlr/v4/test/runtime/cpp/BaseCppTest.java b/runtime-testsuite/test/org/antlr/v4/test/runtime/cpp/BaseCppTest.java
@@ -214,7 +214,7 @@ private boolean buildRuntime() {
 		System.out.println("Building ANTLR4 C++ runtime (if necessary) at "+ runtimePath);
 
 		try {
-			String[] command = { "cmake", ".", /*"-DCMAKE_CXX_COMPILER=clang++",*/ "-DCMAKE_BUILD_TYPE=release" };
+			String[] command = { "cmake", ".", "-DCMAKE_BUILD_TYPE=Debug" };
 			if (runCommand(command, runtimePath, "antlr runtime cmake", false) == null) {
 				return false;
 			}
@@ -224,7 +224,7 @@ private boolean buildRuntime() {
 		}
 
 		try {
-			String[] command = { "make", "-j", "8" }; // Assuming a reasonable amount of available CPU cores.
+			String[] command = { "make", "-j", Integer.toString(Runtime.getRuntime().availableProcessors()) };
 			if (runCommand(command, runtimePath, "building antlr runtime", true) == null)
 				return false;
 		}

diff --git a/runtime/Cpp/runtime/src/atn/ParserATNSimulator.cpp b/runtime/Cpp/runtime/src/atn/ParserATNSimulator.cpp
@@ -715,7 +715,7 @@ std::vector<dfa::DFAState::PredPrediction> ParserATNSimulator::getPredicatePredi
   }) != altToPred.end();
   std::vector<dfa::DFAState::PredPrediction> pairs;
   if (containsPredicate) {
-    for (size_t i = 0; i < altToPred.size(); i++) {
+    for (size_t i = 1; i < altToPred.size(); i++) {
       const auto &pred = altToPred[i];
       assert(pred != nullptr); // unpredicted is indicated by SemanticContext.NONE
       if (ambigAlts.test(i)) {

diff --git a/runtime/Cpp/runtime/src/atn/PredictionContextMergeCache.cpp b/runtime/Cpp/runtime/src/atn/PredictionContextMergeCache.cpp
@@ -47,7 +47,6 @@ Ref<const PredictionContext> PredictionContextMergeCache::put(
 
   auto [existing, inserted] = _entries.try_emplace(std::make_pair(key1.get(), key2.get()));
   if (inserted) {
-    _size++;
     try {
       existing->second.reset(new Entry());
     } catch (...) {
@@ -56,7 +55,7 @@ Ref<const PredictionContext> PredictionContextMergeCache::put(
     }
     existing->second->key = std::make_pair(key1, key2);
     existing->second->value = std::move(value);
-    pushFront(existing->second.get());
+    pushToFront(existing->second.get());
   } else {
     if (existing->second->value != value) {
       existing->second->value = std::move(value);
@@ -94,40 +93,49 @@ void PredictionContextMergeCache::clear() {
 
 void PredictionContextMergeCache::moveToFront(Entry *entry) const {
   if (entry->prev == nullptr) {
+    assert(entry == _head);
     return;
   }
   entry->prev->next = entry->next;
-  if (entry->next) {
+  if (entry->next != nullptr) {
     entry->next->prev = entry->prev;
   } else {
+    assert(entry == _tail);
     _tail = entry->prev;
   }
   entry->prev = nullptr;
   entry->next = _head;
   _head->prev = entry;
   _head = entry;
+  assert(entry->prev == nullptr);
 }
 
-void PredictionContextMergeCache::pushFront(Entry *entry) {
+void PredictionContextMergeCache::pushToFront(Entry *entry) {
+  ++_size;
   entry->prev = nullptr;
   entry->next = _head;
-  if (_head) {
+  if (_head != nullptr) {
     _head->prev = entry;
+    _head = entry;
   } else {
+    assert(entry->next == nullptr);
     _head = entry;
     _tail = entry;
   }
+  assert(entry->prev == nullptr);
 }
 
 void PredictionContextMergeCache::remove(Entry *entry) {
-  if (entry->prev) {
+  if (entry->prev != nullptr) {
     entry->prev->next = entry->next;
   } else {
+    assert(entry == _head);
     _head = entry->next;
   }
-  if (entry->next) {
+  if (entry->next != nullptr) {
     entry->next->prev = entry->prev;
   } else {
+    assert(entry == _tail);
     _tail = entry->prev;
   }
   --_size;
@@ -137,7 +145,7 @@ void PredictionContextMergeCache::remove(Entry *entry) {
 void PredictionContextMergeCache::compact(const Entry *preserve) {
   Entry *entry = _tail;
   while (entry != nullptr && _size > getOptions().getMaxSize()) {
-    Entry *next = entry->next;
+    Entry *next = entry->prev;
     if (entry != preserve) {
       remove(entry);
     }

diff --git a/runtime/Cpp/runtime/src/atn/PredictionContextMergeCache.h b/runtime/Cpp/runtime/src/atn/PredictionContextMergeCache.h
@@ -78,7 +78,7 @@ namespace atn {
 
     void moveToFront(Entry *entry) const;
 
-    void pushFront(Entry *entry);
+    void pushToFront(Entry *entry);
 
     void remove(Entry *entry);