--with-system-icu fails to build: js/src/builtin/Intl.cpp:3732:12: error: reference to 'PluralRules' is ambiguous

RESOLVED FIXED in Firefox 53

Status

()

defect
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: jbeich, Assigned: jbeich)

Tracking

({regression})

Trunk
mozilla53
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox50 unaffected, firefox51 unaffected, firefox52 unaffected, firefox53 fixed)

Details

Attachments

(1 attachment)

Assignee

Description

2 years ago
$ pkg info -x icu
icu-58.2,1

$ diff intl/icu/source/i18n/unicode/plurrule.h /usr/local/include/unicode/plurrule.h
$ diff intl/icu/source/i18n/unicode/upluralrules.h /usr/local/include/unicode/upluralrules.h

$ echo ac_add_options --with-system-icu >>.mozconfig

$ ./mach build
[...]
In file included from objdir/js/src/Unified_cpp_js_src0.cpp:20:
js/src/builtin/Intl.cpp:3732:12: error: reference to 'PluralRules' is ambiguous
    return PluralRules(cx, args, args.isConstructing());
           ^
js/src/builtin/Intl.cpp:3729:1: note: candidate found by name lookup is 'PluralRules'
PluralRules(JSContext* cx, unsigned argc, Value* vp)
^
js/src/builtin/Intl.cpp:3684:1: note: candidate found by name lookup is 'PluralRules'
PluralRules(JSContext* cx, const CallArgs& args, bool construct)
^
/usr/local/include/unicode/plurrule.h:194:18: note: candidate found by name lookup is
      'icu::PluralRules'
