1300925 - MSVC compilation error in Intl.cpp with --without-intl-api after bug 1289934

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Attachment: Use anon namespace for Intl stubs

After bug 1289934 landed, building with MSVC (2015) and --without-intl-api results in compilation errors like the following:

Unified_cpp_js_src3.cpp
c:/proj/r/mozjs/mozjs/js/src/builtin/Intl.cpp(859): error C2971: 'ScopedICUObject': template parameter 'Delete': 'uenum_close': a variable with non-static storage duration cannot be used as a non-type argument
c:/proj/r/mozjs/mozjs/js/src/builtin/Intl.cpp(615): note: see declaration of 'ScopedICUObject'
c:/proj/r/mozjs/mozjs/js/src/builtin/Intl.cpp(128): note: see declaration of 'uenum_close'

In Intl.cpp, putting all the stub code in an anonymous namespace, and changing the functions to non-static fixes this issue, like in the attached patch.

Comment 1

[Jeff Walden [:Waldo]]

Comment on attachment 8788700 [details] [diff] [review]
Use anon namespace for Intl stubs

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

Functions passed as non-type arguments to templates are required to have external or internal linkage, C++11 [temp.arg.nontype]p1.  And a name having namespace scope -- that is, one at top level or directly within a namespace -- has internal linkage if it's a function explicitly declared static, C++11 [basic.link]p3.  So this is an MSVC bug.

But this seems to basically work, I guess, and it doesn't really hurt anything else.  So okay.

Comment 2

[Daniel Holbert [:dholbert]]

Vlad, looks like this is ready to land?

(I just ran into this issue myself, and was going to file a bug and then found this one, with an r+'d patch that's been sitting for 2 months. :))

Comment 3

[(Away)]

(In reply to Daniel Holbert [:dholbert] from comment #2)
> Vlad, looks like this is ready to land?
> 
> (I just ran into this issue myself, and was going to file a bug and then
> found this one, with an r+'d patch that's been sitting for 2 months. :))

Same here. I'll push the patch myself if I don't hear objections soon. :-)

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/9822e0525b82
Change Intl.cpp stubs to an anonymous namespace. r=Waldo

Comment 5

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/9822e0525b82