From 7a1bc7d58b824bacee72f9b40101ef9809c23d7c Mon Sep 17 00:00:00 2001 From: Frank Lin Date: Tue, 14 Jul 2026 18:57:01 -0700 Subject: [PATCH 1/2] Add experimental Converter::GetConversionCandidates behind a macro guard MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Introduce a librime-style word-level candidate query that enumerates every form a single word can take through a converter's conversion chain (e.g. s2t expands 里 to 里 and 裏). The API is gated by OPENCC_ENABLE_UNSTABLE_API so it stays out of the installed headers and the OPENCC_ABI_VERSION contract until it is promoted at the next ABI bump. The member is deliberately non-virtual for now, so enabling the macro does not alter the Converter vtable and remains ABI-safe; it delegates to a free function kept in a private (non-installed) header, making the eventual promotion to a virtual method a trivial change. Detailed Changes: - **New candidate API**: - Add `GetAllConversions(const Converter&, std::string_view)` in the private `ConversionCandidates.hpp/.cpp`, porting librime's ConvertWord loop onto OpenCC's Dict/DictEntry primitives (exact match yields all values; otherwise longest-prefix default conversion carries partial matches down the chain). - Add the guarded, non-virtual `Converter::GetConversionCandidates` member that forwards to the free function; define it alongside the free function to avoid a Bazel dependency cycle with converter_lib. - **Build wiring**: - CMake: register the new source/private header/test and define `OPENCC_ENABLE_UNSTABLE_API` as PRIVATE on libopencc and the test target. - Bazel: add `conversion_candidates_lib` and its test using non-propagating `local_defines`, keeping the header out of `exports_files`. - **Tests**: - Add ConversionCandidatesTest covering multi-value expansion, chained branch carry-through, dedup, not-found and prefix-only empties, partial conversion in a later dict, PipelineConverter returning empty, and member/free-function agreement. --- src/BUILD.bazel | 44 ++++++++++ src/CMakeLists.txt | 14 ++++ src/ConversionCandidates.cpp | 99 ++++++++++++++++++++++ src/ConversionCandidates.hpp | 65 +++++++++++++++ src/ConversionCandidatesTest.cpp | 137 +++++++++++++++++++++++++++++++ src/Converter.hpp | 20 +++++ 6 files changed, 379 insertions(+) create mode 100644 src/ConversionCandidates.cpp create mode 100644 src/ConversionCandidates.hpp create mode 100644 src/ConversionCandidatesTest.cpp diff --git a/src/BUILD.bazel b/src/BUILD.bazel index d38d34453..db43ea3fc 100644 --- a/src/BUILD.bazel +++ b/src/BUILD.bazel @@ -40,6 +40,7 @@ cc_library( ":common_lib", ":config_lib", ":conversion_lib", + ":conversion_candidates_lib", ":conversion_chain_lib", ":converter_lib", ":darts_dict_lib", @@ -275,6 +276,49 @@ cc_library( ], ) +# Internal, unstable candidate-enumeration API. local_defines (non-propagating) +# keeps OPENCC_ENABLE_UNSTABLE_API scoped to this library's own sources, so the +# guarded Converter::GetConversionCandidates member is compiled here without +# leaking the macro into dependents. ConversionCandidates.hpp is intentionally +# absent from exports_files, so consumers cannot reach the API until the next +# ABI bump. +cc_library( + name = "conversion_candidates_lib", + srcs = ["ConversionCandidates.cpp"], + hdrs = ["ConversionCandidates.hpp"], + local_defines = ["OPENCC_ENABLE_UNSTABLE_API"], + deps = [ + ":common_lib", + ":conversion_lib", + ":conversion_chain_lib", + ":converter_lib", + ":dict_lib", + ":dict_entry_lib", + ":utf8_util_lib", + ], +) + +cc_test( + name = "conversion_candidates_test", + size = "small", + srcs = ["ConversionCandidatesTest.cpp"], + local_defines = ["OPENCC_ENABLE_UNSTABLE_API"], + deps = [ + ":conversion_candidates_lib", + ":conversion_lib", + ":conversion_chain_lib", + ":converter_lib", + ":lexicon_lib", + ":max_match_segmentation_lib", + ":pipeline_converter_lib", + ":single_stage_converter_lib", + ":test_utils_lib", + ":test_utils_utf8_lib", + ":text_dict_lib", + "@googletest//:gtest_main", + ], +) + cc_library( name = "single_stage_converter_lib", srcs = ["SingleStageConverter.cpp"], diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index 17538302d..bdd969fa4 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -45,6 +45,7 @@ set( set( LIBOPENCC_PRIVATE_HEADERS ConfigBasedConverter.hpp + ConversionCandidates.hpp DictConverter.hpp PhraseExtract.hpp PipelineConverter.hpp @@ -59,6 +60,7 @@ set( LIBOPENCC_SOURCES Config.cpp Conversion.cpp + ConversionCandidates.cpp ConversionChain.cpp Converter.cpp Dict.cpp @@ -85,6 +87,7 @@ set( set(UNITTESTS ConfigTest + ConversionCandidatesTest ConversionChainTest ConversionInspectionTest ConversionTest @@ -150,6 +153,11 @@ target_link_libraries(libopencc marisa) if (NOT BUILD_SHARED_LIBS) target_compile_definitions(libopencc PUBLIC Opencc_BUILT_AS_STATIC) endif() +# Experimental, not-yet-stable API (e.g. Converter::GetConversionCandidates). +# PRIVATE so the guarded declarations compile into the library but never leak +# into the installed headers' interface; consumers cannot see the API until it +# is promoted at the next ABI bump. See src/ConversionCandidates.hpp. +target_compile_definitions(libopencc PRIVATE OPENCC_ENABLE_UNSTABLE_API) # PluginSegmentation.cpp uses LoadLibrary on _WIN32 (MSVC and MinGW). # Only non-Windows targets may need libdl for dlopen. if (NOT CMAKE_SYSTEM_NAME STREQUAL "Windows") @@ -245,6 +253,12 @@ if (ENABLE_GTEST) add_dependencies(${TESTCASE} copy_gtest_to_src copy_gtest_main_to_src) endif() endforeach() + + # ConversionCandidatesTest exercises the experimental + # Converter::GetConversionCandidates member, so its own translation unit must + # see the guarded declaration. This mirrors the PRIVATE define on libopencc. + target_compile_definitions(ConversionCandidatesTest + PRIVATE OPENCC_ENABLE_UNSTABLE_API) endif() # Benchmark diff --git a/src/ConversionCandidates.cpp b/src/ConversionCandidates.cpp new file mode 100644 index 000000000..14b7201ba --- /dev/null +++ b/src/ConversionCandidates.cpp @@ -0,0 +1,99 @@ +/* + * Open Chinese Convert + * + * Copyright 2010-2026 Carbo Kuo and contributors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include "ConversionCandidates.hpp" + +#include + +#include "Common.hpp" +#include "Conversion.hpp" +#include "ConversionChain.hpp" +#include "Converter.hpp" +#include "Dict.hpp" +#include "DictEntry.hpp" +#include "Optional.hpp" +#include "UTF8Util.hpp" + +namespace opencc { + +std::vector GetAllConversions(const Converter& converter, + std::string_view word) { + const ConversionChainPtr chain = converter.GetConversionChain(); + if (chain == nullptr) { + return {}; + } + + std::vector currentWords{std::string(word)}; + bool matched = false; + for (const ConversionPtr& conversion : chain->GetConversions()) { + const DictPtr dict = conversion->GetDict(); + if (dict == nullptr) { + return {}; + } + std::unordered_set seen; + std::vector nextWords; + for (const std::string& currentWord : currentWords) { + const Optional item = dict->Match(currentWord); + if (item.IsNull()) { + // No exact match: convert greedily by longest prefix so a partially + // convertible word can still flow through the rest of the chain. Even + // when the result is unchanged we keep it, because a later dictionary + // may still convert it. + std::string buffer; + for (const char* p = currentWord.c_str(); *p != '\0';) { + const Optional prefix = dict->MatchPrefix(p); + size_t matchedLength; + if (prefix.IsNull()) { + matchedLength = UTF8Util::NextCharLength(p); + buffer.append(p, matchedLength); + } else { + matchedLength = prefix.Get()->KeyLength(); + buffer.append(prefix.Get()->GetDefault()); + } + p += matchedLength; + } + if (seen.insert(buffer).second) { + nextWords.push_back(std::move(buffer)); + } + continue; + } + matched = true; + for (const std::string& value : item.Get()->Values()) { + if (seen.insert(value).second) { + nextWords.push_back(value); + } + } + } + currentWords.swap(nextWords); + } + + if (!matched) { + // No dictionary in the chain contains the word. + return {}; + } + return currentWords; +} + +#ifdef OPENCC_ENABLE_UNSTABLE_API +std::vector +Converter::GetConversionCandidates(std::string_view word) const { + return GetAllConversions(*this, word); +} +#endif + +} // namespace opencc diff --git a/src/ConversionCandidates.hpp b/src/ConversionCandidates.hpp new file mode 100644 index 000000000..d7a385178 --- /dev/null +++ b/src/ConversionCandidates.hpp @@ -0,0 +1,65 @@ +/* + * Open Chinese Convert + * + * Copyright 2010-2026 Carbo Kuo and contributors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#pragma once + +#include +#include +#include + +#include "Export.hpp" + +namespace opencc { + +class Converter; + +/** + * Enumerates every candidate form of a single word produced by running it + * through @p converter's conversion chain. + * + * Unlike Converter::Convert(), which segments the whole text and yields exactly + * one output string, this walks @p word through each dictionary in the chain + * and keeps @b all branch values. For example, @c s2t expands @c 里 to both + * @c 里 and @c 裏, then @c t2tw passes @c 里 through unchanged and converts + * @c 裏 to @c 裡. This mirrors the behaviour input-method engines (e.g. + * librime's @c ConvertWord) need to offer users every plausible conversion of a + * single word. + * + * Matching a word against a dictionary: + * - On an exact match, every value of the entry becomes a candidate. + * - Otherwise the word is converted greedily by longest-prefix, taking each + * matched prefix's default value, so a partially convertible word still + * flows through the remaining dictionaries in the chain. + * + * @param converter Source of the conversion chain. A converter without a + * single chain (e.g. @c PipelineConverter, whose GetConversionChain() + * returns @c nullptr) yields an empty result. + * @param word UTF-8 text of a single word; need not be null-terminated. + * @return Candidate forms in discovery order with duplicates removed. Empty + * when no dictionary in the chain contains @p word, matching librime's + * convention of reporting "not found". + * + * @note Internal, unstable API. This header is intentionally kept out of the + * installed public header set and is not part of the OPENCC_ABI_VERSION + * contract. It backs the experimental Converter::GetConversionCandidates() + * member, which is compiled only when OPENCC_ENABLE_UNSTABLE_API is defined. + */ +OPENCC_EXPORT std::vector +GetAllConversions(const Converter& converter, std::string_view word); + +} // namespace opencc diff --git a/src/ConversionCandidatesTest.cpp b/src/ConversionCandidatesTest.cpp new file mode 100644 index 000000000..c2ee24194 --- /dev/null +++ b/src/ConversionCandidatesTest.cpp @@ -0,0 +1,137 @@ +/* + * Open Chinese Convert + * + * Copyright 2010-2026 Carbo Kuo and contributors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include "ConversionCandidates.hpp" + +#include "Conversion.hpp" +#include "ConversionChain.hpp" +#include "Converter.hpp" +#include "Lexicon.hpp" +#include "MaxMatchSegmentation.hpp" +#include "PipelineConverter.hpp" +#include "SingleStageConverter.hpp" +#include "TestUtils.hpp" +#include "TestUtilsUTF8.hpp" +#include "TextDict.hpp" + +namespace opencc { + +class ConversionCandidatesTest : public ::testing::Test { +protected: + static DictPtr MakeDict( + const std::vector>>& + entries) { + LexiconPtr lexicon(new Lexicon); + for (const auto& entry : entries) { + lexicon->Add(DictEntryFactory::New(entry.first, entry.second)); + } + lexicon->Sort(); + return DictPtr(new TextDict(lexicon)); + } + + static ConverterPtr MakeConverter(const std::list& dicts) { + std::list conversions; + for (const DictPtr& dict : dicts) { + conversions.push_back(ConversionPtr(new Conversion(dict))); + } + ConversionChainPtr chain(new ConversionChain(conversions)); + // Segmentation is irrelevant to word-level candidate lookup; any dict works. + SegmentationPtr segmentation( + new MaxMatchSegmentation(dicts.empty() ? MakeDict({}) : dicts.front())); + return ConverterPtr(new SingleStageConverter(segmentation, chain)); + } +}; + +// A single dictionary with multiple values returns every value. +TEST_F(ConversionCandidatesTest, SingleDictMultipleValues) { + const ConverterPtr converter = + MakeConverter({MakeDict({{utf8("里"), {utf8("里"), utf8("裏")}}})}); + const std::vector candidates = + converter->GetConversionCandidates(utf8("里")); + EXPECT_EQ((std::vector{utf8("里"), utf8("裏")}), candidates); +} + +// Candidates from an earlier dictionary flow independently through a later one: +// s2t expands 里 -> {里, 裏}; t2tw then passes 里 through and maps 裏 -> 裡. +TEST_F(ConversionCandidatesTest, ChainedExpansionCarriesBranches) { + const ConverterPtr converter = MakeConverter({ + MakeDict({{utf8("里"), {utf8("里"), utf8("裏")}}}), + MakeDict({{utf8("裏"), {utf8("裡")}}}), + }); + const std::vector candidates = + converter->GetConversionCandidates(utf8("里")); + EXPECT_EQ((std::vector{utf8("里"), utf8("裡")}), candidates); +} + +// Duplicate branches collapse: both 麵 and 面 map to 面 downstream. +TEST_F(ConversionCandidatesTest, DuplicateCandidatesAreDeduped) { + const ConverterPtr converter = MakeConverter({ + MakeDict({{utf8("面"), {utf8("麵"), utf8("面")}}}), + MakeDict({{utf8("麵"), {utf8("面")}}}), + }); + const std::vector candidates = + converter->GetConversionCandidates(utf8("面")); + EXPECT_EQ((std::vector{utf8("面")}), candidates); +} + +// A word that no dictionary in the chain contains yields an empty result. +TEST_F(ConversionCandidatesTest, WordNotInAnyDictReturnsEmpty) { + const ConverterPtr converter = + MakeConverter({MakeDict({{utf8("里"), {utf8("裏")}}})}); + EXPECT_TRUE(converter->GetConversionCandidates(utf8("国")).empty()); +} + +// Only a prefix (not the whole word) matches: no exact match means "not found", +// matching librime's ConvertWord contract. +TEST_F(ConversionCandidatesTest, PrefixOnlyMatchReturnsEmpty) { + const ConverterPtr converter = + MakeConverter({MakeDict({{utf8("里"), {utf8("裏")}}})}); + EXPECT_TRUE(converter->GetConversionCandidates(utf8("里面")).empty()); +} + +// An exact match on a longer word still carries an unconvertible tail through a +// later dictionary via longest-prefix conversion. +TEST_F(ConversionCandidatesTest, PartialConversionInLaterDict) { + const ConverterPtr converter = MakeConverter({ + MakeDict({{utf8("里面"), {utf8("裏面")}}}), + MakeDict({{utf8("裏"), {utf8("裡")}}}), + }); + const std::vector candidates = + converter->GetConversionCandidates(utf8("里面")); + // Stage 1: 里面 -> 裏面. Stage 2: 裏 -> 裡 (prefix), 面 copied -> 裡面. + EXPECT_EQ((std::vector{utf8("裡面")}), candidates); +} + +// A converter without a single chain (PipelineConverter) yields no candidates. +TEST_F(ConversionCandidatesTest, PipelineConverterYieldsEmpty) { + const ConverterPtr stage = + MakeConverter({MakeDict({{utf8("里"), {utf8("裏")}}})}); + ConverterPtr pipeline(new PipelineConverter({stage})); + EXPECT_EQ(nullptr, pipeline->GetConversionChain()); + EXPECT_TRUE(GetAllConversions(*pipeline, utf8("里")).empty()); +} + +// The free function and the member wrapper agree. +TEST_F(ConversionCandidatesTest, MemberDelegatesToFreeFunction) { + const ConverterPtr converter = + MakeConverter({MakeDict({{utf8("里"), {utf8("里"), utf8("裏")}}})}); + EXPECT_EQ(GetAllConversions(*converter, utf8("里")), + converter->GetConversionCandidates(utf8("里"))); +} + +} // namespace opencc diff --git a/src/Converter.hpp b/src/Converter.hpp index df3567a41..be4174994 100644 --- a/src/Converter.hpp +++ b/src/Converter.hpp @@ -21,6 +21,7 @@ #include #include #include +#include #include "Common.hpp" #include "ConversionInspection.hpp" @@ -74,6 +75,25 @@ class OPENCC_EXPORT Converter { * converter has no single chain (e.g. @c PipelineConverter). */ virtual ConversionChainPtr GetConversionChain() const = 0; + +#ifdef OPENCC_ENABLE_UNSTABLE_API + /** + * Enumerates every candidate form of a single @p word produced by this + * converter's conversion chain (e.g. @c s2t expands @c 里 to @c 里 and + * @c 裏). Unlike Convert(), which segments the whole input and returns one + * string, this returns all branch values for a single word and is intended + * for input-method and tooling use. Delegates to GetAllConversions(); see + * ConversionCandidates.hpp for exact semantics. + * + * @warning Experimental, unstable API guarded by @c OPENCC_ENABLE_UNSTABLE_API + * and deliberately absent from released headers. It is @b not part of the + * @c OPENCC_ABI_VERSION contract. This is currently a non-virtual member so + * that enabling the macro does not alter the vtable; when the API is + * promoted at the next ABI bump the guard is removed and this becomes a + * @c virtual method. + */ + std::vector GetConversionCandidates(std::string_view word) const; +#endif }; class OPENCC_EXPORT ConverterStream { From 4e91f32d965019360c2979c35f055e5d10e1522f Mon Sep 17 00:00:00 2001 From: Frank Lin Date: Tue, 14 Jul 2026 20:07:16 -0700 Subject: [PATCH 2/2] Fix correctness and containment issues in GetConversionCandidates Code review of the experimental candidate-enumeration API found a heap over-read on truncated UTF-8, divergence from Convert() on IDS sequences and key-only dictionary entries, a config normalization pre-pass being skipped, and an ODR violation plus ineffective containment from the macro-guarded Converter member. Detailed Changes: - **ConversionCandidates.cpp**: - Delegate the no-exact-match fallback to Conversion::Convert() instead of a hand-rolled prefix loop, fixing an out-of-bounds read on truncated UTF-8 input and restoring atomic handling of ideographic description sequences. - Treat a key-only dictionary entry as converting to itself, matching Convert()'s GetDefault() fallback, instead of silently dropping the word. - Normalize the input through ConfigBasedConverter's normalization pre-pass before walking the main chain, so standard configs like s2t.json no longer report "not found" for compatibility ideographs that Convert() handles correctly. - **Converter.hpp / CMakeLists.txt / BUILD.bazel**: Remove the OPENCC_ENABLE_UNSTABLE_API-guarded Converter member and all four macro definition sites. The guard gave class Converter differing definitions across translation units (an ODR violation, live today in the Bazel build), and Converter.hpp is an installed public header, so the API was never actually contained. Only the private free function GetAllConversions remains; a member can be added in one step at the next ABI bump. - **BUILD.bazel**: Restrict conversion_candidates_lib to package-private visibility, since exports_files does not gate cc_library header reachability as the previous comment assumed. Split ConfigBasedConverter.hpp out of config_lib into its own package-private target so both config_lib and conversion_candidates_lib can depend on it. - **ConversionCandidatesTest.cpp**: Switch tests to call GetAllConversions directly; add regression coverage for truncated UTF-8, IDS atomicity, key-only entries, and the normalization pre-pass. --- src/BUILD.bazel | 33 ++++++++++------- src/CMakeLists.txt | 11 ------ src/ConfigBasedConverter.hpp | 8 +++++ src/ConversionCandidates.cpp | 45 +++++++++++------------ src/ConversionCandidates.hpp | 24 ++++++++----- src/ConversionCandidatesTest.cpp | 62 ++++++++++++++++++++++++++------ src/Converter.hpp | 20 ----------- 7 files changed, 119 insertions(+), 84 deletions(-) diff --git a/src/BUILD.bazel b/src/BUILD.bazel index db43ea3fc..823cedd88 100644 --- a/src/BUILD.bazel +++ b/src/BUILD.bazel @@ -127,15 +127,24 @@ diff_test( file2 = "opencc_config_schema.inc", ) +# Private header shared between Config.cpp (which builds the converter) and +# conversion_candidates_lib (which unwraps the normalization pre-pass). cc_library( - name = "config_lib", - srcs = [ - "Config.cpp", - "ConfigBasedConverter.hpp", + name = "config_based_converter_lib", + hdrs = ["ConfigBasedConverter.hpp"], + visibility = ["//src:__pkg__"], + deps = [ + ":converter_lib", ], +) + +cc_library( + name = "config_lib", + srcs = ["Config.cpp"], hdrs = ["Config.hpp"], deps = [ ":common_lib", + ":config_based_converter_lib", ":config_schema_inc_lib", ":conversion_chain_lib", ":converter_lib", @@ -276,25 +285,23 @@ cc_library( ], ) -# Internal, unstable candidate-enumeration API. local_defines (non-propagating) -# keeps OPENCC_ENABLE_UNSTABLE_API scoped to this library's own sources, so the -# guarded Converter::GetConversionCandidates member is compiled here without -# leaking the macro into dependents. ConversionCandidates.hpp is intentionally -# absent from exports_files, so consumers cannot reach the API until the next -# ABI bump. +# Internal, unstable candidate-enumeration API (GetAllConversions). The header +# is not installed by the CMake build and the function is not part of the +# OPENCC_ABI_VERSION contract; visibility is restricted to this package so no +# other Bazel target can depend on it directly until the API is promoted. cc_library( name = "conversion_candidates_lib", srcs = ["ConversionCandidates.cpp"], hdrs = ["ConversionCandidates.hpp"], - local_defines = ["OPENCC_ENABLE_UNSTABLE_API"], + visibility = ["//src:__pkg__"], deps = [ ":common_lib", + ":config_based_converter_lib", ":conversion_lib", ":conversion_chain_lib", ":converter_lib", ":dict_lib", ":dict_entry_lib", - ":utf8_util_lib", ], ) @@ -302,8 +309,8 @@ cc_test( name = "conversion_candidates_test", size = "small", srcs = ["ConversionCandidatesTest.cpp"], - local_defines = ["OPENCC_ENABLE_UNSTABLE_API"], deps = [ + ":config_based_converter_lib", ":conversion_candidates_lib", ":conversion_lib", ":conversion_chain_lib", diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index bdd969fa4..ec6361936 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -153,11 +153,6 @@ target_link_libraries(libopencc marisa) if (NOT BUILD_SHARED_LIBS) target_compile_definitions(libopencc PUBLIC Opencc_BUILT_AS_STATIC) endif() -# Experimental, not-yet-stable API (e.g. Converter::GetConversionCandidates). -# PRIVATE so the guarded declarations compile into the library but never leak -# into the installed headers' interface; consumers cannot see the API until it -# is promoted at the next ABI bump. See src/ConversionCandidates.hpp. -target_compile_definitions(libopencc PRIVATE OPENCC_ENABLE_UNSTABLE_API) # PluginSegmentation.cpp uses LoadLibrary on _WIN32 (MSVC and MinGW). # Only non-Windows targets may need libdl for dlopen. if (NOT CMAKE_SYSTEM_NAME STREQUAL "Windows") @@ -253,12 +248,6 @@ if (ENABLE_GTEST) add_dependencies(${TESTCASE} copy_gtest_to_src copy_gtest_main_to_src) endif() endforeach() - - # ConversionCandidatesTest exercises the experimental - # Converter::GetConversionCandidates member, so its own translation unit must - # see the guarded declaration. This mirrors the PRIVATE define on libopencc. - target_compile_definitions(ConversionCandidatesTest - PRIVATE OPENCC_ENABLE_UNSTABLE_API) endif() # Benchmark diff --git a/src/ConfigBasedConverter.hpp b/src/ConfigBasedConverter.hpp index c8aa6492e..0cb9ee579 100644 --- a/src/ConfigBasedConverter.hpp +++ b/src/ConfigBasedConverter.hpp @@ -63,6 +63,14 @@ class ConfigBasedConverter : public Converter { return mainConverter->GetConversionChain(); } + /** + * Returns the normalization pre-pass converter. Used by GetAllConversions() + * to normalize a word before walking the main chain, mirroring Convert(). + */ + const ConverterPtr& GetNormalizationConverter() const { + return normConverter; + } + private: const ConverterPtr normConverter; const ConverterPtr mainConverter; diff --git a/src/ConversionCandidates.cpp b/src/ConversionCandidates.cpp index 14b7201ba..63bd2c986 100644 --- a/src/ConversionCandidates.cpp +++ b/src/ConversionCandidates.cpp @@ -21,13 +21,13 @@ #include #include "Common.hpp" +#include "ConfigBasedConverter.hpp" #include "Conversion.hpp" #include "ConversionChain.hpp" #include "Converter.hpp" #include "Dict.hpp" #include "DictEntry.hpp" #include "Optional.hpp" -#include "UTF8Util.hpp" namespace opencc { @@ -38,6 +38,17 @@ std::vector GetAllConversions(const Converter& converter, return {}; } + // A config-based converter runs a normalization pre-pass (e.g. CJK + // Compatibility Ideographs) before its main chain, but GetConversionChain() + // exposes only the main chain. Mirror Convert() by normalizing first, so a + // word whose dictionary key appears only after normalization is still found. + std::string normalized; + if (const auto* configBased = + dynamic_cast(&converter)) { + normalized = configBased->GetNormalizationConverter()->Convert(word); + word = normalized; + } + std::vector currentWords{std::string(word)}; bool matched = false; for (const ConversionPtr& conversion : chain->GetConversions()) { @@ -53,26 +64,23 @@ std::vector GetAllConversions(const Converter& converter, // No exact match: convert greedily by longest prefix so a partially // convertible word can still flow through the rest of the chain. Even // when the result is unchanged we keep it, because a later dictionary - // may still convert it. - std::string buffer; - for (const char* p = currentWord.c_str(); *p != '\0';) { - const Optional prefix = dict->MatchPrefix(p); - size_t matchedLength; - if (prefix.IsNull()) { - matchedLength = UTF8Util::NextCharLength(p); - buffer.append(p, matchedLength); - } else { - matchedLength = prefix.Get()->KeyLength(); - buffer.append(prefix.Get()->GetDefault()); - } - p += matchedLength; - } + // may still convert it. Conversion::Convert also handles ideographic + // description sequences and truncated UTF-8 safely. + std::string buffer = conversion->Convert(currentWord); if (seen.insert(buffer).second) { nextWords.push_back(std::move(buffer)); } continue; } matched = true; + if (item.Get()->NumValues() == 0) { + // A key-only entry converts the word to itself, matching Convert(), + // which falls back to the key via DictEntry::GetDefault(). + if (seen.insert(currentWord).second) { + nextWords.push_back(currentWord); + } + continue; + } for (const std::string& value : item.Get()->Values()) { if (seen.insert(value).second) { nextWords.push_back(value); @@ -89,11 +97,4 @@ std::vector GetAllConversions(const Converter& converter, return currentWords; } -#ifdef OPENCC_ENABLE_UNSTABLE_API -std::vector -Converter::GetConversionCandidates(std::string_view word) const { - return GetAllConversions(*this, word); -} -#endif - } // namespace opencc diff --git a/src/ConversionCandidates.hpp b/src/ConversionCandidates.hpp index d7a385178..6226c1214 100644 --- a/src/ConversionCandidates.hpp +++ b/src/ConversionCandidates.hpp @@ -41,10 +41,15 @@ class Converter; * single word. * * Matching a word against a dictionary: - * - On an exact match, every value of the entry becomes a candidate. - * - Otherwise the word is converted greedily by longest-prefix, taking each - * matched prefix's default value, so a partially convertible word still - * flows through the remaining dictionaries in the chain. + * - On an exact match, every value of the entry becomes a candidate; a + * key-only entry (no values) yields the word itself, matching Convert(). + * - Otherwise the word is converted with Conversion::Convert (greedy + * longest-prefix, each matched prefix's default value), so a partially + * convertible word still flows through the remaining dictionaries in the + * chain. + * + * A converter loaded from a config with a @c normalization step (e.g. + * @c s2t.json) normalizes @p word first, mirroring Converter::Convert(). * * @param converter Source of the conversion chain. A converter without a * single chain (e.g. @c PipelineConverter, whose GetConversionChain() @@ -54,10 +59,13 @@ class Converter; * when no dictionary in the chain contains @p word, matching librime's * convention of reporting "not found". * - * @note Internal, unstable API. This header is intentionally kept out of the - * installed public header set and is not part of the OPENCC_ABI_VERSION - * contract. It backs the experimental Converter::GetConversionCandidates() - * member, which is compiled only when OPENCC_ENABLE_UNSTABLE_API is defined. + * @note Internal, unstable API with no compatibility guarantee. This header + * is not installed by CMake and the function is not part of the + * OPENCC_ABI_VERSION contract; like other non-installed headers + * (e.g. PhraseExtract.hpp) the symbol is still OPENCC_EXPORT so unit tests + * can link against a shared libopencc on Windows, which means the symbol is + * technically reachable by forward declaration — binding to it outside this + * repository is unsupported and may break without an ABI bump. */ OPENCC_EXPORT std::vector GetAllConversions(const Converter& converter, std::string_view word); diff --git a/src/ConversionCandidatesTest.cpp b/src/ConversionCandidatesTest.cpp index c2ee24194..cb4a897a9 100644 --- a/src/ConversionCandidatesTest.cpp +++ b/src/ConversionCandidatesTest.cpp @@ -18,6 +18,7 @@ #include "ConversionCandidates.hpp" +#include "ConfigBasedConverter.hpp" #include "Conversion.hpp" #include "ConversionChain.hpp" #include "Converter.hpp" @@ -62,7 +63,7 @@ TEST_F(ConversionCandidatesTest, SingleDictMultipleValues) { const ConverterPtr converter = MakeConverter({MakeDict({{utf8("里"), {utf8("里"), utf8("裏")}}})}); const std::vector candidates = - converter->GetConversionCandidates(utf8("里")); + GetAllConversions(*converter, utf8("里")); EXPECT_EQ((std::vector{utf8("里"), utf8("裏")}), candidates); } @@ -74,7 +75,7 @@ TEST_F(ConversionCandidatesTest, ChainedExpansionCarriesBranches) { MakeDict({{utf8("裏"), {utf8("裡")}}}), }); const std::vector candidates = - converter->GetConversionCandidates(utf8("里")); + GetAllConversions(*converter, utf8("里")); EXPECT_EQ((std::vector{utf8("里"), utf8("裡")}), candidates); } @@ -85,7 +86,7 @@ TEST_F(ConversionCandidatesTest, DuplicateCandidatesAreDeduped) { MakeDict({{utf8("麵"), {utf8("面")}}}), }); const std::vector candidates = - converter->GetConversionCandidates(utf8("面")); + GetAllConversions(*converter, utf8("面")); EXPECT_EQ((std::vector{utf8("面")}), candidates); } @@ -93,7 +94,7 @@ TEST_F(ConversionCandidatesTest, DuplicateCandidatesAreDeduped) { TEST_F(ConversionCandidatesTest, WordNotInAnyDictReturnsEmpty) { const ConverterPtr converter = MakeConverter({MakeDict({{utf8("里"), {utf8("裏")}}})}); - EXPECT_TRUE(converter->GetConversionCandidates(utf8("国")).empty()); + EXPECT_TRUE(GetAllConversions(*converter, utf8("国")).empty()); } // Only a prefix (not the whole word) matches: no exact match means "not found", @@ -101,7 +102,7 @@ TEST_F(ConversionCandidatesTest, WordNotInAnyDictReturnsEmpty) { TEST_F(ConversionCandidatesTest, PrefixOnlyMatchReturnsEmpty) { const ConverterPtr converter = MakeConverter({MakeDict({{utf8("里"), {utf8("裏")}}})}); - EXPECT_TRUE(converter->GetConversionCandidates(utf8("里面")).empty()); + EXPECT_TRUE(GetAllConversions(*converter, utf8("里面")).empty()); } // An exact match on a longer word still carries an unconvertible tail through a @@ -112,7 +113,7 @@ TEST_F(ConversionCandidatesTest, PartialConversionInLaterDict) { MakeDict({{utf8("裏"), {utf8("裡")}}}), }); const std::vector candidates = - converter->GetConversionCandidates(utf8("里面")); + GetAllConversions(*converter, utf8("里面")); // Stage 1: 里面 -> 裏面. Stage 2: 裏 -> 裡 (prefix), 面 copied -> 裡面. EXPECT_EQ((std::vector{utf8("裡面")}), candidates); } @@ -126,12 +127,53 @@ TEST_F(ConversionCandidatesTest, PipelineConverterYieldsEmpty) { EXPECT_TRUE(GetAllConversions(*pipeline, utf8("里")).empty()); } -// The free function and the member wrapper agree. -TEST_F(ConversionCandidatesTest, MemberDelegatesToFreeFunction) { +// A key-only entry (no values) converts the word to itself, matching +// Convert(), instead of silently dropping the branch. +TEST_F(ConversionCandidatesTest, KeyOnlyEntryPassesWordThrough) { const ConverterPtr converter = + MakeConverter({MakeDict({{utf8("通道"), {}}})}); + const std::vector candidates = + GetAllConversions(*converter, utf8("通道")); + EXPECT_EQ((std::vector{utf8("通道")}), candidates); +} + +// Input ending in a truncated UTF-8 sequence must not read out of bounds; the +// longest-prefix fallback delegates to Conversion::Convert, which clamps. +TEST_F(ConversionCandidatesTest, TruncatedUtf8InputIsSafe) { + const ConverterPtr converter = + MakeConverter({MakeDict({{utf8("里"), {utf8("裏")}}})}); + // First two bytes of 里 (0xE9 0x87 0x8C): an incomplete 3-byte sequence. + EXPECT_TRUE(GetAllConversions(*converter, "\xE9\x87").empty()); + // Truncated tail after a convertible prefix: no exact match anywhere. + EXPECT_TRUE(GetAllConversions(*converter, utf8("里") + "\xE9\x87").empty()); +} + +// An ideographic description sequence flowing through a later dictionary's +// longest-prefix fallback stays atomic, matching Converter::Convert. +TEST_F(ConversionCandidatesTest, IdsKeptAtomicInLaterDict) { + const ConverterPtr converter = MakeConverter({ + MakeDict({{utf8("里字"), {utf8("⿱艹里")}}}), + MakeDict({{utf8("里"), {utf8("裏")}}}), + }); + const std::vector candidates = + GetAllConversions(*converter, utf8("里字")); + // Stage 2 must not convert the 里 inside the IDS ⿱艹里. + EXPECT_EQ((std::vector{utf8("⿱艹里")}), candidates); +} + +// A config-based converter's normalization pre-pass is applied before the main +// chain, mirroring Convert(): a CJK Compatibility Ideograph key is normalized +// first and then expanded by the main chain. +TEST_F(ConversionCandidatesTest, NormalizationPrePassApplied) { + // \xEF\xA7\xA9 is U+F9E9, the compatibility ideograph for 里. + const ConverterPtr norm = + MakeConverter({MakeDict({{"\xEF\xA7\xA9", {utf8("里")}}})}); + const ConverterPtr main = MakeConverter({MakeDict({{utf8("里"), {utf8("里"), utf8("裏")}}})}); - EXPECT_EQ(GetAllConversions(*converter, utf8("里")), - converter->GetConversionCandidates(utf8("里"))); + const ConverterPtr converter(new ConfigBasedConverter(norm, main)); + const std::vector candidates = + GetAllConversions(*converter, "\xEF\xA7\xA9"); + EXPECT_EQ((std::vector{utf8("里"), utf8("裏")}), candidates); } } // namespace opencc diff --git a/src/Converter.hpp b/src/Converter.hpp index be4174994..df3567a41 100644 --- a/src/Converter.hpp +++ b/src/Converter.hpp @@ -21,7 +21,6 @@ #include #include #include -#include #include "Common.hpp" #include "ConversionInspection.hpp" @@ -75,25 +74,6 @@ class OPENCC_EXPORT Converter { * converter has no single chain (e.g. @c PipelineConverter). */ virtual ConversionChainPtr GetConversionChain() const = 0; - -#ifdef OPENCC_ENABLE_UNSTABLE_API - /** - * Enumerates every candidate form of a single @p word produced by this - * converter's conversion chain (e.g. @c s2t expands @c 里 to @c 里 and - * @c 裏). Unlike Convert(), which segments the whole input and returns one - * string, this returns all branch values for a single word and is intended - * for input-method and tooling use. Delegates to GetAllConversions(); see - * ConversionCandidates.hpp for exact semantics. - * - * @warning Experimental, unstable API guarded by @c OPENCC_ENABLE_UNSTABLE_API - * and deliberately absent from released headers. It is @b not part of the - * @c OPENCC_ABI_VERSION contract. This is currently a non-virtual member so - * that enabling the macro does not alter the vtable; when the API is - * promoted at the next ABI bump the guard is removed and this becomes a - * @c virtual method. - */ - std::vector GetConversionCandidates(std::string_view word) const; -#endif }; class OPENCC_EXPORT ConverterStream {