Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

'hb-set.hh' build errors with MSVC #1374

Closed
johne53 opened this issue Nov 9, 2018 · 21 comments
Closed

'hb-set.hh' build errors with MSVC #1374

johne53 opened this issue Nov 9, 2018 · 21 comments

Comments

@johne53
Copy link

johne53 commented Nov 9, 2018

I'm seeing lots of errors when trying to build harfbuzz (from git master) with MSVC. They relate to some recent changes in 'src/hb-set.hh' (approx line 370) where 'sizeof (pages.arrayZ()[0])' got changed to 'sizeof (pages[0])'.

A similar error occurs on the following line - where 'sizeof (page_map.arrayZ()[0])' got changed to 'sizeof (page_map[0])'. This is the general form for each error:-

harfbuzz\src\hb-set.hh(378) : error C2666: 'hb_vector_t<Type,StaticSize>::operator []' : 4 overloads have similar conversions
: could be 'const hb_set_t::page_t &hb_vector_t<Type,StaticSize>::operator [](unsigned int) const'
: or 'hb_set_t::page_t &hb_vector_t<Type,StaticSize>::operator [](unsigned int)'
: or 'built-in C++ operator[(T *, int)'
: or 'built-in C++ operator[(const T *, int)'

I'm also seeing "multiple overload" errors in other places - e.g. for DEFINE_SIZE_ARRAY and its variants. As it happens, this project needs to get built with MSVC2008 - although these particular errors look like they'd occur, even with the more recent versions.

@behdad
Copy link
Member

behdad commented Nov 9, 2018

John. If you want a fix, I need all different instances of the error.

behdad added a commit that referenced this issue Nov 9, 2018
@johne53
Copy link
Author

johne53 commented Nov 10, 2018

Hi Behdad - I haven't loaded your new commit yet (if it only deals with warnings) but here are the errors I see from just compiling 'hb-aat-layout.cc' (the remaining source files seem to produce the same errors):-

(line 378 - hb-set.hh) - memcpy (pages, other->pages, count * sizeof (pages[0]));
error C2666: 'hb_vector_t<Type,StaticSize>::operator []' : 4 overloads have similar conversions
: could be 'const hb_set_t::page_t &hb_vector_t<Type,StaticSize>::operator [](unsigned int) const'
: or 'hb_set_t::page_t &hb_vector_t<Type,StaticSize>::operator [](unsigned int)'
: or 'built-in C++ operator[(T *, int)'
: or 'built-in C++ operator[(const T *, int)'

(line 379 - hb-set.hh) - memcpy (page_map, other->page_map, count * sizeof (page_map[0]));
error C2666: 'hb_vector_t<Type,StaticSize>::operator []' : 4 overloads have similar conversions
: could be 'const hb_set_t::page_map_t &hb_vector_t<Type,StaticSize>::operator [](unsigned int) const'
: or 'hb_set_t::page_map_t &hb_vector_t<Type,StaticSize>::operator [](unsigned int)'
: or 'built-in C++ operator[(T *, int)'
: or 'built-in C++ operator[(const T *, int)'

(line 663 - hb-set.hh) - return page_map[i].major * page_t::PAGE_BITS + page_at (i).get_max ();
error C2666: 'hb_vector_t<Type,StaticSize>::operator []' : 2 overloads have similar conversions
: could be 'const hb_set_t::page_map_t &hb_vector_t<Type,StaticSize>::operator [](unsigned int) const'
: or 'built-in C++ operator[(const T *, int)'

(line 681 - hb-set.hh) - (page_map.len - 1 - i) * sizeof (page_map[0]))
error C2666: 'hb_vector_t<Type,StaticSize>::operator []' : 4 overloads have similar conversions
: could be 'const hb_set_t::page_map_t &hb_vector_t<Type,StaticSize>::operator [](unsigned int) const'
: or 'hb_set_t::page_map_t &hb_vector_t<Type,StaticSize>::operator [](unsigned int)'
: or 'built-in C++ operator[(T *, int)'
: or 'built-in C++ operator[(const T *, int)'

(line 1520 - hb-ot-layout-common.hh) - DEFINE_SIZE_ARRAY (4, axesZ);
error C2666: 'OT::UnsizedArrayOf::operator []' : 2 overloads have similar conversions
: could be 'const OT::VarRegionAxis &OT::UnsizedArrayOf::operator [](unsigned int) const'
: or 'built-in C++ operator[(const T *, int)'

