One of the bots is unhappy when HB_NODISCARD comes after HB_INTERNAL.
No idea why. But, again, we're testing HarfBuzz, not C++, not clang. Ugh.
In file included from src/harfbuzz.cc:1:
In file included from src/hb-aat-layout.cc:30:
In file included from src/hb-aat-layout.hh:32:
In file included from src/hb-ot-shape.hh:32:
In file included from src/hb-ot-map.hh:32:
src/hb-buffer.hh:335:15: error: an attribute list cannot appear here
HB_INTERNAL HB_NODISCARD bool move_to (unsigned int i); /* i is output-buffer index. */
^~~~~~~~~~~~
https://app.circleci.com/pipelines/github/harfbuzz/harfbuzz/1693/workflows/77459205-a189-45d3-bc58-52a8fd952c3f/jobs/155912/parallel-runs/0/steps/0-110?invite=true
To my surprise, saves ~20kb in my build (non-size-optimized) build.
The output_glyph() method is never used in the fast paths, so doesn't
matter if is not fully optimized for the special case it is.
Previous error-handling philosophy was that user doesn't need to
immediately know whether operation failed. But as can be seen after
we added malloc-failing fuzzing, there's just so many places in the
code that a failure of these operations needs to be mitigated before
further operations. So I'm moving towards returning success here,
and possibly making it nodiscard.
I did a review; changed some "return"s to "break"s, which should be identical.
Removed one check just before "continue" because not necessary.
The added error check is the actual fix.
Should fix https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=31755
We treat Class0 as "doesn't intersect". That's the only meaningful
interpretation. If one allos Class0 to mean "intersects", then the
intersects() result should be true iff glyphset is non-empty.
Related to https://github.com/harfbuzz/harfbuzz/issues/2703
Don’t replace Default_Ignorables with zero-width space if they are
substituted or multiplied, not just when ligated.
After this change, HarfBuzz output matches that of Uniscribe and
CoreText for the new tests.
Fixes https://github.com/harfbuzz/harfbuzz/issues/2883
I was getting check-symbols failure because my previous build was
without CoreText, and after reconfiguring with CoreText, the old
harfbuzz.defs file was not being regenerated.
Was producing non-monotonic cluster numbers because our faulty logic
was not merging clusters if something from before base and after base
had switched positions.
Fixes https://github.com/harfbuzz/harfbuzz/issues/2272
Previous commit didn't fix the bots. Putting it back now that I
understand why I initially did the "Wide" casts. But only doing
it for out-cast this time. This causes "narrowing" warnings
whenever we are converting signed/unsigned to smaller HBUINT16
etc. But those are valuable warnings. We should address those
separately instead of ignoring.
Maybe we should start using uint16_t more liberally in the
internal subsetter function signatures then.
My local clang12 is fine, but many bots are not:
../src/hb-ot-cff1-table.hh: In instantiation of ‘bool CFF::Charset1_2<TYPE>::sanitize(hb_sanitize_context_t*, unsigned int) const [with TYPE = OT::IntType<unsigned char>]’:
../src/hb-ot-cff1-table.hh:554:13: required from here
../src/hb-ot-cff1-table.hh:377:60: error: comparison between signed and unsigned integer expressions [-Werror=sign-compare]
if (unlikely (!ranges[i].sanitize (c) || (num_glyphs < ranges[i].nLeft + 1)))
~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~
Enabling the extra cast operator mentioned in previous commit to see if
that fixes this case.
Again, I'd be happy to say "use 1u instead of 1" if this was universally
erred on. But since some compilers happily compile this while others
err, it would be a huge headache. Let's see...
https://github.com/harfbuzz/harfbuzz/pull/2875
Say for USHORT, we were implementing casts from and to unsigned.
With this change, we cast from and to uint16_t only. This allows
compiler more opportunities to catch possible narrowing issues in
the code.
It needed a couple of fixes in the codebase though, because
previously, if a USHORT was participating in arithmetic with signed
numbers, eg. "u + 1", the result would have been unsigned. With
this change, it would be signed. The correct fix is to update the
code to read "u + 1u".
That said, I think about conditionally adding back the cast
out to signed/unsigned, to facilitate better type deduction.
But I couldn't think of a real situation where that would help
with anything. So I didn't add. Here's what it was:
template <typename Type2 = hb_conditional<hb_is_signed (Type), signed, unsigned>,
hb_enable_if (sizeof (Type) < sizeof (Type2))>
operator hb_type_identity_t<Type2> () const { return v; }
https://github.com/harfbuzz/harfbuzz/pull/2875
Behdad Esfahbod
David Corbett
Khaled Hosny