Merge branch 'google:master' into master

.github/workflows/macos.yaml

@@ -13,6 +13,9 @@ concurrency:
   group: ${{ github.workflow }}-${{ github.head_ref || github.ref }}
   cancel-in-progress: true
 
+env:
+  PYTHON_VENV_ROOT: ${{ github.workspace }}/src/python-venv
+
 jobs:
   build_arm64:
     # https://github.com/actions/virtual-environments/blob/main/images/macos/macos-14-Readme.md
@@ -25,9 +28,10 @@ jobs:
         with:
           submodules: 'recursive'
 
-      - name: Install pip dependencies
-        working-directory: ./src
+      - name: Setup Python
         run: |
+          python3 -m venv ${PYTHON_VENV_ROOT}
+          source ${PYTHON_VENV_ROOT}/bin/activate
           python3 -m pip install requests
 
       - name: Try to restore update_deps cache
@@ -40,11 +44,13 @@ jobs:
         working-directory: ./src
         # This command uses src/third_party_cache as the download cache.
         run: |
+          source ${PYTHON_VENV_ROOT}/bin/activate
           python3 build_tools/update_deps.py
 
       - name: Build Qt
         working-directory: ./src
         run: |
+          source ${PYTHON_VENV_ROOT}/bin/activate
           python3 build_tools/build_qt.py --release --confirm_license --macos_cpus=arm64
           echo "MOZC_QT_PATH=${PWD}/third_party/qt" >> $GITHUB_ENV
 
@@ -71,9 +77,10 @@ jobs:
         with:
           submodules: 'recursive'
 
-      - name: Install pip dependencies
-        working-directory: ./src
+      - name: Setup Python
         run: |
+          python3 -m venv ${PYTHON_VENV_ROOT}
+          source ${PYTHON_VENV_ROOT}/bin/activate
           python3 -m pip install requests
 
       - name: Try to restore update_deps cache
@@ -86,11 +93,13 @@ jobs:
         working-directory: ./src
         # This command uses src/third_party_cache as the download cache.
         run: |
+          source ${PYTHON_VENV_ROOT}/bin/activate
           python3 build_tools/update_deps.py
 
       - name: Build Qt
         working-directory: ./src
         run: |
+          source ${PYTHON_VENV_ROOT}/bin/activate
           python3 build_tools/build_qt.py --release --confirm_license --macos_cpus=x86_64
           echo "MOZC_QT_PATH=${PWD}/third_party/qt" >> $GITHUB_ENV
 
@@ -117,9 +126,10 @@ jobs:
         with:
           submodules: 'recursive'
 
-      - name: Install pip dependencies
-        working-directory: ./src
+      - name: Setup Python
         run: |
+          python3 -m venv ${PYTHON_VENV_ROOT}
+          source ${PYTHON_VENV_ROOT}/bin/activate
           python3 -m pip install requests
 
       - name: Try to restore update_deps cache
@@ -132,11 +142,13 @@ jobs:
         working-directory: ./src
         # This command uses src/third_party_cache as the download cache.
         run: |
+          source ${PYTHON_VENV_ROOT}/bin/activate
           python3 build_tools/update_deps.py
 
       - name: Build Qt
         working-directory: ./src
         run: |
+          source ${PYTHON_VENV_ROOT}/bin/activate
           python3 build_tools/build_qt.py --release --confirm_license --macos_cpus=x86_64,arm64
           echo "MOZC_QT_PATH=${PWD}/third_party/qt" >> $GITHUB_ENV
 
@@ -163,9 +175,10 @@ jobs:
         with:
           submodules: 'recursive'
 
-      - name: Install pip dependencies
-        working-directory: ./src
+      - name: Setup Python
         run: |
+          python3 -m venv ${PYTHON_VENV_ROOT}
+          source ${PYTHON_VENV_ROOT}/bin/activate
           python3 -m pip install requests
 
       - name: Try to restore update_deps cache
@@ -178,11 +191,13 @@ jobs:
         working-directory: ./src
         # This command uses src/third_party_cache as the download cache.
         run: |
+          source ${PYTHON_VENV_ROOT}/bin/activate
           python3 build_tools/update_deps.py
 
       - name: Build Qt
         working-directory: ./src
         run: |
+          source ${PYTHON_VENV_ROOT}/bin/activate
           python3 build_tools/build_qt.py --release --confirm_license
           echo "MOZC_QT_PATH=${PWD}/third_party/qt" >> $GITHUB_ENV
 
@@ -203,9 +218,10 @@ jobs:
       - name: checkout
         uses: actions/checkout@v4
 
-      - name: Install pip dependencies
-        working-directory: ./src
+      - name: Setup Python
         run: |
