Stop using const pointer to initialize predictor and rewriter.

We use reference as pointer can be nullable.

PiperOrigin-RevId: 727755551
This commit is contained in:
Taku Kudo
2025-02-17 09:04:58 +00:00
committed by Hiroyuki Komatsu
parent 080eddd316
commit 55f00ee1df
13 changed files with 68 additions and 71 deletions

View File

@ -133,7 +133,7 @@ Converter::Converter(
reverse_converter_(*immutable_converter_),
general_noun_id_(pos_matcher_.GetGeneralNounId()) {
DCHECK(immutable_converter_);
predictor_ = predictor_factory(*modules_, this, immutable_converter_.get());
predictor_ = predictor_factory(*modules_, *this, *immutable_converter_);
rewriter_ = rewriter_factory(*modules_);
DCHECK(predictor_);
DCHECK(rewriter_);

View File

@ -63,8 +63,8 @@ class Converter final : public ConverterInterface {
using PredictorFactory =
std::function<std::unique_ptr<prediction::PredictorInterface>(
const engine::Modules &modules, const ConverterInterface *converter,
const ImmutableConverterInterface *immutable_converter)>;
const engine::Modules &modules, const ConverterInterface &converter,
const ImmutableConverterInterface &immutable_converter)>;
using RewriterFactory = std::function<std::unique_ptr<RewriterInterface>(
const engine::Modules &modules)>;

View File

@ -257,15 +257,15 @@ class ConverterTest : public testing::TestWithTempUserProfile {
// mozc/engine/engine.cc for details. Caller should manage the ownership.
std::unique_ptr<PredictorInterface> CreatePredictor(
const engine::Modules &modules, PredictorType predictor_type,
const ConverterInterface *converter,
const ImmutableConverterInterface *immutable_converter) {
const ConverterInterface &converter,
const ImmutableConverterInterface &immutable_converter) {
if (predictor_type == STUB_PREDICTOR) {
return std::make_unique<StubPredictor>();
}
std::function<std::unique_ptr<PredictorInterface>(
std::unique_ptr<PredictorInterface>,
std::unique_ptr<PredictorInterface>, const ConverterInterface *)>
std::unique_ptr<PredictorInterface>, const ConverterInterface &)>
predictor_factory = nullptr;
bool enable_content_word_learning = false;
@ -313,8 +313,8 @@ class ConverterTest : public testing::TestWithTempUserProfile {
[&](const engine::Modules &modules) {
return std::make_unique<ImmutableConverter>(modules);
},
[&](const engine::Modules &modules, const ConverterInterface *converter,
const ImmutableConverterInterface *immutable_converter) {
[&](const engine::Modules &modules, const ConverterInterface &converter,
const ImmutableConverterInterface &immutable_converter) {
return CreatePredictor(modules, predictor_type, converter,
immutable_converter);
},
@ -1071,8 +1071,8 @@ TEST_F(ConverterTest, VariantExpansionForSuggestion) {
[&](const engine::Modules &modules) {
return std::make_unique<ImmutableConverter>(modules);
},
[](const engine::Modules &modules, const ConverterInterface *converter,
const ImmutableConverterInterface *immutable_converter) {
[](const engine::Modules &modules, const ConverterInterface &converter,
const ImmutableConverterInterface &immutable_converter) {
return DefaultPredictor::CreateDefaultPredictor(
std::make_unique<DictionaryPredictor>(modules, converter,
immutable_converter),
@ -1763,8 +1763,8 @@ TEST_F(ConverterTest, RevertConversion) {
return std::make_unique<ImmutableConverter>(modules);
},
[&mock_predictor](
const engine::Modules &modules, const ConverterInterface *converter,
const ImmutableConverterInterface *immutable_converter) {
const engine::Modules &modules, const ConverterInterface &converter,
const ImmutableConverterInterface &immutable_converter) {
return std::move(mock_predictor);
},
[&mock_rewriter](const engine::Modules &modules) {

View File

@ -116,8 +116,8 @@ absl::Status Engine::Init(std::unique_ptr<engine::Modules> modules,
auto predictor_factory =
[is_mobile](const engine::Modules &modules,
const ConverterInterface *converter,
const ImmutableConverterInterface *immutable_converter) {
const ConverterInterface &converter,
const ImmutableConverterInterface &immutable_converter) {
auto dictionary_predictor =
std::make_unique<prediction::DictionaryPredictor>(
modules, converter, immutable_converter);

View File

@ -520,8 +520,8 @@ class DictionaryPredictionAggregator::HandwritingLookupCallback
};
DictionaryPredictionAggregator::DictionaryPredictionAggregator(
const engine::Modules &modules, const ConverterInterface *converter,
const ImmutableConverterInterface *immutable_converter)
const engine::Modules &modules, const ConverterInterface &converter,
const ImmutableConverterInterface &immutable_converter)
: modules_(modules),
converter_(converter),
immutable_converter_(immutable_converter),
@ -838,7 +838,7 @@ bool DictionaryPredictionAggregator::PushBackTopConversionResult(
.SetConversionRequest(request)
.SetOptions(std::move(options))
.Build();
if (!converter_->StartConversion(tmp_request, &tmp_segments)) {
if (!converter_.StartConversion(tmp_request, &tmp_segments)) {
return false;
}
@ -891,8 +891,6 @@ void DictionaryPredictionAggregator::AggregateRealtimeConversion(
const ConversionRequest &request, size_t realtime_candidates_size,
bool insert_realtime_top_from_actual_converter, const Segments &segments,
std::vector<Result> *results) const {
DCHECK(converter_);
DCHECK(immutable_converter_);
DCHECK(results);
if (realtime_candidates_size == 0) {
return;
@ -915,8 +913,8 @@ void DictionaryPredictionAggregator::AggregateRealtimeConversion(
realtime_candidates_size);
Segments tmp_segments = GetSegmentsForRealtimeCandidatesGeneration(segments);
if (!immutable_converter_->ConvertForRequest(request_for_realtime,
&tmp_segments) ||
if (!immutable_converter_.ConvertForRequest(request_for_realtime,
&tmp_segments) ||
tmp_segments.conversion_segments_size() == 0 ||
tmp_segments.conversion_segment(0).candidates_size() == 0) {
LOG(WARNING) << "Convert failed";
@ -1019,8 +1017,8 @@ DictionaryPredictionAggregator::GenerateQueryForHandwriting(
.SetConversionRequest(request)
.SetRequestType(ConversionRequest::REVERSE_CONVERSION)
.Build();
if (!immutable_converter_->ConvertForRequest(request_for_realtime,
&tmp_segments) ||
if (!immutable_converter_.ConvertForRequest(request_for_realtime,
&tmp_segments) ||
tmp_segments.conversion_segments_size() == 0 ||
tmp_segments.conversion_segment(0).candidates_size() == 0) {
LOG(WARNING) << "Reverse conversion failed";

View File

@ -64,8 +64,8 @@ class DictionaryPredictionAggregator : public PredictionAggregatorInterface {
~DictionaryPredictionAggregator() override = default;
DictionaryPredictionAggregator(
const engine::Modules &modules, const ConverterInterface *converter,
const ImmutableConverterInterface *immutable_converter);
const engine::Modules &modules, const ConverterInterface &converter,
const ImmutableConverterInterface &immutable_converter);
std::vector<Result> AggregateResults(const ConversionRequest &request,
const Segments &segments) const override;
@ -286,8 +286,8 @@ class DictionaryPredictionAggregator : public PredictionAggregatorInterface {
friend class DictionaryPredictionAggregatorTestPeer;
const engine::Modules &modules_;
const ConverterInterface *converter_;
const ImmutableConverterInterface *immutable_converter_;
const ConverterInterface &converter_;
const ImmutableConverterInterface &immutable_converter_;
const dictionary::DictionaryInterface *dictionary_;
const dictionary::DictionaryInterface *suffix_dictionary_;
const uint16_t counter_suffix_word_id_;

View File

@ -84,8 +84,8 @@ using ::mozc::composer::TypeCorrectedQuery;
class DictionaryPredictionAggregatorTestPeer {
public:
DictionaryPredictionAggregatorTestPeer(
const ConverterInterface *converter,
const ImmutableConverterInterface *immutable_converter,
const ConverterInterface &converter,
const ImmutableConverterInterface &immutable_converter,
const engine::Modules &modules)
: aggregator_(modules, converter, immutable_converter) {}
virtual ~DictionaryPredictionAggregatorTestPeer() = default;
@ -408,7 +408,7 @@ class MockDataAndAggregator {
CHECK_NE(modules_.GetSuffixDictionary(), nullptr);
aggregator_ = std::make_unique<DictionaryPredictionAggregatorTestPeer>(
&converter_, &mock_immutable_converter_, modules_);
converter_, mock_immutable_converter_, modules_);
}
MockDictionary *mutable_dictionary() { return mock_dictionary_; }

View File

@ -204,8 +204,8 @@ void MaybeFixRealtimeTopCost(absl::string_view input_key,
} // namespace
DictionaryPredictor::DictionaryPredictor(
const engine::Modules &modules, const ConverterInterface *converter,
const ImmutableConverterInterface *immutable_converter)
const engine::Modules &modules, const ConverterInterface &converter,
const ImmutableConverterInterface &immutable_converter)
: DictionaryPredictor(
"DictionaryPredictor", modules,
std::make_unique<prediction::DictionaryPredictionAggregator>(
@ -215,7 +215,7 @@ DictionaryPredictor::DictionaryPredictor(
DictionaryPredictor::DictionaryPredictor(
std::string predictor_name, const engine::Modules &modules,
std::unique_ptr<const prediction::PredictionAggregatorInterface> aggregator,
const ImmutableConverterInterface *immutable_converter)
const ImmutableConverterInterface &immutable_converter)
: aggregator_(std::move(aggregator)),
immutable_converter_(immutable_converter),
connector_(modules.GetConnector()),
@ -1140,7 +1140,7 @@ bool DictionaryPredictor::IsAggressiveSuggestion(size_t query_len,
int DictionaryPredictor::CalculatePrefixPenalty(
const ConversionRequest &request, const absl::string_view input_key,
const Result &result,
const ImmutableConverterInterface *immutable_converter,
const ImmutableConverterInterface &immutable_converter,
absl::flat_hash_map<PrefixPenaltyKey, int> *cache) const {
if (input_key == result.key) {
LOG(WARNING) << "Invalid prefix key: " << result.key;
@ -1173,7 +1173,7 @@ int DictionaryPredictor::CalculatePrefixPenalty(
.SetOptions(std::move(options))
.Build();
if (immutable_converter->ConvertForRequest(req, &tmp_segments) &&
if (immutable_converter.ConvertForRequest(req, &tmp_segments) &&
tmp_segments.segment(0).candidates_size() > 0) {
const Segment::Candidate &top_candidate =
tmp_segments.segment(0).candidate(0);

View File

@ -80,8 +80,8 @@ class DictionaryPredictor : public PredictorInterface {
// Initializes a predictor with given references to submodules. Note that
// pointers are not owned by the class and to be deleted by the caller.
DictionaryPredictor(const engine::Modules &modules,
const ConverterInterface *converter,
const ImmutableConverterInterface *immutable_converter);
const ConverterInterface &converter,
const ImmutableConverterInterface &immutable_converter);
DictionaryPredictor(const DictionaryPredictor &) = delete;
DictionaryPredictor &operator=(const DictionaryPredictor &) = delete;
@ -140,7 +140,7 @@ class DictionaryPredictor : public PredictorInterface {
std::string predictor_name, const engine::Modules &modules,
std::unique_ptr<const prediction::PredictionAggregatorInterface>
aggregator,
const ImmutableConverterInterface *immutable_converter);
const ImmutableConverterInterface &immutable_converter);
// It is better to pass the rvalue of `results` if the
// caller doesn't use the results after calling this method.
@ -261,7 +261,7 @@ class DictionaryPredictor : public PredictorInterface {
int CalculatePrefixPenalty(
const ConversionRequest &request, absl::string_view input_key,
const Result &result,
const ImmutableConverterInterface *immutable_converter,
const ImmutableConverterInterface &immutable_converter,
absl::flat_hash_map<PrefixPenaltyKey, int> *cache) const;
// Populates typing corrected results to `results`.
@ -299,7 +299,7 @@ class DictionaryPredictor : public PredictorInterface {
mutable std::shared_ptr<Result> prev_top_result_;
mutable std::atomic<int32_t> prev_top_key_length_ = 0;
const ImmutableConverterInterface *immutable_converter_;
const ImmutableConverterInterface &immutable_converter_;
const Connector &connector_;
const Segmenter *segmenter_;
const SuggestionFilter &suggestion_filter_;

View File

@ -80,7 +80,7 @@ class DictionaryPredictorTestPeer {
const engine::Modules &modules,
std::unique_ptr<const prediction::PredictionAggregatorInterface>
aggregator,
const ImmutableConverterInterface *immutable_converter)
const ImmutableConverterInterface &immutable_converter)
: predictor_("DictionaryPredictorForTest", modules, std::move(aggregator),
immutable_converter) {}
@ -307,7 +307,7 @@ class MockDataAndPredictor {
predictor_ = std::make_unique<DictionaryPredictorTestPeer>(
modules_, absl::WrapUnique(mock_aggregator_),
&mock_immutable_converter_);
mock_immutable_converter_);
}
MockImmutableConverter *mutable_immutable_converter() {

View File

@ -107,13 +107,12 @@ std::optional<std::string> GetReading(const ConverterInterface &converter,
BasePredictor::BasePredictor(
std::unique_ptr<PredictorInterface> dictionary_predictor,
std::unique_ptr<PredictorInterface> user_history_predictor,
const ConverterInterface *converter)
const ConverterInterface &converter)
: dictionary_predictor_(std::move(dictionary_predictor)),
user_history_predictor_(std::move(user_history_predictor)),
converter_{converter} {
converter_(converter) {
DCHECK(dictionary_predictor_);
DCHECK(user_history_predictor_);
DCHECK(converter_);
}
void BasePredictor::Finish(const ConversionRequest &request,
@ -174,7 +173,7 @@ void BasePredictor::PopulateReadingOfCommittedCandidateIfMissing(
if (!cand->key.empty() || cand->value.empty()) return;
if (cand->content_value == cand->value) {
if (std::optional<std::string> key = GetReading(*converter_, cand->value);
if (std::optional<std::string> key = GetReading(converter_, cand->value);
key.has_value()) {
cand->key = *key;
cand->content_key = *std::move(key);
@ -193,7 +192,7 @@ void BasePredictor::PopulateReadingOfCommittedCandidateIfMissing(
return;
}
if (std::optional<std::string> content_key =
GetReading(*converter_, cand->content_value);
GetReading(converter_, cand->content_value);
content_key.has_value()) {
cand->key = absl::StrCat(*content_key, functional_value);
cand->content_key = *std::move(content_key);
@ -204,7 +203,7 @@ void BasePredictor::PopulateReadingOfCommittedCandidateIfMissing(
std::unique_ptr<PredictorInterface> DefaultPredictor::CreateDefaultPredictor(
std::unique_ptr<PredictorInterface> dictionary_predictor,
std::unique_ptr<PredictorInterface> user_history_predictor,
const ConverterInterface *converter) {
const ConverterInterface &converter) {
return std::make_unique<DefaultPredictor>(std::move(dictionary_predictor),
std::move(user_history_predictor),
converter);
@ -213,7 +212,7 @@ std::unique_ptr<PredictorInterface> DefaultPredictor::CreateDefaultPredictor(
DefaultPredictor::DefaultPredictor(
std::unique_ptr<PredictorInterface> dictionary_predictor,
std::unique_ptr<PredictorInterface> user_history_predictor,
const ConverterInterface *converter)
const ConverterInterface &converter)
: BasePredictor(std::move(dictionary_predictor),
std::move(user_history_predictor), converter),
predictor_name_("DefaultPredictor") {}
@ -271,7 +270,7 @@ bool DefaultPredictor::PredictForRequest(const ConversionRequest &request,
std::unique_ptr<PredictorInterface> MobilePredictor::CreateMobilePredictor(
std::unique_ptr<PredictorInterface> dictionary_predictor,
std::unique_ptr<PredictorInterface> user_history_predictor,
const ConverterInterface *converter) {
const ConverterInterface &converter) {
return std::make_unique<MobilePredictor>(std::move(dictionary_predictor),
std::move(user_history_predictor),
converter);
@ -280,7 +279,7 @@ std::unique_ptr<PredictorInterface> MobilePredictor::CreateMobilePredictor(
MobilePredictor::MobilePredictor(
std::unique_ptr<PredictorInterface> dictionary_predictor,
std::unique_ptr<PredictorInterface> user_history_predictor,
const ConverterInterface *converter)
const ConverterInterface &converter)
: BasePredictor(std::move(dictionary_predictor),
std::move(user_history_predictor), converter),
predictor_name_("MobilePredictor") {}

View File

@ -47,7 +47,7 @@ class BasePredictor : public PredictorInterface {
// Initializes the composite of predictor with given sub-predictors.
BasePredictor(std::unique_ptr<PredictorInterface> dictionary_predictor,
std::unique_ptr<PredictorInterface> user_history_predictor,
const ConverterInterface *converter);
const ConverterInterface &converter);
// Hook(s) for all mutable operations.
void Finish(const ConversionRequest &request, Segments *segments) override;
@ -86,7 +86,7 @@ class BasePredictor : public PredictorInterface {
private:
void PopulateReadingOfCommittedCandidateIfMissing(Segments *segments) const;
const ConverterInterface *converter_;
const ConverterInterface &converter_;
};
// TODO(team): The name should be DesktopPredictor
@ -95,11 +95,11 @@ class DefaultPredictor : public BasePredictor {
static std::unique_ptr<PredictorInterface> CreateDefaultPredictor(
std::unique_ptr<PredictorInterface> dictionary_predictor,
std::unique_ptr<PredictorInterface> user_history_predictor,
const ConverterInterface *converter);
const ConverterInterface &converter);
DefaultPredictor(std::unique_ptr<PredictorInterface> dictionary_predictor,
std::unique_ptr<PredictorInterface> user_history_predictor,
const ConverterInterface *converter);
const ConverterInterface &converter);
~DefaultPredictor() override;
ABSL_MUST_USE_RESULT bool PredictForRequest(
@ -118,11 +118,11 @@ class MobilePredictor : public BasePredictor {
static std::unique_ptr<PredictorInterface> CreateMobilePredictor(
std::unique_ptr<PredictorInterface> dictionary_predictor,
std::unique_ptr<PredictorInterface> user_history_predictor,
const ConverterInterface *converter);
const ConverterInterface &converter);
MobilePredictor(std::unique_ptr<PredictorInterface> dictionary_predictor,
std::unique_ptr<PredictorInterface> user_history_predictor,
const ConverterInterface *converter);
const ConverterInterface &converter);
~MobilePredictor() override;
ABSL_MUST_USE_RESULT bool PredictForRequest(

View File

@ -180,7 +180,7 @@ TEST_F(MobilePredictorTest, CallPredictorsForMobileSuggestion) {
MockConverter converter;
auto predictor = std::make_unique<MobilePredictor>(
std::make_unique<CheckCandSizeDictionaryPredictor>(20),
std::make_unique<CheckCandSizeUserHistoryPredictor>(3, 4), &converter);
std::make_unique<CheckCandSizeUserHistoryPredictor>(3, 4), converter);
Segments segments;
{
Segment *segment = segments.add_segment();
@ -196,7 +196,7 @@ TEST_F(MobilePredictorTest, CallPredictorsForMobilePartialSuggestion) {
auto predictor = std::make_unique<MobilePredictor>(
std::make_unique<CheckCandSizeDictionaryPredictor>(20),
// We don't call history predictor
std::make_unique<CheckCandSizeUserHistoryPredictor>(-1, -1), &converter);
std::make_unique<CheckCandSizeUserHistoryPredictor>(-1, -1), converter);
Segments segments;
{
Segment *segment = segments.add_segment();
@ -211,7 +211,7 @@ TEST_F(MobilePredictorTest, CallPredictorsForMobilePrediction) {
MockConverter converter;
auto predictor = std::make_unique<MobilePredictor>(
std::make_unique<CheckCandSizeDictionaryPredictor>(200),
std::make_unique<CheckCandSizeUserHistoryPredictor>(3, 4), &converter);
std::make_unique<CheckCandSizeUserHistoryPredictor>(3, 4), converter);
Segments segments;
{
Segment *segment = segments.add_segment();
@ -229,7 +229,7 @@ TEST_F(MobilePredictorTest, CallPredictorsForMobilePartialPrediction) {
CHECK_OK(modules.Init(std::make_unique<testing::MockDataManager>()));
auto predictor = std::make_unique<MobilePredictor>(
std::make_unique<CheckCandSizeDictionaryPredictor>(200),
std::make_unique<UserHistoryPredictor>(modules, true), &converter);
std::make_unique<UserHistoryPredictor>(modules, true), converter);
Segments segments;
{
Segment *segment = segments.add_segment();
@ -252,7 +252,7 @@ TEST_F(MobilePredictorTest, CallPredictForRequestMobile) {
MockConverter converter;
auto predictor = std::make_unique<MobilePredictor>(
std::move(predictor1), std::move(predictor2), &converter);
std::move(predictor1), std::move(predictor2), converter);
Segments segments;
{
Segment *segment = segments.add_segment();
@ -290,7 +290,7 @@ TEST_F(PredictorTest, AllPredictorsReturnTrue) {
MockConverter converter;
auto predictor = std::make_unique<DefaultPredictor>(
std::make_unique<NullPredictor>(true),
std::make_unique<NullPredictor>(true), &converter);
std::make_unique<NullPredictor>(true), converter);
Segments segments;
{
Segment *segment = segments.add_segment();
@ -304,7 +304,7 @@ TEST_F(PredictorTest, MixedReturnValue) {
MockConverter converter;
auto predictor = std::make_unique<DefaultPredictor>(
std::make_unique<NullPredictor>(true),
std::make_unique<NullPredictor>(false), &converter);
std::make_unique<NullPredictor>(false), converter);
Segments segments;
{
Segment *segment = segments.add_segment();
@ -319,7 +319,7 @@ TEST_F(PredictorTest, AllPredictorsReturnFalse) {
MockConverter converter;
auto predictor = std::make_unique<DefaultPredictor>(
std::make_unique<NullPredictor>(false),
std::make_unique<NullPredictor>(false), &converter);
std::make_unique<NullPredictor>(false), converter);
Segments segments;
{
Segment *segment = segments.add_segment();
@ -338,7 +338,7 @@ TEST_F(PredictorTest, CallPredictorsForSuggestion) {
std::make_unique<CheckCandSizeDictionaryPredictor>(suggestions_size),
std::make_unique<CheckCandSizeUserHistoryPredictor>(suggestions_size,
suggestions_size),
&converter);
converter);
Segments segments;
{
Segment *segment = segments.add_segment();
@ -356,7 +356,7 @@ TEST_F(PredictorTest, CallPredictorsForPrediction) {
std::make_unique<CheckCandSizeDictionaryPredictor>(kPredictionSize),
std::make_unique<CheckCandSizeUserHistoryPredictor>(kPredictionSize,
kPredictionSize),
&converter);
converter);
Segments segments;
{
Segment *segment = segments.add_segment();
@ -379,7 +379,7 @@ TEST_F(PredictorTest, CallPredictForRequest) {
MockConverter converter;
auto predictor = std::make_unique<DefaultPredictor>(
std::move(predictor1), std::move(predictor2), &converter);
std::move(predictor1), std::move(predictor2), converter);
Segments segments;
{
Segment *segment = segments.add_segment();
@ -397,7 +397,7 @@ TEST_F(PredictorTest, DisableAllSuggestion) {
const auto *pred2 = predictor2.get(); // Keep the reference
MockConverter converter;
auto predictor = std::make_unique<DefaultPredictor>(
std::move(predictor1), std::move(predictor2), &converter);
std::move(predictor1), std::move(predictor2), converter);
Segments segments;
{
Segment *segment = segments.add_segment();
@ -423,7 +423,7 @@ TEST_F(PredictorTest, PopulateReadingOfCommittedCandidateIfMissing) {
MockConverter converter;
auto predictor = std::make_unique<MobilePredictor>(
std::make_unique<NullPredictor>(true),
std::make_unique<NullPredictor>(true), &converter);
std::make_unique<NullPredictor>(true), converter);
// Mock reverse conversion adds reading "とうきょう".
EXPECT_CALL(converter, StartReverseConversion(_, StrEq("東京")))
@ -554,7 +554,7 @@ TEST_F(MobilePredictorTest, FillPos) {
MockConverter converter;
auto predictor = std::make_unique<MobilePredictor>(
std::move(mock_dictionary_predictor), std::move(mock_history_predictor),
&converter);
converter);
const ConversionRequest convreq =
CreateConversionRequest(ConversionRequest::SUGGESTION);