hb-iter-private.hh:75:3: error: bad implicit conversion operator for 'Iter<const int *>' with HB 1.8.5

[rvandermeulen]

Hi, I'm looking at updating HarfBuzz in Firefox and our clang static analysis checker is hitting the below errors:

INFO -  /builds/worker/workspace/build/src/gfx/harfbuzz/src/hb-iter-private.hh:75:3: error: bad implicit conversion operator for 'Iter<const int *>'
INFO -    inline operator bool (void) const { return bool (length); }
INFO -    ^
INFO -  /builds/worker/workspace/build/src/gfx/harfbuzz/src/hb-iter-private.hh:75:3: note: consider adding the explicit keyword to 'operator bool'
INFO -  /builds/worker/workspace/build/src/gfx/harfbuzz/src/hb-iter-private.hh:75:3: error: bad implicit conversion operator for 'Iter<int *>'
INFO -    inline operator bool (void) const { return bool (length); }
INFO -    ^
INFO -  /builds/worker/workspace/build/src/gfx/harfbuzz/src/hb-iter-private.hh:75:3: note: consider adding the explicit keyword to 'operator bool'
INFO -  /builds/worker/workspace/build/src/gfx/harfbuzz/src/hb-iter-private.hh:75:3: error: bad implicit conversion operator for 'Iter<T *>'
INFO -    inline operator bool (void) const { return bool (length); }
INFO -  /builds/worker/workspace/build/src/gfx/harfbuzz/src/hb-iter-private.hh:75:3: note: consider adding the explicit keyword to 'operator bool'

I can confirm that adding an explicit to that line resolves the errors.

[behdad]

I like it to be implicit though. What's bad about it?

I'm going to make a new release later today, if you want to wait.

[behdad]

Ie. I want the iterators to be used in for loops like: for (Iter<T> t (...); t; t++).

[behdad]

Or is that too much magic?

[rvandermeulen]

Hmm, this check has been in our code base for about 3 years now (bug 1153348). We do allow for a MOZ_IMPLICIT annotation on lines we don't want to mark as explicit, but that obviously seems a tad non-ideal for upstream libraries 😃.

I believe I can opt HarfBuzz out of that specific static analysis check as well, but that seems like the only real option here if using explicit isn't what you want.

[behdad]

We haven't started using that idiom yet. I understand why it's desirable to ban it. I think I'm fine making it explicit.

[behdad]

Released 1.8.6 with this change.

[behdad]

Some of our compilers (Oracle Studio and Apple gcc 4.2) do not like explicit on non-constructors :(.
https://circleci.com/gh/harfbuzz/harfbuzz/30289

I think I'll revert this. Do you think you can handle it on your side?

[rvandermeulen]

Yeah, we can work around it.

…

On Thu, Aug 9, 2018, 4:00 AM Behdad Esfahbod ***@***.***> wrote: Reopened #1127 <#1127>. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#1127 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ADfdv4wZElUh4JpUY4Y9z4Cx5TIELMrsks5uO-wAgaJpZM4Vw81o> .

[behdad]

Actually, I think I'll take care of this by enabling the explicit only in C++11 or later. Reading https://en.cppreference.com/w/cpp/language/explicit reminds me believe that if and for statements perform an explicit cast to bool, so I was wrong that we would need to write "for (...; bool(it); ...)" if I add explicit.

You don't need to do anything. I'll fix this.