+          python3 -m venv ${PYTHON_VENV_ROOT}
+          source ${PYTHON_VENV_ROOT}/bin/activate
           python3 -m pip install requests
 
       - name: Try to restore update_deps cache
@@ -218,4 +234,5 @@ jobs:
         working-directory: ./src
         # This command uses src/third_party_cache as the download cache.
         run: |
+          source ${PYTHON_VENV_ROOT}/bin/activate
           python3 build_tools/update_deps.py --cache_only

.gitignore

@@ -34,6 +34,10 @@
 /src-*
 /src_*
 
+# Python venv dir used to conform PEP 668
+# https://peps.python.org/pep-0668/
+/src/python-venv
+
 # Mozc build artifacts by Bazel (symbolic links)
 /src/bazel-bin
 /src/bazel-out

docs/build_mozc_in_osx.md

@@ -9,11 +9,14 @@ If you are not sure what the following commands do, please check the description
 and make sure the operations before running them.
 
 ```
-python3 -m pip install requests
-
 git clone https://github.com/google/mozc.git
 cd mozc/src
 
+export PYTHON_VENV_ROOT=${PWD}/python-venv
+python3 -m venv ${PYTHON_VENV_ROOT}
+source ${PYTHON_VENV_ROOT}/bin/activate
+python3 -m pip install requests
+
 python3 build_tools/update_deps.py
 
 # CMake is also required to build Qt.
@@ -62,6 +65,19 @@ cd mozc/src
 
 Hereafter you can do all the operations without changing directory.
 
+### Set up and enable Python virtual environment
+
+The following commands set up Python virtual environment under `mozc/src/python-venv`.
+
+```
+export PYTHON_VENV_ROOT=${PWD}/python-venv
+python3 -m venv ${PYTHON_VENV_ROOT}
+source ${PYTHON_VENV_ROOT}/bin/activate
+python3 -m pip install requests
+```
+
+Using `mozc/src/python-venv` as the virtual environment location is not mandatory. Any other location should also work.
+
 ### Check out additional build dependencies
 
 ```

src/base/thread.h

@@ -47,9 +47,9 @@ namespace mozc {
 // Represents a thread, exposing a subset of `std::thread` APIs.
 //
 // Most notably, threads are undetachable unlike `std::thread`, thus must be
-// `join()`ed before destruction. This means that the `mozc::Thread` instance
-// must be retained even for a long-running one, though which may be until
-// the end of the process.
+// `Join()`ed before destruction if `Joinable()`. This means that the
+// `mozc::Thread` instance must be retained even for a long-running one, though
+// which may be until the end of the process.
 //
 // The semantics of the present APIs are mostly the same as `std::thread`
 // counterpart of the same (but lowercase) name, except that the behavior of
@@ -77,6 +77,8 @@ class Thread {
   Thread(Thread &&) noexcept = default;
   Thread &operator=(Thread &&) noexcept = default;
 
+  bool Joinable() const noexcept { return thread_.joinable(); }
+
   void Join() { thread_.join(); }
 
  private:
@@ -182,7 +184,9 @@ BackgroundFuture<R>::BackgroundFuture(F &&f, Args &&...args)
 
 template <class R>
 BackgroundFuture<R>::~BackgroundFuture() {
-  thread_.Join();
+  if (thread_.Joinable()) {
+    thread_.Join();
+  }
 }
 
 template <class R>
@@ -229,7 +233,11 @@ BackgroundFuture<void>::BackgroundFuture(F &&f, Args &&...args)
         done.Notify();
       }) {}
 
