expand/ortermlist.cc:166:12: warning: duplicated 'if' condition

[dcb]

gcc with compiler flag -Wduplicated-cond says

xpand/ortermlist.cc:166:12: warning: duplicated 'if' condition [-Wduplicated-cond]

166 | } else if (cmp < 0) {

| ^~

expand/ortermlist.cc:158:5: note: previously used here

158 | if (cmp < 0) {

| ^~

[Olly Betts]

You don't mention the version, but checking this is present in both git master and 1.4 branch.

It was introduced in master by 6c522cf1e2afc520b02e47a6e0ce3615f544a885, and backported in de75e6508538c6f8553e8313e2185fcc68143c0a which is in 1.4.17 and 1.4.18.

The equivalent code in the next() case is correct, so only skip_to() is affected.

This class is used in a few places, but the skip_to() method can never get called in some uses (such as generating spelling suggestions).

I think this could be triggered with a sharded database by getting a TermIterator from Database::spellings_begin() or Database::synonyms_begin() or Database::synonym_keys_begin() and calling skip_to() on it.

[dcb]

You might consider it prudent to add compiler flag -Wduplicated-cond to your gcc builds. It doesn't seem to be in -Wall.

[Olly Betts]

Replying to Olly Betts:

I think this could be triggered with a sharded database by getting a TermIterator from Database::spellings_begin() or Database::synonyms_begin() or Database::synonym_keys_begin() and calling skip_to() on it.

I wrote a simple regression test using Database::spellings_begin() which highlighted that the implementation of this method is just wrong - currently it skips the left child or right child or both based on whether the current positions are <, > or ==, but really it should skip both whatever the current position order is. The current code will sometimes fail to skip all the way to the requested new position.

[Olly Betts]

Fixed TermIterator::skip_to() in 81c892e9244ffd05b4ae9d8c8970e67f43515256.

[Olly Betts]

Backported that fix for 1.4.19 in 6694111e3a5e1d076ce209fe0cd28d40a4eb4d44.

I've also added code to enable -Wduplicated-cond (and also -Wduplicated-branches) if using a GCC version which is new enough in d00d879ad6edb7474b9c2ca452c29c0792be1994, and backported that in cd4152fd5173cabf7763bcd69e0bf7cccbe75bf0.