Skip to content

Commit

Permalink
Remove the segments_size param of ConverterInterface::ResizeSegment().
Browse files Browse the repository at this point in the history 
* Also renamed one of overloaded ResizeSegment functions to ResizeSegments.

#codehealth

PiperOrigin-RevId: 704137708
  • Loading branch information
@hiroyuki-komatsu
hiroyuki-komatsu committed Dec 9, 2024
1 parent 8354f11 commit bc3c413
Show file tree
Hide file tree
Showing 9 changed files with 59 additions and 71 deletions.
9 changes: 4 additions & 5 deletions src/converter/converter.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -525,11 +525,10 @@ bool Converter::ResizeSegment(Segments *segments,
return true;
}

bool Converter::ResizeSegment(Segments *segments,
const ConversionRequest &request,
size_t start_segment_index,
size_t unused_segments_size,
absl::Span<const uint8_t> new_size_array) const {
bool Converter::ResizeSegments(Segments *segments,
const ConversionRequest &request,
size_t start_segment_index,
absl::Span<const uint8_t> new_size_array) const {
if (request.request_type() != ConversionRequest::CONVERSION) {
return false;
}
Expand Down
4 changes: 2 additions & 2 deletions src/converter/converter.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -106,9 +106,9 @@ class Converter final : public ConverterInterface {
const ConversionRequest &request,
size_t segment_index,
int offset_length) const override;
ABSL_MUST_USE_RESULT bool ResizeSegment(
ABSL_MUST_USE_RESULT bool ResizeSegments(
Segments *segments, const ConversionRequest &request,
size_t start_segment_index, size_t unused_segments_size,
size_t start_segment_index,
absl::Span<const uint8_t> new_size_array) const override;

// Execute ImmutableConverter, Rewriters, SuppressionDictionary.
Expand Down
4 changes: 2 additions & 2 deletions src/converter/converter_interface.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -144,9 +144,9 @@ class ConverterInterface {

// Resize [start_segment_index, start_segment_index + segment_size]
// segments with the new size in new_size_array.
ABSL_MUST_USE_RESULT virtual bool ResizeSegment(
ABSL_MUST_USE_RESULT virtual bool ResizeSegments(
Segments *segments, const ConversionRequest &request,
size_t start_segment_index, size_t segments_size,
size_t start_segment_index,
absl::Span<const uint8_t> new_size_array) const = 0;

protected:
Expand Down
13 changes: 9 additions & 4 deletions src/converter/converter_main.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -388,15 +388,20 @@ bool ExecCommand(const ConverterInterface &converter, const std::string &line,
return converter.ResizeSegment(segments, conversion_request,
NumberUtil::SimpleAtoi(fields[1]),
NumberUtil::SimpleAtoi(fields[2]));
} else if (fields.size() > 3) {
}
} else if (func == "resizesegments" || func == "resizes") {
options.request_type = ConversionRequest::CONVERSION;
const ConversionRequest conversion_request = ConversionRequest(
composer, request, context, *config, std::move(options));
if (fields.size() > 3) {
std::vector<uint8_t> new_arrays;
for (size_t i = 3; i < fields.size(); ++i) {
for (size_t i = 2; i < fields.size(); ++i) {
new_arrays.push_back(
static_cast<uint8_t>(NumberUtil::SimpleAtoi(fields[i])));
}
return converter.ResizeSegment(
return converter.ResizeSegments(
segments, conversion_request, NumberUtil::SimpleAtoi(fields[1]),
NumberUtil::SimpleAtoi(fields[2]), new_arrays);
new_arrays);
}
} else if (func == "disableuserhistory") {
config->set_history_learning_level(config::Config::NO_HISTORY);
Expand Down
4 changes: 2 additions & 2 deletions src/converter/converter_mock.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -84,9 +84,9 @@ class StrictMockConverter : public ConverterInterface {
(Segments * segments, const ConversionRequest &request,
size_t segment_index, int offset_length),
(const, override));
MOCK_METHOD(bool, ResizeSegment,
MOCK_METHOD(bool, ResizeSegments,
(Segments * segments, const ConversionRequest &request,
size_t start_segment_index, size_t segments_size,
size_t start_segment_index,
absl::Span<const uint8_t> new_size_array),
(const, override));
};
Expand Down
42 changes: 17 additions & 25 deletions src/converter/converter_test.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2062,7 +2062,7 @@ TEST_F(ConverterTest, ResizeSegmentWithOffset) {
}
}

TEST_F(ConverterTest, ResizeSegmentWithArray) {
TEST_F(ConverterTest, ResizeSegmentsWithArray) {
constexpr Segment::SegmentType kFixedBoundary = Segment::FIXED_BOUNDARY;
constexpr Segment::SegmentType kFree = Segment::FREE;

Expand All @@ -2076,11 +2076,10 @@ TEST_F(ConverterTest, ResizeSegmentWithArray) {
AddSegment("あいうえ", kFree, segments);
const ConversionRequest convreq;
const int start_segment_index = 0;
const int segments_size = 1;
const std::array<uint8_t, 2> size_array = {3, 1};
EXPECT_EQ(segments.conversion_segments_size(), 1);
EXPECT_TRUE(converter->ResizeSegment(
&segments, convreq, start_segment_index, segments_size, size_array));
EXPECT_TRUE(converter->ResizeSegments(&segments, convreq,
start_segment_index, size_array));
ASSERT_EQ(segments.conversion_segments_size(), 2);
EXPECT_EQ(segments.conversion_segment(0).key(), "あいう");
EXPECT_EQ(segments.conversion_segment(1).key(), "え");
Expand All @@ -2100,11 +2099,10 @@ TEST_F(ConverterTest, ResizeSegmentWithArray) {
AddSegment("あいうえ", Segment::FREE, segments);
const ConversionRequest convreq;
const int start_segment_index = 0;
const int segments_size = 1;
const std::array<uint8_t, 2> size_array = {3, 1};
EXPECT_EQ(segments.conversion_segments_size(), 1);
EXPECT_TRUE(converter->ResizeSegment(
&segments, convreq, start_segment_index, segments_size, size_array));
EXPECT_TRUE(converter->ResizeSegments(&segments, convreq,
start_segment_index, size_array));
ASSERT_EQ(segments.conversion_segments_size(), 2);
EXPECT_EQ(segments.conversion_segment(0).key(), "あいう");
EXPECT_EQ(segments.conversion_segment(1).key(), "え");
Expand All @@ -2119,11 +2117,10 @@ TEST_F(ConverterTest, ResizeSegmentWithArray) {
AddSegment("あいうえ", Segment::FREE, segments);
const ConversionRequest convreq;
const int start_segment_index = 0;
const int segments_size = 1;
const std::array<uint8_t, 8> size_array = {3, 1, 0, 0, 0, 0, 0, 0};
EXPECT_EQ(segments.conversion_segments_size(), 1);
EXPECT_TRUE(converter->ResizeSegment(
&segments, convreq, start_segment_index, segments_size, size_array));
EXPECT_TRUE(converter->ResizeSegments(&segments, convreq,
start_segment_index, size_array));
ASSERT_EQ(segments.conversion_segments_size(), 2);
EXPECT_EQ(segments.conversion_segment(0).key(), "あいう");
EXPECT_EQ(segments.conversion_segment(1).key(), "え");
Expand All @@ -2137,11 +2134,10 @@ TEST_F(ConverterTest, ResizeSegmentWithArray) {
AddSegment("あいうえおかきくけ", Segment::FREE, segments);
const ConversionRequest convreq;
const int start_segment_index = 0;
const int segments_size = 1;
const std::array<uint8_t, 5> size_array = {2, 2, 1, 2, 2};
EXPECT_EQ(segments.conversion_segments_size(), 1);
EXPECT_TRUE(converter->ResizeSegment(
&segments, convreq, start_segment_index, segments_size, size_array));
EXPECT_TRUE(converter->ResizeSegments(&segments, convreq,
start_segment_index, size_array));
ASSERT_EQ(segments.conversion_segments_size(), 5);
EXPECT_EQ(segments.conversion_segment(0).key(), "あい");
EXPECT_EQ(segments.conversion_segment(1).key(), "うえ");
Expand All @@ -2165,11 +2161,10 @@ TEST_F(ConverterTest, ResizeSegmentWithArray) {
AddSegment("くけ", Segment::FREE, segments);
const ConversionRequest convreq;
const int start_segment_index = 0;
const int segments_size = 4;
const std::array<uint8_t, 3> size_array = {4, 1, 4};
EXPECT_EQ(segments.conversion_segments_size(), 4);
EXPECT_TRUE(converter->ResizeSegment(
&segments, convreq, start_segment_index, segments_size, size_array));
EXPECT_TRUE(converter->ResizeSegments(&segments, convreq,
start_segment_index, size_array));
ASSERT_EQ(segments.conversion_segments_size(), 3);
EXPECT_EQ(segments.conversion_segment(0).key(), "あいうえ");
EXPECT_EQ(segments.conversion_segment(1).key(), "お");
Expand All @@ -2189,11 +2184,10 @@ TEST_F(ConverterTest, ResizeSegmentWithArray) {
AddSegment("くけ", Segment::FREE, segments);
const ConversionRequest convreq;
const int start_segment_index = 0;
const int segments_size = 2;
const std::array<uint8_t, 2> size_array = {4, 1};
EXPECT_EQ(segments.conversion_segments_size(), 4);
EXPECT_TRUE(converter->ResizeSegment(
&segments, convreq, start_segment_index, segments_size, size_array));
EXPECT_TRUE(converter->ResizeSegments(&segments, convreq,
start_segment_index, size_array));
ASSERT_EQ(segments.conversion_segments_size(), 4);
EXPECT_EQ(segments.conversion_segment(0).key(), "あいうえ");
EXPECT_EQ(segments.conversion_segment(1).key(), "お");
Expand All @@ -2211,11 +2205,10 @@ TEST_F(ConverterTest, ResizeSegmentWithArray) {
AddSegment("あいうえ", Segment::FREE, segments);
const ConversionRequest convreq;
const int start_segment_index = 0;
const int segments_size = 1;
const std::array<uint8_t, 1> size_array = {3};
EXPECT_EQ(segments.conversion_segments_size(), 1);
EXPECT_TRUE(converter->ResizeSegment(
&segments, convreq, start_segment_index, segments_size, size_array));
EXPECT_TRUE(converter->ResizeSegments(&segments, convreq,
start_segment_index, size_array));
ASSERT_EQ(segments.conversion_segments_size(), 2);
EXPECT_EQ(segments.conversion_segment(0).key(), "あいう");
EXPECT_EQ(segments.conversion_segment(1).key(), "え");
Expand All @@ -2235,11 +2228,10 @@ TEST_F(ConverterTest, ResizeSegmentWithArray) {
AddSegment("くけ", Segment::FREE, segments);
const ConversionRequest convreq;
const int start_segment_index = 2;
const int segments_size = 2;
const std::array<uint8_t, 1> size_array = {4};
EXPECT_EQ(segments.conversion_segments_size(), 4);
EXPECT_TRUE(converter->ResizeSegment(
&segments, convreq, start_segment_index, segments_size, size_array));
EXPECT_TRUE(converter->ResizeSegments(&segments, convreq,
start_segment_index, size_array));

// Since {"あいう", "えお"} may be modified too, the segment index for
// "かきくけ" may be different from 2.
Expand Down
6 changes: 3 additions & 3 deletions src/engine/minimal_engine.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -146,9 +146,9 @@ class MinimalConverter : public ConverterInterface {
return true;
}

bool ResizeSegment(Segments *segments, const ConversionRequest &request,
size_t start_segment_index, size_t segments_size,
absl::Span<const uint8_t> new_size_array) const override {
bool ResizeSegments(Segments *segments, const ConversionRequest &request,
size_t start_segment_index,
absl::Span<const uint8_t> new_size_array) const override {
return true;
}
};
Expand Down
4 changes: 2 additions & 2 deletions src/rewriter/user_boundary_history_rewriter.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -312,8 +312,8 @@ bool UserBoundaryHistoryRewriter::Resize(
MOZC_VLOG(2) << "ResizeSegment key: " << key << " segments: [" << seg_idx
<< ", " << seg_size << "] "
<< "resize: [" << absl::StrJoin(updated_array, " ") << "]";
if (parent_converter_->ResizeSegment(&segments, request, seg_idx,
seg_size, updated_array)) {
if (parent_converter_->ResizeSegments(&segments, request, seg_idx,
updated_array)) {
result = true;
} else {
LOG(WARNING) << "ResizeSegment failed for key: " << key;
Expand Down
44 changes: 18 additions & 26 deletions src/rewriter/user_boundary_history_rewriter_test.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -142,10 +142,9 @@ TEST_F(UserBoundaryHistoryRewriterTest, SplitSegmentByHistory) {
// TODO(noriyukit): The current implementation always sets the length array
// size to 8 with padded zeros. Better to set the actual length.
Segments segments = MakeSegments({"たんぽぽ"}, Segment::FREE);
EXPECT_CALL(converter, ResizeSegment(&segments, Ref(convreq),
/*start_segment_index=*/0,
/*segments_size=*/1,
ElementsAre(2, 2, 0, 0, 0, 0, 0, 0)))
EXPECT_CALL(converter, ResizeSegments(&segments, Ref(convreq),
/*start_segment_index=*/0,
ElementsAre(2, 2, 0, 0, 0, 0, 0, 0)))
.WillOnce(Return(true));
EXPECT_TRUE(rewriter.Rewrite(convreq, &segments));
}
Expand Down Expand Up @@ -175,10 +174,9 @@ TEST_F(UserBoundaryHistoryRewriterTest, JoinSegmentsByHistory) {
// TODO(noriyukit): The current implementation always sets the length array
// size to 8 with padded zeros. Better to set the actual length.
Segments segments = MakeSegments({"たん", "ぽぽ"}, Segment::FREE);
EXPECT_CALL(converter, ResizeSegment(&segments, Ref(convreq),
/*start_segment_index=*/0,
/*segments_size=*/2,
ElementsAre(4, 0, 0, 0, 0, 0, 0, 0)))
EXPECT_CALL(converter, ResizeSegments(&segments, Ref(convreq),
/*start_segment_index=*/0,
ElementsAre(4, 0, 0, 0, 0, 0, 0, 0)))
.WillOnce(Return(true));
EXPECT_TRUE(rewriter.Rewrite(convreq, &segments));
}
Expand Down Expand Up @@ -416,13 +414,11 @@ TEST_F(UserBoundaryHistoryRewriterTest, FailureOfSplitIsNotFatal) {
{
const ConversionRequest convreq = CreateConversionRequest();
Segments segments = MakeSegments({"たんぽぽ", "わたげ"}, Segment::FREE);
EXPECT_CALL(converter, ResizeSegment(&segments, Ref(convreq),
/*start_segment_index=*/0,
/*segments_size=*/1, _))
EXPECT_CALL(converter, ResizeSegments(&segments, Ref(convreq),
/*start_segment_index=*/0, _))
.WillOnce(Return(false));
EXPECT_CALL(converter, ResizeSegment(&segments, Ref(convreq),
/*start_segment_index=*/1,
/*segments_size=*/1, _))
EXPECT_CALL(converter, ResizeSegments(&segments, Ref(convreq),
/*start_segment_index=*/1, _))
.WillOnce(Return(false));
EXPECT_FALSE(rewriter.Rewrite(convreq, &segments));
}
Expand All @@ -431,13 +427,11 @@ TEST_F(UserBoundaryHistoryRewriterTest, FailureOfSplitIsNotFatal) {
Segments segments = MakeSegments({"たんぽぽ", "わたげ"}, Segment::FREE);
const Segments resized =
MakeSegments({"たん", "ぽぽ", "わたげ"}, Segment::FREE);
EXPECT_CALL(converter, ResizeSegment(&segments, Ref(convreq),
/*start_segment_index=*/0,
/*segments_size=*/1, _))
EXPECT_CALL(converter, ResizeSegments(&segments, Ref(convreq),
/*start_segment_index=*/0, _))
.WillOnce(DoAll(SetArgPointee<0>(resized), Return(true)));
EXPECT_CALL(converter, ResizeSegment(&segments, Ref(convreq),
/*start_segment_index=*/1,
/*segments_size=*/1, _))
EXPECT_CALL(converter, ResizeSegments(&segments, Ref(convreq),
/*start_segment_index=*/1, _))
.WillOnce(Return(false));
EXPECT_TRUE(rewriter.Rewrite(convreq, &segments));
}
Expand All @@ -446,13 +440,11 @@ TEST_F(UserBoundaryHistoryRewriterTest, FailureOfSplitIsNotFatal) {
Segments segments = MakeSegments({"たんぽぽ", "わたげ"}, Segment::FREE);
const Segments resized =
MakeSegments({"たんぽぽ", "わた", "げ"}, Segment::FREE);
EXPECT_CALL(converter, ResizeSegment(&segments, Ref(convreq),
/*start_segment_index=*/0,
/*segments_size=*/1, _))
EXPECT_CALL(converter, ResizeSegments(&segments, Ref(convreq),
/*start_segment_index=*/0, _))
.WillOnce(Return(false));
EXPECT_CALL(converter, ResizeSegment(&segments, Ref(convreq),
/*start_segment_index=*/1,
/*segments_size=*/1, _))
EXPECT_CALL(converter, ResizeSegments(&segments, Ref(convreq),
/*start_segment_index=*/1, _))
.WillOnce(DoAll(SetArgPointee<0>(resized), Return(true)));
EXPECT_TRUE(rewriter.Rewrite(convreq, &segments));
}
Expand Down

0 comments on commit bc3c413

Please sign in to comment.