fix(table_translator): null pointer exception when dict entries are f…

…iltered
rime · Feb 13, 2019 · 77438a9 · 77438a9
1 parent d45240f
commit 77438a9
Show file tree

Hide file tree

Showing 7 changed files with 97 additions and 56 deletions.
diff --git a/src/rime/dict/dictionary.cc b/src/rime/dict/dictionary.cc
@@ -68,37 +68,36 @@ void DictEntryIterator::Sort() {
       dictionary::compare_chunk_by_head_element);
 }
 
-void DictEntryIterator::PrepareEntry() {
-  if (exhausted() || !table_) {
-    return;
-  }
-  const auto& chunk(chunks_[chunk_index_]);
-  entry_ = New<DictEntry>();
-  const auto& e(chunk.entries[chunk.cursor]);
-  DLOG(INFO) << "creating temporary dict entry '"
-             << table_->GetEntryText(e) << "'.";
-  entry_->code = chunk.code;
-  entry_->text = table_->GetEntryText(e);
-  const double kS = 1e8;
-  entry_->weight = (e.weight + 1) / kS * chunk.credibility;
-  if (!chunk.remaining_code.empty()) {
-    entry_->comment = "~" + chunk.remaining_code;
-    entry_->remaining_code_length = chunk.remaining_code.length();
+void DictEntryIterator::AddFilter(DictEntryFilter filter) {
+  DictEntryFilterBinder::AddFilter(filter);
+  // the introduced filter could invalidate the current or even all the
+  // remaining entries
+  while (!exhausted() && !filter_(Peek())) {
+    FindNextEntry();
   }
 }
 
 an<DictEntry> DictEntryIterator::Peek() {
-  while (!entry_ && !exhausted()) {
-    PrepareEntry();
-    if (filter_ && !filter_(entry_)) {
-      Next();
+  if (!entry_ && !exhausted() && table_) {
+    // get next entry from current chunk
+    const auto& chunk(chunks_[chunk_index_]);
+    const auto& e(chunk.entries[chunk.cursor]);
+    DLOG(INFO) << "creating temporary dict entry '"
+               << table_->GetEntryText(e) << "'.";
+    entry_ = New<DictEntry>();
+    entry_->code = chunk.code;
+    entry_->text = table_->GetEntryText(e);
+    const double kS = 1e8;
+    entry_->weight = (e.weight + 1) / kS * chunk.credibility;
+    if (!chunk.remaining_code.empty()) {
+      entry_->comment = "~" + chunk.remaining_code;
+      entry_->remaining_code_length = chunk.remaining_code.length();
     }
   }
   return entry_;
 }
 
-bool DictEntryIterator::Next() {
-  entry_.reset();
+bool DictEntryIterator::FindNextEntry() {
   if (exhausted()) {
     return false;
   }
@@ -113,6 +112,20 @@ bool DictEntryIterator::Next() {
   return !exhausted();
 }
 
+bool DictEntryIterator::Next() {
+  entry_.reset();
+  if (!FindNextEntry()) {
+    return false;
+  }
+  while (filter_ && !filter_(Peek())) {
+    if (!FindNextEntry()) {
+      return false;
+    }
+  }
+  return true;
+}
+
+// Note: does not apply filters
 bool DictEntryIterator::Skip(size_t num_entries) {
   while (num_entries > 0) {
     if (exhausted()) return false;

diff --git a/src/rime/dict/dictionary.h b/src/rime/dict/dictionary.h
@@ -47,14 +47,15 @@ class DictEntryIterator : public DictEntryFilterBinder {
 
   void AddChunk(dictionary::Chunk&& chunk, Table* table);
   void Sort();
+  void AddFilter(DictEntryFilter filter) override;
   RIME_API an<DictEntry> Peek();
   RIME_API bool Next();
   bool Skip(size_t num_entries);
-  bool exhausted() const { return chunk_index_ == chunks_.size(); }
+  bool exhausted() const { return chunk_index_ >= chunks_.size(); }
   size_t entry_count() const { return entry_count_; }
 
  protected:
-  void PrepareEntry();
+  bool FindNextEntry();
 
  private:
   vector<dictionary::Chunk> chunks_;

diff --git a/src/rime/dict/user_dictionary.cc b/src/rime/dict/user_dictionary.cc
@@ -96,23 +96,40 @@ bool UserDictEntryIterator::Release(DictEntryList* receiver) {
   return true;
 }
 
+void UserDictEntryIterator::AddFilter(DictEntryFilter filter) {
+  DictEntryFilterBinder::AddFilter(filter);
+  // the introduced filter could invalidate the current or even all the
+  // remaining entries
+  while (!exhausted() && !filter_(Peek())) {
+    FindNextEntry();
+  }
+}
+
 an<DictEntry> UserDictEntryIterator::Peek() {
-  an<DictEntry> result;
-  while (!result && !exhausted()) {
-    result = (*entries_)[index_];
-    if (filter_ && !filter_(result)) {
-      ++index_;
-      result.reset();
-    }
+  if (exhausted()) {
+    return nullptr;
   }
-  return result;
+  return (*entries_)[index_];
 }
 
-bool UserDictEntryIterator::Next() {
-  if (exhausted())
+bool UserDictEntryIterator::FindNextEntry() {
+  if (exhausted()) {
     return false;
+  }
   ++index_;
-  return exhausted();
+  return !exhausted();
+}
+
+bool UserDictEntryIterator::Next() {
+  if (!FindNextEntry()) {
+    return false;
+  }
+  while (filter_ && !filter_(Peek())) {
+    if (!FindNextEntry()) {
+      return false;
+    }
+  }
+  return true;
 }
 
 // UserDictionary members

diff --git a/src/rime/dict/user_dictionary.h b/src/rime/dict/user_dictionary.h
@@ -26,6 +26,7 @@ class UserDictEntryIterator : public DictEntryFilterBinder {
   void SortRange(size_t start, size_t count);
   bool Release(DictEntryList* receiver);
 
+  void AddFilter(DictEntryFilter filter) override;
   an<DictEntry> Peek();
   bool Next();
   bool exhausted() const {
@@ -36,6 +37,8 @@ class UserDictEntryIterator : public DictEntryFilterBinder {
   }
 
  protected:
+  bool FindNextEntry();
+
   an<DictEntryList> entries_;
   size_t index_ = 0;
 };

diff --git a/src/rime/dict/vocabulary.h b/src/rime/dict/vocabulary.h
@@ -53,7 +53,8 @@ using DictEntryFilter = function<bool (an<DictEntry> entry)>;
 
 class DictEntryFilterBinder {
  public:
-  void AddFilter(DictEntryFilter filter);
+  virtual ~DictEntryFilterBinder() = default;
+  virtual void AddFilter(DictEntryFilter filter);
 
  protected:
   DictEntryFilter filter_;

diff --git a/src/rime/gear/table_translator.cc b/src/rime/gear/table_translator.cc
@@ -67,7 +67,7 @@ an<Candidate> TableTranslation::Peek() {
   if (exhausted())
     return nullptr;
   bool is_user_phrase = PreferUserPhrase();
-  auto e = PreferedEntry(is_user_phrase);
+  auto e = PreferredEntry(is_user_phrase);
   string comment(is_constructed(e.get()) ? kUnitySymbol : e->comment);
   if (options_) {
     options_->comment_formatter().Apply(&comment);
@@ -559,12 +559,14 @@ TableTranslator::MakeSentence(const string& input, size_t start,
         if (filter_by_charset) {
           uter.AddFilter(CharsetFilter::FilterDictEntry);
         }
-        entries[consumed_length] = uter.Peek();
-        if (start_pos == 0 && !uter.exhausted()) {
-          // also provide words for manual composition
-          uter.Release(&user_phrase_collector[consumed_length]);
-          DLOG(INFO) << "user phrase[" << consumed_length << "]: "
-                     << user_phrase_collector[consumed_length].size();
+        if (!uter.exhausted()) {
+          entries[consumed_length] = uter.Peek();
+          if (start_pos == 0) {
+            // also provide words for manual composition
+            uter.Release(&user_phrase_collector[consumed_length]);
+            DLOG(INFO) << "user phrase[" << consumed_length << "]: "
+                       << user_phrase_collector[consumed_length].size();
+          }
         }
         if (resume_key > active_key &&
             !boost::starts_with(resume_key, active_key))
@@ -586,12 +588,14 @@ TableTranslator::MakeSentence(const string& input, size_t start,
         if (filter_by_charset) {
           uter.AddFilter(CharsetFilter::FilterDictEntry);
         }
-        entries[consumed_length] = uter.Peek();
-        if (start_pos == 0 && !uter.exhausted()) {
-          // also provide words for manual composition
-          uter.Release(&user_phrase_collector[consumed_length]);
-          DLOG(INFO) << "unity phrase[" << consumed_length << "]: "
-                     << user_phrase_collector[consumed_length].size();
+        if (!uter.exhausted()) {
+          entries[consumed_length] = uter.Peek();
+          if (start_pos == 0) {
+            // also provide words for manual composition
+            uter.Release(&user_phrase_collector[consumed_length]);
+            DLOG(INFO) << "unity phrase[" << consumed_length << "]: "
+                       << user_phrase_collector[consumed_length].size();
+          }
         }
         if (resume_key > active_key &&
             !boost::starts_with(resume_key, active_key))
@@ -615,12 +619,14 @@ TableTranslator::MakeSentence(const string& input, size_t start,
         if (filter_by_charset) {
           iter.AddFilter(CharsetFilter::FilterDictEntry);
         }
-        entries[consumed_length] = iter.Peek();
-        if (start_pos == 0 && !iter.exhausted()) {
-          // also provide words for manual composition
-          collector[consumed_length] = std::move(iter);
-          DLOG(INFO) << "table[" << consumed_length << "]: "
-                     << collector[consumed_length].entry_count();
+        if (!iter.exhausted()) {
+          entries[consumed_length] = iter.Peek();
+          if (start_pos == 0) {
+            // also provide words for manual composition
+            collector[consumed_length] = std::move(iter);
+            DLOG(INFO) << "table[" << consumed_length << "]: "
+                       << collector[consumed_length].entry_count();
+          }
         }
       }
     }

diff --git a/src/rime/gear/table_translator.h b/src/rime/gear/table_translator.h
@@ -69,7 +69,7 @@ class TableTranslation : public Translation {
   bool CheckEmpty();
   bool PreferUserPhrase();
 
-  an<DictEntry> PreferedEntry(bool prefer_user_phrase) {
+  an<DictEntry> PreferredEntry(bool prefer_user_phrase) {
     return prefer_user_phrase ? uter_.Peek() : iter_.Peek();
   }