ConvertibleTester causes incomplete type errors when UniquePtr is used inside Skia

RESOLVED FIXED in Firefox 47

Status

()

defect
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: lsalzman, Assigned: lsalzman)

Tracking

unspecified
mozilla47
Points:
---

Firefox Tracking Flags

(firefox47 fixed)

Details

Attachments

(1 attachment)

When Skia's skstd::unique_ptr is replaced with our mfbt UniquePtr, we get the following errors:

 0:01.73 In file included from /home/lee/central/gfx/skia/skia/src/core/SkAdvancedTypefaceMetrics.cpp:10:0:
 0:01.73 /home/lee/central/gfx/skia/skia/src/core/SkAdvancedTypefaceMetrics.h: In instantiation of ‘struct SkAdvancedTypefaceMetrics::AdvanceMetric<short int>’:
 0:01.73 /home/lee/central/obj-dbg/dist/include/mozilla/TypeTraits.h:657:58:   required by substitution of ‘template<class From1, class To1, class> static char mozilla::detail::ConvertibleTester<From, To>::test(int) [with From1 = SkAdvancedTypefaceMetrics::AdvanceMetric<short int>*; To1 = SkAdvancedTypefaceMetrics::AdvanceMetric<short int>*; <template-parameter-1-3> = <missing>]’
 0:01.73 /home/lee/central/obj-dbg/dist/include/mozilla/TypeTraits.h:665:26:   required from ‘const bool mozilla::detail::ConvertibleTester<SkAdvancedTypefaceMetrics::AdvanceMetric<short int>*, SkAdvancedTypefaceMetrics::AdvanceMetric<short int>*>::value’
 0:01.73 /home/lee/central/obj-dbg/dist/include/mozilla/TypeTraits.h:696:8:   required from ‘struct mozilla::IsConvertible<SkAdvancedTypefaceMetrics::AdvanceMetric<short int>*, SkAdvancedTypefaceMetrics::AdvanceMetric<short int>*>’
 0:01.73 /home/lee/central/obj-dbg/dist/include/mozilla/UniquePtr.h:474:16:   required by substitution of ‘template<class U> mozilla::DefaultDelete<T>::DefaultDelete(const mozilla::DefaultDelete<U>&, typename mozilla::EnableIf<mozilla::IsConvertible<U*, T*>::value, int>::Type) [with U = SkAdvancedTypefaceMetrics::AdvanceMetric<short int>]’
 0:01.73 /home/lee/central/obj-dbg/dist/include/mozilla/Pair.h:152:8:   required from ‘struct mozilla::Pair<SkAdvancedTypefaceMetrics::AdvanceMetric<short int>*, mozilla::DefaultDelete<SkAdvancedTypefaceMetrics::AdvanceMetric<short int> > >’
 0:01.73 /home/lee/central/obj-dbg/dist/include/mozilla/UniquePtr.h:162:30:   required from ‘class mozilla::UniquePtr<SkAdvancedTypefaceMetrics::AdvanceMetric<short int> >’
 0:01.73 /home/lee/central/gfx/skia/skia/include/core/../private/SkTemplates.h:97:29:   required from ‘class SkAutoTDelete<SkAdvancedTypefaceMetrics::AdvanceMetric<short int> >’
 0:01.73 /home/lee/central/gfx/skia/skia/src/core/SkAdvancedTypefaceMetrics.h:112:31:   required from here
 0:01.73 /home/lee/central/gfx/skia/skia/src/core/SkAdvancedTypefaceMetrics.h:100:45: error: ‘SkAdvancedTypefaceMetrics::AdvanceMetric<Data>::fNext’ has incomplete type
 0:01.73          SkAutoTDelete<AdvanceMetric<Data> > fNext;
 0:01.73                                              ^
 0:01.73 In file included from /home/lee/central/gfx/skia/skia/include/core/SkTArray.h:11:0,
 0:01.73                  from /home/lee/central/gfx/skia/skia/include/core/SkString.h:14,
 0:01.73                  from /home/lee/central/gfx/skia/skia/src/core/SkAdvancedTypefaceMetrics.h:15,
 0:01.73                  from /home/lee/central/gfx/skia/skia/src/core/SkAdvancedTypefaceMetrics.cpp:10:
 0:01.73 /home/lee/central/gfx/skia/skia/include/core/../private/SkTemplates.h:97:29: note: declaration of ‘class SkAutoTDelete<SkAdvancedTypefaceMetrics::AdvanceMetric<short int> >’
 0:01.73  template <typename T> class SkAutoTDelete : public skstd::unique_ptr<T> {
 0:01.73                              ^

This appears to be caused by a change introduced in bug 1221680.
This uses an approach closer to what Skia was using in its own shims, and it bypasses the hazard build issue that Botond was experiencing.

The issue is seemingly caused by having ConvertibleTestHelper outside of the templated class. But moving it inside the class was causing builds problems with the hazard build.

This reformulation seems to, however, not cause either problem anymore.
Attachment #8713334 - Flags: review?(nfroyd)
Comment on attachment 8713334 [details] [diff] [review]
fix ConvertibleTester to not cause incomplete type errors with UniquePtr and Skia

Review of attachment 8713334 [details] [diff] [review]:
-----------------------------------------------------------------

r=me if it works everywhere.
Comment on attachment 8713334 [details] [diff] [review]
fix ConvertibleTester to not cause incomplete type errors with UniquePtr and Skia

Review of attachment 8713334 [details] [diff] [review]:
-----------------------------------------------------------------

r=me  Cross-platform C++ is so much fun, isn't it?
Attachment #8713334 - Flags: review?(nfroyd) → review+
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/57d4d83ecea8
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla47
You need to log in before you can comment on or make changes to this bug.