Closed rvandermeulen closed 1 year ago
Regression from dbb7f47?
No. The name_able_overrides
code is new. If anything, we might need more changes similar to https://github.com/harfbuzz/harfbuzz/commit/dbb7f47b19e60551ef4707f6a2cb60f1bd8334dd
So it looks like it's failing at this code:
+ hb_array (items, mask ? mask + 1 : 0)
| hb_filter (&item_t::is_real)
| hb_map (&item_t::value)
| hb_map (hb_ridentity)
when value_t
has an overriden operator &
. I tried rewriting the line with value
in it as a lambda, but that's only allowed in C++20 the way this code is used.
What baffles me most is this error message: no type named 'item_t' in 'hb_map_iter_t
.
Right now the best I can think of is to somehow disable this code unless HB_EXPERIMENTAL is on, since its use is only switched on that flag.
@rvandermeulen can you check if this is fixed?
@behdad Still some issues with current tip, but fewer than before: https://treeherder.mozilla.org/logviewer?job_id=396484035&repo=try&lineNumber=11767
INFO - /builds/worker/checkouts/gecko/gfx/harfbuzz/src/hb-algs.hh:449:3: note: candidate template ignored: substitution failure [with Proj = const char *, Val = hb_hashmap_t<unsigned int, hb_array_t
, false>::item_t &]: no matching member function for call to 'impl'
That Proj
here is const char *
clearly shows that &item_t::value
instead of resulting in a pointer-to-member, calls the operator &
of the type instead, which is wrong.
Now, we removed all the calls that go through this. The fact that there's still one error left is because this construct appears in the signatures of the values()
function, which is of a non-template type, so gets instantiated.
The only way to work around this is to not instantiate hb_map_t
with hb_bytes_t
on this compiler at all.
We probably need to rewrite this code such that if the experimental feature is not used, the problematic new function signature is not exposed at all.
We probably need to rewrite this code such that if the experimental feature is not used, the problematic new function signature is not exposed at all.
I just did that. Please test.
Confirmed, thanks!
@rvandermeulen We want to move the affected code out of experimental. Is clang 5 still the minimum supported compiler for Mozilla? cc @jfkthame
No, we're at Clang 8.0 minimum now.
Thanks. We'll go ahead with moving those API out of experimental. Please shout out if it breaks clang 8.
Current tip is hitting build failures in Mozilla's CI with clang 5 builds (our minimum supported compiler): https://treeherder.mozilla.org/logviewer?job_id=396298960&repo=try&lineNumber=11512
Regression from https://github.com/harfbuzz/harfbuzz/commit/dbb7f47b19e60551ef4707f6a2cb60f1bd8334dd?