-inline BackgroundFuture<void>::~BackgroundFuture() { thread_.Join(); }
+inline BackgroundFuture<void>::~BackgroundFuture() {
+  if (thread_.Joinable()) {
+    thread_.Join();
+  }
+}
 
 inline void BackgroundFuture<void>::Wait() const {
   done_->WaitForNotification();

src/base/thread_test.cc

@@ -31,6 +31,7 @@
 
 #include <atomic>
 #include <memory>
+#include <optional>
 #include <utility>
 
 #include "absl/synchronization/notification.h"
@@ -93,6 +94,16 @@ TEST(ThreadTest, CopiesThingsAtMostOnce) {
   EXPECT_EQ(c2->load(), 0);
 }
 
+TEST(ThreadTest, Joinable) {
+  Thread default_constructed;
+  EXPECT_FALSE(default_constructed.Joinable());
+
+  Thread real_work([] {});
+  EXPECT_TRUE(real_work.Joinable());
+  real_work.Join();
+  EXPECT_FALSE(real_work.Joinable());
+}
+
 TEST(BackgroundFutureTest, ReturnsComputedValueOnReady) {
   auto future = BackgroundFuture<int>([] {
     absl::SleepFor(absl::Milliseconds(100));
@@ -164,5 +175,14 @@ TEST(BackgroundFutureTest, CopiesThingsAtMostOnce) {
   }
 }
 
+TEST(BackgroundFutureTest, DestructingMovedOutFutureDoesNotCrash) {
+  std::optional<BackgroundFuture<int>> f;
+  {
+    auto g = BackgroundFuture<int>([] { return 42; });
+    f = std::move(g);
+  }
+  EXPECT_EQ(f->Get(), 42);
+}
+
 }  // namespace
 }  // namespace mozc

src/converter/BUILD.bazel

@@ -235,6 +235,7 @@ mozc_cc_library(
         "//prediction:suggestion_filter",
         "//protocol:commands_cc_proto",
         "//request:conversion_request",
+        "//request:request_util",
         "@com_google_absl//absl/container:flat_hash_set",
         "@com_google_absl//absl/hash",
         "@com_google_absl//absl/log:check",
@@ -251,7 +252,6 @@ mozc_cc_test(
         ":candidate_filter",
         ":node",
         ":segments",
-        "//base:logging",
         "//base/container:freelist",
         "//data_manager/testing:mock_data_manager",
         "//dictionary:pos_matcher",
@@ -261,6 +261,7 @@ mozc_cc_test(
         "//request:conversion_request",
         "//request:request_test_util",
         "//testing:gunit_main",
+        "@com_google_absl//absl/log",
         "@com_google_absl//absl/strings",
         "@com_google_absl//absl/strings:str_format",
     ],

src/converter/candidate_filter.cc

@@ -52,6 +52,7 @@
 #include "prediction/suggestion_filter.h"
 #include "protocol/commands.pb.h"
 #include "request/conversion_request.h"
+#include "request/request_util.h"
 
 namespace mozc {
 namespace converter {
@@ -222,6 +223,36 @@ bool IsStrictModeEnabled(const ConversionRequest &request) {
               .enable_realtime_conversion_v2();
 }
 
+// Returns true if there is a number node that does not follow the
+bool IsNoisyNumberCandidate(const dictionary::PosMatcher &pos_matcher,
+                            const absl::Span<const Node *const> nodes) {
+  auto is_converted_number = [&](const Node &node) {
+    if (node.lid != node.rid) {
+      return false;
+    }
+    if (!Util::IsScriptType(node.key, Util::HIRAGANA)) {
+      return false;
+    }
+    return pos_matcher.IsNumber(node.lid) ||
+           pos_matcher.IsKanjiNumber(node.rid);
+  };
+  for (int i = 0; i < nodes.size(); ++i) {
+    if (!is_converted_number(*nodes[i])) {
+      continue;
+    }
+    if (i + 1 < nodes.size() && !is_converted_number(*nodes[i + 1]) &&
+        !pos_matcher.IsCounterSuffixWord(nodes[i + 1]->lid)) {
+      // "にいく": "2行く"
+      return true;
+    }
+    if (i - 1 >= 0 && pos_matcher.IsUniqueNoun(nodes[i - 1]->rid)) {
+      // "しんじゅくに": "新宿2"
+      return true;
+    }
+  }
+  return false;
+}
+
 }  // namespace
 
 CandidateFilter::CandidateFilter(
@@ -321,6 +352,12 @@ CandidateFilter::ResultType CandidateFilter::FilterCandidateInternal(
     return CandidateFilter::GOOD_CANDIDATE;
   }
 
+  if (request_util::ShouldFilterNoisyNumberCandidate(request)) {
+    if (IsNoisyNumberCandidate(*pos_matcher_, nodes)) {
+      return CandidateFilter::BAD_CANDIDATE;
+    }
+  }
+
   const size_t candidate_size = seen_.size();
   if (top_candidate_ == nullptr || candidate_size == 0) {
     top_candidate_ = candidate;
@@ -542,8 +579,8 @@ CandidateFilter::ResultType CandidateFilter::FilterCandidateInternal(
           candidate->structure_cost) {
     // Stops candidates enumeration when we see sufficiently high cost
     // candidate.
-    MOZC_VLOG(2) << "cost is invalid: "
-                 << "top_cost=" << top_cost << " cost_offset=" << cost_offset
+    MOZC_VLOG(2) << "cost is invalid: " << "top_cost=" << top_cost
+                 << " cost_offset=" << cost_offset
                  << " value=" << candidate->value << " cost=" << candidate->cost
                  << " top_structure_cost=" << top_structure_cost
                  << " structure_cost=" << candidate->structure_cost