class U_I18N_API PluralRules : public UObject {
                 ^
In file included from objdir/js/src/Unified_cpp_js_src0.cpp:20:
js/src/builtin/Intl.cpp:3732:12: error: no matching constructor for initialization of
      'icu::PluralRules'
    return PluralRules(cx, args, args.isConstructing());
           ^           ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/usr/local/include/unicode/plurrule.h:204:5: note: candidate constructor not viable: requires single
      argument 'status', but 3 arguments were provided
    PluralRules(UErrorCode& status);
    ^
/usr/local/include/unicode/plurrule.h:210:5: note: candidate constructor not viable: requires single
      argument 'other', but 3 arguments were provided
    PluralRules(const PluralRules& other);
    ^
/usr/local/include/unicode/plurrule.h:498:5: note: candidate constructor not viable: requires 0
      arguments, but 3 were provided
    PluralRules();   // default constructor not implemented
    ^
In file included from objdir/js/src/Unified_cpp_js_src0.cpp:20:
js/src/builtin/Intl.cpp:3740:12: error: reference to 'PluralRules' is ambiguous
    return PluralRules(cx, args, true);
           ^
js/src/builtin/Intl.cpp:3729:1: note: candidate found by name lookup is 'PluralRules'
PluralRules(JSContext* cx, unsigned argc, Value* vp)
^
js/src/builtin/Intl.cpp:3684:1: note: candidate found by name lookup is 'PluralRules'
PluralRules(JSContext* cx, const CallArgs& args, bool construct)
^
/usr/local/include/unicode/plurrule.h:194:18: note: candidate found by name lookup is
      'icu::PluralRules'
class U_I18N_API PluralRules : public UObject {
                 ^
In file included from objdir/js/src/Unified_cpp_js_src0.cpp:20:
js/src/builtin/Intl.cpp:3740:12: error: no matching constructor for initialization of
      'icu::PluralRules'
    return PluralRules(cx, args, true);
           ^           ~~~~~~~~~~~~~~
/usr/local/include/unicode/plurrule.h:204:5: note: candidate constructor not viable: requires single
      argument 'status', but 3 arguments were provided
    PluralRules(UErrorCode& status);
    ^
/usr/local/include/unicode/plurrule.h:210:5: note: candidate constructor not viable: requires single
      argument 'other', but 3 arguments were provided
    PluralRules(const PluralRules& other);
    ^
/usr/local/include/unicode/plurrule.h:498:5: note: candidate constructor not viable: requires 0
      arguments, but 3 were provided
    PluralRules();   // default constructor not implemented
    ^
In file included from objdir/js/src/Unified_cpp_js_src0.cpp:20:
js/src/builtin/Intl.cpp:3763:43: error: reference to 'PluralRules' is ambiguous
    ctor = global->createConstructor(cx, &PluralRules, cx->names().PluralRules, 0);
                                          ^
js/src/builtin/Intl.cpp:3729:1: note: candidate found by name lookup is 'PluralRules'
PluralRules(JSContext* cx, unsigned argc, Value* vp)
^
js/src/builtin/Intl.cpp:3684:1: note: candidate found by name lookup is 'PluralRules'
PluralRules(JSContext* cx, const CallArgs& args, bool construct)
^
/usr/local/include/unicode/plurrule.h:194:18: note: candidate found by name lookup is
      'icu::PluralRules'
class U_I18N_API PluralRules : public UObject {
                 ^
5 errors generated.
Taking.

:waldo, any idea on what to do here?
Assignee: nobody → gandalf
Flags: needinfo?(jwalden+bmo)
Assignee

Comment 2

2 years ago
After comment 0 there's one more:

objdir/js/src/Unified_cpp_js_src0.o: In function `js::intl_GetPluralCategories(JSContext*, unsigned int, JS::Value*)':
objdir/js/src/Unified_cpp_js_src0.cpp:(.text._ZN2js24intl_GetPluralCategoriesEP9JSContextjPN2JS5ValueE+0xe4): undefined reference to `icu::PluralRules::getKeywords(UErrorCode&) const'
/usr/bin/ld: objdir/js/src/Unified_cpp_js_src0.o: relocation R_X86_64_PC32 against `_ZNK3icu11PluralRules11getKeywordsER10UErrorCode' can not be used when making a shared object; recompile with-fPIC
Comment hidden (mozreview-request)
Assignee

Comment 4

2 years ago
Comment on attachment 8820988 [details]
Bug 1325246 - Unbreak build --with-system-icu after bug 1270146.

With this patch my build finished.
Oh, sweet! Thanks for the patch Jan!
Assignee: gandalf → jbeich
Flags: needinfo?(jwalden+bmo)
I don't particularly have problems with the patch, if it's what's necessary, and there's no way to avoid the one-off weirdness.

But why exactly is it necessary?  builtin/Intl.cpp doesn't have a |using namespace icu;| to make |icu::PluralRules| visible.  build/autoconf/icu.m4 has AC_DEFINE(U_USING_ICU_NAMESPACE,0) to prevent the namespace from being opened.  So why is |icu::PluralRules| relevant here?  And, if it's somehow not, why is name lookup complaining about this?

If we can avoid one-off qualifications like this for no apparent reason, I'd really like to do that.  Fixing the apparent mistaken namespace-opening would solve that, and it would avoid problems in the future.
Assignee

Comment 7

2 years ago
(In reply to Jeff Walden [:Waldo] (remove +bmo to email) from comment #6)
> build/autoconf/icu.m4 has AC_DEFINE(U_USING_ICU_NAMESPACE,0) to prevent the
> namespace from being opened.

Which is hidden behind -z $MOZ_SYSTEM_ICU and has been that way since bug 851992.
Oh, ugh.  Could you possibly write a patch to apply that define to any build that compiles intl stuff, whether it's using embedded ICU or system ICU?  Let's cut off all future instances of this problem now, the right way.
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)

Comment 11

2 years ago
mozreview-review
Comment on attachment 8820988 [details]
Bug 1325246 - Unbreak build --with-system-icu after bug 1270146.

https://reviewboard.mozilla.org/r/100366/#review104370
Attachment #8820988 - Flags: review?(mh+mozilla) → review+
Assignee

Updated

2 years ago
Keywords: checkin-needed

Comment 12

2 years ago
Pushed by ihsiao@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/cefe354e0bad
Unbreak build --with-system-icu after bug 1270146. r=glandium
Keywords: checkin-needed

Comment 13

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/cefe354e0bad
Status: NEW → RESOLVED
Last Resolved: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla53
You need to log in before you can comment on or make changes to this bug.