(line 1579 - hb-ot-layout-common.hh) - DEFINE_SIZE_ARRAY2 (6, regionIndices, bytesX);
error C2666: 'OT::UnsizedArrayOf::operator []' : 2 overloads have similar conversions
: could be 'const OT::HBUINT8 &OT::UnsizedArrayOf::operator [](unsigned int) const'
: or 'built-in C++ operator[(const T *, int)'

(line 1895- hb-ot-layout-common.hh) - DEFINE_SIZE_ARRAY (6, deltaValueZ);
error C2666: 'OT::UnsizedArrayOf::operator []' : 2 overloads have similar conversions
: could be 'const OT::HBUINT16 &OT::UnsizedArrayOf::operator [](unsigned int) const'
: or 'built-in C++ operator[(const T *, int)'

(line 1350 - hb-ot-layout-gsubgpos.hh) - return_trace (inputCount.sanitize (c) &&
error C2666: 'OT::UnsizedArrayOf::operator []' : 2 overloads have similar conversions
: could be 'const OT::HBUINT16 &OT::UnsizedArrayOf::operator [](unsigned int) const'
: or 'built-in C++ operator[(const T *, int)'

(line 1369 - hb-ot-layout-gsubgpos.hh) - DEFINE_SIZE_ARRAY (4, inputZ);
error C2666: 'OT::UnsizedArrayOf::operator []' : 2 overloads have similar conversions
: could be 'const OT::HBUINT16 &OT::UnsizedArrayOf::operator [](unsigned int) const'
: or 'built-in C++ operator[(const T *, int)'

(line 1675 - hb-ot-layout-gsubgpos.hh) - if (!(this+coverageZ[0]).intersects (glyphs))
error C2666: 'OT::UnsizedArrayOf::operator []' : 2 overloads have similar conversions
: could be 'const OT::HBUINT16 &OT::UnsizedArrayOf::operator [](unsigned int) const'
: or 'built-in C++ operator[(const T *, int)'

(line 1690 - hb-ot-layout-gsubgpos.hh) - if (!(this+coverageZ[0]).intersects (c->glyphs))
error C2666: 'OT::UnsizedArrayOf::operator []' : 2 overloads have similar conversions
: could be 'const OT::OffsetTo &OT::UnsizedArrayOf<OT::OffsetTo>::operator [](unsigned int) const'
: or 'built-in C++ operator[(const T *, int)'

(line 1707 - hb-ot-layout-gsubgpos.hh) - (this+coverageZ[0]).add_coverage (c->input)
error C2666: 'OT::UnsizedArrayOf::operator []' : 2 overloads have similar conversions
: could be 'const OT::OffsetTo &OT::UnsizedArrayOf<OT::OffsetTo>::operator [](unsigned int) const'
: or 'built-in C++ operator[(const T *, int)'

(line 1734 - hb-ot-layout-gsubgpos.hh) - { return this+coverageZ[0]; }
error C2666: 'OT::UnsizedArrayOf::operator []' : 2 overloads have similar conversions
: could be 'const OT::OffsetTo &OT::UnsizedArrayOf<OT::OffsetTo>::operator [](unsigned int) const'
: or 'built-in C++ operator[(const T *, int)'

(line 1739 - hb-ot-layout-gsubgpos.hh) - unsigned int index = (this+coverageZ[0]).get_coverage (c->buffer->cur().codepoint);
error C2666: 'OT::UnsizedArrayOf::operator []' : 2 overloads have similar conversions
: could be 'const OT::OffsetTo &OT::UnsizedArrayOf<OT::OffsetTo>::operator [](unsigned int) const'
: or 'built-in C++ operator[(const T *, int)'

(line 1782 - hb-ot-layout-gsubgpos.hh) - DEFINE_SIZE_ARRAY (6, coverageZ);
error C2666: 'OT::UnsizedArrayOf::operator []' : 2 overloads have similar conversions
: could be 'const OT::OffsetTo &OT::UnsizedArrayOf<OT::OffsetTo>::operator [](unsigned int) const'
: or 'built-in C++ operator[(const T *, int)'

(line 399 - hb-ot-layout-gpos-table.hh) - DEFINE_SIZE_ARRAY (2, matrixZ);
error C2666: 'OT::UnsizedArrayOf::operator []' : 2 overloads have similar conversions
: could be 'const OT::OffsetTo &OT::UnsizedArrayOf<OT::OffsetTo>::operator [](unsigned int) const'
: or 'built-in C++ operator[(const T *, int)'

(line 165 - hb-aat-layout-trak-table.hh) - DEFINE_SIZE_ARRAY (8, trackTable)
error C2666: 'OT::UnsizedArrayOf::operator []' : 2 overloads have similar conversions
: could be 'const AAT::TrackTableEntry &OT::UnsizedArrayOf::operator [](unsigned int) const'
: or 'built-in C++ operator[(const T *, int)'

(line 165 - hb-aat-layout-trak-table.hh) - DEFINE_SIZE_ARRAY (8, trackTable)
error C2228: left of '.static_size' must have class/struct/union

@johne53
Copy link
Author

johne53 commented Nov 10, 2018

Hi Behdad - I just updated from git and your changes do seem to have fixed the errors (in 'hb-set.hh') at lines 378 and 379. So you probably need to do something similar in the other cases. Thanks for helping with this.

@behdad
Copy link
Member

behdad commented Nov 10, 2018

Okay, give it another try.

@behdad behdad closed this as completed in b4c6113 Nov 10, 2018
behdad added a commit that referenced this issue Nov 11, 2018
@johne53
Copy link
Author

johne53 commented Nov 11, 2018

Okay, there seem to be problems now with the use of 'static_size' in various places. The most common ones seem to be in the macro DEFINE_SIZE_ARRAY. Wherever it gets used I see an error like this:-

error C2228: left of '.static_size' must have class/struct/union : type is 'const T'

I'm seeing similar issues with the use of 'coverageZ' which gives similar errors when it exposes members like 'sanitize' . 'get_coverage' / 'add_coverage' etc.

Maybe I should go back to the previous version and just accept that Harfbuzz won't build for me any more..?

@behdad
Copy link
Member

behdad commented Nov 11, 2018

error C2228: left of '.static_size' must have class/struct/union : type is 'const T'

Okay, this is becoming bizarre. What was the last version you could compile?

I'm seeing similar issues with the use of 'coverageZ' which gives similar errors when it exposes members like 'sanitize' . 'get_coverage' / 'add_coverage' etc.

Please paste error message.

@behdad behdad reopened this Nov 11, 2018
@johne53
Copy link
Author

johne53 commented Nov 11, 2018

The last time Harfbuzz built successfully was only about 3 weeks ago (Ebrahim's commit #03e144135b from 18th Oct).

To be honest, these feel like multiple instances of a small number of problems. This isn't a comprehensive list but as some examples, I'm seeing errors like these, relating to DEFINE_SIZE_ARRAY:-

(line 1520 - hb-ot-layout-common.hh) - DEFINE_SIZE_ARRAY (4, axesZ);
error C2228: left of '.static_size' must have class/struct/union
: type is 'const T'

(line 1579 - hb-ot-layout-common.hh) - DEFINE_SIZE_ARRAY2 (6, regionIndices, bytesX);
error C2228: left of '.static_size' must have class/struct/union
: type is 'const T'

(line 1369 - hb-ot-layout-gsubgpos.hh) - DEFINE_SIZE_ARRAY (4, inputZ);
error C2228: left of '.static_size' must have class/struct/union
: type is 'const T'

*** and several more like the above - plus errors like these (relating to 'coverageZ') ***

(line 1675 - hb-ot-layout-gsubgpos.hh) - if (!(this+coverageZ[0]).intersects (glyphs))
error C2228: left of '.intersects' must have class/struct/union
: type is 'const T'

(line 1707 - hb-ot-layout-gsubgpos.hh) - (this+coverageZ[0]).add_coverage (c->input);
error C2228: left of '.add_coverage' must have class/struct/union
: type is 'const T'

(line 1739 - hb-ot-layout-gsubgpos.hh) - unsigned int index = (this+coverageZ[0]).get_coverage (c->buffer->cur().codepoint);
error C2228: left of '.get_coverage' must have class/struct/union
: type is 'const T'

(line 1765 - hb-ot-layout-gsubgpos.hh) - if (!coverageZ[i].sanitize (c, this)) return_trace (false);
error C2228: left of '.sanitize' must have class/struct/union
: type is 'const T'

@behdad
Copy link
Member

behdad commented Nov 11, 2018

Such bizarre errors. I'm out of ideas. Either play with DEFINE_SIZE_ARRAY, diff it with the last good version, change, see what works (does removing the paranthesis around array help?!!! Not that that would be safe...), or bisect and find which change introduced it. That compiler seems to be so b0rked.

@johne53
Copy link
Author

johne53 commented Nov 12, 2018

Yes it does seem strange. From a superficial glance, the lines which are failing don't seem to have changed between the working version and the non-working version. I'll try git bisect and see if that reveals anything.

@johne53
Copy link
Author

johne53 commented Nov 12, 2018

Okay, the problematic commit seems to be commit #955aa56b11 from 26th Oct 2018 and it's entitled "[vector] Make it act more like pointer". I can't say if that's responsible for all the problems I'm seeing but that's definitely where they began.

Can I ask an obvious question..? Does Harfbuzz need C++11? (because obviously, that wouldn't be available as far back as VS2008).

@johne53
Copy link
Author

johne53 commented Nov 12, 2018

And this is only a hunch - but I've a feeling the remaining problems originate in the changes made to 'hb-open-type.hh' and 'hb-dsalgs.hh' between 31st Oct and 5th Nov (I'm referring to the changes that have now affected 'DEFINE_SIZE_ARRAY'). Those ones look quite similar to the changes I just reported in 'hb-vector.hh'.

@behdad
Copy link
Member

behdad commented Nov 18, 2018

Please reopen if you can debug and send a patch. Otherwise, faulty compiler none of the developers have access to.

@behdad behdad closed this as completed Nov 18, 2018
ebraminio pushed a commit to ebraminio/harfbuzz that referenced this issue Nov 24, 2018
ebraminio pushed a commit to ebraminio/harfbuzz that referenced this issue Nov 24, 2018
ebraminio pushed a commit to ebraminio/harfbuzz that referenced this issue Nov 24, 2018
@ebraminio
Copy link
Collaborator

Hey @johne53, we made some progress here, have a look at master.

@johne53
Copy link
Author

johne53 commented Nov 30, 2018

Thanks for persevering guys. Things have definitely improved but essentially, I'm seeing the same kind of problems that I documented on Nov 10th.. Taking 'hb-set.hh' as an example, the C2666 errors I reported (at lines 378, 379 and 663) are gone now - but the error at line 681 is still present.

And for 'hb-ot-layout-common.hh', the DEFINE_SIZE_ARRAY errors are still present - although the error for DEFINE_SIZE_ARRAY2 seems to have gone now.

Essentially, Behdad is right (this is definitely a compiler issue). I can build Harfbuzz okay with VS2015.

It'd be nice to get the errors fixed (as I still need to use VS2008 for this particular project). But if that involves too much work, I can just use an older version of Harfbuzz when building this project

@ebraminio
Copy link
Collaborator

Interesting that 32bit version of llvm-gcc-4.2 reproduces the same issues apparently https://circleci.com/gh/harfbuzz/harfbuzz/60776 so Behdad now has access to the compiler :)

@ebraminio
Copy link
Collaborator

ebraminio commented Nov 30, 2018

Now our weirdest bot compiles harfbuzz, I guess VS2008 should also, please test it :)

behdad added a commit that referenced this issue Dec 1, 2018
The built-in operator takes signed int.  So, match it, such that
the built-in is never a better or equally-good match to our operator.
Fixes "ambiguous overload" errors from gcc 4.2 and VS 2008.

See #1374
behdad added a commit that referenced this issue Dec 1, 2018
Now that we have 6daf45e0, revert cryptic hacks...

This reverts commit abd81ed.
This reverts commit 9c6921c.
This reverts commit d39760c.
This reverts commit fedd8e6.

Fixes #1374
behdad added a commit that referenced this issue Dec 1, 2018
The built-in operator takes signed int.  So, match it, such that
the built-in is never a better or equally-good match to our operator.
Fixes "ambiguous overload" errors from gcc 4.2 and VS 2008.

See #1374
behdad added a commit that referenced this issue Dec 1, 2018
Now that we have 6daf45e0, revert cryptic hacks...

This reverts commit abd81ed.
This reverts commit 9c6921c.
This reverts commit d39760c.
This reverts commit fedd8e6.

Fixes #1374
@johne53
Copy link
Author

johne53 commented Dec 1, 2018

Thanks again guys.

The good news is that the error I referred to in 'hb-set.hh' (line 681) is fixed now. And likewise, the errors in 'hb-ot-layout-common.hh' (i.e. DEFINE_SIZE_ARRAY) are also now fixed. The bad news is that this has just exposed similar errors elsewhere - e.g. 'hb-set.hh' now shows the same kind of error at lines 241, 395, 548, 550, 552, 559, 562, 579, 581 and 583 (which I wasn't seeing before). That's to be expected though (it just means that the compilation can get a bit further now).

FWIW I also see similar errors in:-
'hb-ot-map.hh' (lines 143 / 144 / 145)
'hb-aat-layout-feat-table.hh' (lines 113 / 123 / 171)

One thing that complicates everything is the sheer number of updates to Harfbuzz (often hundreds of commits per day). If you need to solve this issue specifically, it'd make sense to create a temporary branch (just for fixing this problem) and eventually merge it into master.

I get the feeling this'll turn into a long job though... :-(

@behdad
Copy link
Member

behdad commented Dec 1, 2018

John, I don't know how many times I need to say this: I need all error messages, not just a casual line number mention.

@johne53
Copy link
Author

johne53 commented Dec 1, 2018

Sorry Behdad - you gave the impression that you wanted to consider this one as closed (given that the code compiles okay with more recent versions of MSVC). Here's a list of the ones I'm currently seeing for 'hb-set.hh', To be honest though, I think there'll be dozens more after this (if not hundreds!!)

I'm tending to concentrate on the C2666 errors here since they're by far the most common:-

(line 241 - hb-set.hh) - if (!pages[i].is_empty ())
: error C2666: 'hb_vector_t<Type,PreallocedCount>::operator []' : 2 overloads have similar conversions
: could be 'const hb_set_t::page_t &hb_vector_t<Type,PreallocedCount>::operator const'
: or 'built-in C++ operator[(const T *, unsigned int)'

(line 395 - hb-set.hh) - if (page_map[a].major != other->page_map[b].major ||
: error C2666: 'hb_vector_t<Type,PreallocedCount>::operator []' : 2 overloads have similar conversions
: could be 'const hb_set_t::page_map_t &hb_vector_t<Type,PreallocedCount>::operator const'
: or 'built-in C++ operator[(const T *, unsigned int)'

(line 548 - hb-set.hh) - if (i < page_map.len && page_map[i].major == map.major)
: error C2666: 'hb_vector_t<Type,PreallocedCount>::operator []' : 2 overloads have similar conversions
: could be 'const hb_set_t::page_map_t &hb_vector_t<Type,PreallocedCount>::operator const'
: or 'built-in C++ operator[(const T *, unsigned int)'

(line 550 - hb-set.hh) - if (pages[page_map[i].index].next (codepoint))
: error C2666: 'hb_vector_t<Type,PreallocedCount>::operator []' : 2 overloads have similar conversions
: could be 'const hb_set_t::page_map_t &hb_vector_t<Type,PreallocedCount>::operator const'
: or 'built-in C++ operator[(const T *, unsigned int)'

(line 552 - hb-set.hh) - *codepoint += page_map[i].major * page_t::PAGE_BITS;
: error C2666: 'hb_vector_t<Type,PreallocedCount>::operator []' : 2 overloads have similar conversions
: could be 'const hb_set_t::page_map_t &hb_vector_t<Type,PreallocedCount>::operator const'
: or 'built-in C++ operator[(const T *, unsigned int)'

(line 559 - hb-set.hh) - hb_codepoint_t m = pages[page_map[i].index].get_min ();
: error C2666: 'hb_vector_t<Type,PreallocedCount>::operator []' : 2 overloads have similar conversions
: could be 'const hb_set_t::page_map_t &hb_vector_t<Type,PreallocedCount>::operator const'
: or 'built-in C++ operator[(const T *, unsigned int)'

(line 562 - hb-set.hh) - *codepoint = page_map[i].major * page_t::PAGE_BITS + m;
: error C2666: 'hb_vector_t<Type,PreallocedCount>::operator []' : 2 overloads have similar conversions
: could be 'const hb_set_t::page_map_t &hb_vector_t<Type,PreallocedCount>::operator const'
: or 'built-in C++ operator[(const T *, unsigned int)'

(line 579 - hb-set.hh) - if (i < page_map.len && page_map[i].major == map.major)
: error C2666: 'hb_vector_t<Type,PreallocedCount>::operator []' : 2 overloads have similar conversions
: could be 'const hb_set_t::page_map_t &hb_vector_t<Type,PreallocedCount>::operator const'
: or 'built-in C++ operator[(const T *, unsigned int)'

(line 581 - hb-set.hh) - if (pages[page_map[i].index].previous (codepoint))
: error C2666: 'hb_vector_t<Type,PreallocedCount>::operator []' : 2 overloads have similar conversions
: could be 'const hb_set_t::page_map_t &hb_vector_t<Type,PreallocedCount>::operator const'
: or 'built-in C++ operator[(const T *, unsigned int)'

(line 583 - hb-set.hh) - (page_map.len - 1 - i) * page_map.item_size);
: error C2666: 'hb_vector_t<Type,PreallocedCount>::operator []' : 2 overloads have similar conversions
: could be 'const hb_set_t::page_map_t &hb_vector_t<Type,PreallocedCount>::operator const'
: or 'built-in C++ operator[(const T *, unsigned int)'

@behdad
Copy link
Member

behdad commented Dec 1, 2018

I'm still interested to see, since I've been kicking a similarly old compiler... :)

@behdad
Copy link
Member

behdad commented Dec 1, 2018

But yeah, the new ones are harder to fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants