User Agent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/53.0.2785.143 Safari/537.36 Endless Steps to reproduce: Used JS::CallArgs::requireAtLeast() in embedder code. Actual results: Compiled fine, but linker error: ./.libs/libgjs.so: undefined reference to `JS::CallArgs::requireAtLeast(JSContext*, char const*, unsigned int)' collect2: error: ld returned 1 exit status Expected results: I would expect this to be usable from embeddings like the rest of the JSAPI. Looks like it's missing a JS_PUBLIC_API() macro. If I add the macro in the appropriate place in js/src/public/CallArgs.h and js/src/jsapi.cpp, then it works fine.

CC'ing a few API people. Thanks for the report!

Philip, can you create a patch for this, too? And perhaps ask :fitzgen for review. For this and for other patches, it'd be extremely helpful and speed up landing them if you could ensure that they're formatted in the right way. See https://developer.mozilla.org/en-US/docs/Mercurial/Using_Mercurial#How_can_I_generate_a_patch_for_somebody_else_to_check-in_for_me.3F for details on that. I think the two important parts are 8 lines of diff context and the commit message format "Bug NNNNNNN - Description. r=reviewer"

I found that this one was already fixed in trunk, bug 1346389. The patch doesn't apply cleanly to ESR52, but I will fix it up and attach it there. However, since I reported this I've found other functions that also need to be marked JS_PUBLIC_API, so I'll prepare a patch for those: JS::UTF8CharsToNewTwoByteCharsZ(), JS::LossyUTF8CharsToNewTwoByteCharsZ(), and some more friends from CharacterEncoding.h. Till, thanks for the nudge, I bit the bullet and cloned mozilla-central, and I'll try to generate correct patches from now on :-) I'm not usually sure who to request for review though. Is there some overview of who owns what part of SpiderMonkey code?

(In reply to Philip Chimento [:ptomato] from comment #3) > Till, thanks for the nudge, I bit the bullet and cloned mozilla-central, and > I'll try to generate correct patches from now on :-) > I'm not usually sure who to request for review though. Is there some > overview of who owns what part of SpiderMonkey code? Excellent, thank you! :) We don't really have an up-to-date overview of code owners, no. Bugzilla has a "suggest reviewers" thing, though: when you set the review flag to "?", in addition to the input field there's a dropdown showing a list of potential reviewers. Choosing one of them with a short review queue is usually a good start. Should your review get stuck, feel free to set a needinfo flag for me and I'll redirect.

Comment on attachment 8885103 [details] [diff] [review] Mark character encoding functions as public API Review of attachment 8885103 [details] [diff] [review]: -----------------------------------------------------------------

Ugh, emojis kill bugzilla comments, I forgot. Thanks for the patch!

Try push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=2c19ca1ea6d2

Comment on attachment 8885103 [details] [diff] [review] Mark character encoding functions as public API [Approval Request Comment] If this is not a sec:{high,crit} bug, please state case for ESR consideration: required to embed standalone spidermonkey 52 User impact if declined: can't embed standalone spidermonkey 52 Fix Landed on Version: in the process of landing Risk to taking this patch (and alternatives if risky): none String or UUID changes made by this patch: none See https://wiki.mozilla.org/Release_Management/ESR_Landing_Process for more info.

Thanks!

Pushed by ryanvm@gmail.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/ee1d59fe62a0 Mark character encoding functions as public API. r=fitzgen

Comment on attachment 8885103 [details] [diff] [review] Mark character encoding functions as public API This is needed for SpiderMonkey and low risk. Let's uplift it to ESR52.3.