New issue
Advanced search Search tips

Issue 699747 link

Starred by 1 user

Issue metadata

Status: Available
Owner: ----
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Task



Sign in to add a comment

TextIterator::advance() should have DCHECK(!atEnd()) at the entry of function.

Project Member Reported by xiaoche...@chromium.org, Mar 8 2017

Issue description

When a text iterator is at its end, calling advance() shouldn't do anything. However, it drives the text iterator into an undefined state.

Backwards text iterator has the same issue.

Given that text iterator is currently in an extremely chaotic status with tons of hacks over each other, we may want to rewrite the class entirely, instead of introducing one more hack over all the existing ones...
 

Comment 1 by yosin@chromium.org, Mar 9 2017

Summary: TextIterator::advance() should have DCHECK(!atEnd()) at the entry of function. (was: TextIterator::advance should be no-op when at end)
From code health point view, it is not good to call advance() when atEnd() return true. If callers do so, we can think callers do wrong thing.

All users of TextIterator should be in following form:

for (TextIterator it(...); !it.atEnd(); it.advance()) {
 ...
}

We may want to adopt to range-for:

for (TextIterator::Result& result : TextIterator(...)) {
  ...
}



Labels: Type-Task
Project Member

Comment 3 by sheriffbot@chromium.org, Nov 14

Labels: Hotlist-Recharge-Cold
Status: Untriaged (was: Available)
This issue has been Available for over a year. If it's no longer important or seems unlikely to be fixed, please consider closing it out. If it is important, please re-triage the issue.

Sorry for the inconvenience if the bug really should have been left as Available.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Status: Available (was: Untriaged)

Sign in to add a comment