Have had some issues building gjs against a debug build of standalone js24. It only works if I define DEBUG during the gjs build. Everything is working fine for non-debug builds of js24. It fails with lots of undefined references the root cause being the following snippet https://gist.github.com/anonymous/7510955 I don't really see why the definition of JSVAL/JSID structs is tied to debug builds. Embedders really shouldnt need to know if mozjs was build with debug...

Are you compiling with DEBUG but linking to a library not compiled with DEBUG, or vice versa? > I don't really see why the definition of JSVAL/JSID structs is tied to debug builds. In a non-debug build, jsid is an integer so it'll be fast. In a debug build it's a struct so it'll be easier to debug.

js24 is built with DEBUG, gjs itself doesnt really have a DEBUG build as such. Perhaps DEBUG should be set via pkg-config?

I talked with Jim Blandy about this. The law we are breaking is: Every #define that affects the meaning of public headers must be in js-config.h. One way to fix this: * Change DEBUG to JS_DEBUG throughout js/src (it appears on something like 600 lines, no big deal) * Add an entry to js/src/js-config.h.in for JS_DEBUG * Change configure.in to do AC_DEFINE(JS_DEBUG) if passed --enable-debug. Things to look out for, though: * DEBUG is also used under js/src/assembler, which I'm not sure we ever touch, as a policy matter * it's unclear where in configure.in we even detect --enable-debug or define DEBUG * there is also this MOZ_DEBUG thing, what is that, nobody knows * the build system is a living thing called forth by the gods to punish the hubris of mortals Inquiring further.

(In reply to Jason Orendorff [:jorendorff] from comment #3) > * DEBUG is also used under js/src/assembler, which I'm not sure we ever > touch, as a policy matter js/src/assembler diverges enough from WebKit tip that we don't keep it in sync anymore.

OK, update regarding the "things to look out for": * js/src/assembler: see comment 4 * --enable-debug is detected in build/compiler-opts.m4, which defines MOZ_DEBUG * But MOZ_DEBUG is only set in configure and in Makefiles. So what we would need to do is add some code in configure.in to AC_DEFINE(JS_DEBUG) if and only if $MOZ_DEBUG is nonempty. * wrath of Poseidon: still a consideration So this looks doable, if anyone is willing to take it.

I'll write a patch tomorrow if nobody volunteers before then... Do -all- of the '#ifdef DEBUG's need to be swapped or just those in header files?

Good question. All the public header files must be changed. That means every header mentioned in EXPORTS or EXPORTS.js in js/src/moz.build. In particular it includes js/public/*.h. All those files should #include "js-config.h", directly or indirectly; most already do indirectly via "jstypes.h", but it appears js/public/TypeDecls.h does not. Please just add #include "js-config.h" there.

Holding off on the mozjs24 release until this bug is resolved.

(In reply to Ian Stakenvicius from comment #6) > I'll write a patch tomorrow if nobody volunteers before then... Do -all- of > the '#ifdef DEBUG's need to be swapped or just those in header files? Ian, are you working on this patch? I'd be happy to help if you are not.

Am I correct that this is an issue now, where it wasn't in esr17, because in esr17 the debug-ness distinction was only applied for embeddings using C++? When I first saw this bug I was a bit surprised, because I'd remembered jsid being this way for a pretty long time.

This is what i've got so far -- it's against HEAD, and is untested but it should work according to the earlier discussion. I think this might change a lot more headers than is necessary though; we should be able to cut down the changes to .h files to just those in js/src/ and the 'mozilla' subdir. I will work on it more Monday if nobody else takes over in the meantime.

This patch defines and uses JS_DEBUG in public headers. For completeness, it also lets you link against a JS_DEBUG library built with JS_NO_JSVAL_JSID_STRUCT_TYPES. It applies against mozilla-central. I kept the patch small by only changing exported headers, so that it's easier to backport to ESR24. If this patch is OK, it's probably a good idea to proceed to convert the rest of js/ to JS_DEBUG, so that we don't have two systems.

Comment on attachment 8344053 [details] [diff] [review] Use JS_DEBUG in public headers. Review of attachment 8344053 [details] [diff] [review]: ----------------------------------------------------------------- Follow-up bug for the rest?

https://hg.mozilla.org/integration/mozilla-inbound/rev/83160d1d58c8 Yeah, follow-up for the rest.

Patch for ESR24. Doesn't need to land, since the mozjs standalone release can easily carry it separately.

Sean, we have a XPCOM component that was including jsapi.h file and due to this change our code won't compile anymore. Following is the error I am seeing. include/js/RootingAPI.h: In constructor 'JS::Rooted<T>::Rooted(JSContext*, const mozilla::detail::GuardObjectNotifier&)': include/js/RootingAPI.h:691:9: error: 'IsInRequest' is not a member of 'js' As a workaround I defined JS_DEBUG flag before including jsapi.h but we shouldn't have to do that.

Anshul, does changing RootingAPI.h:535 to "#if defined(JS_THREADSAFE) && defined(JS_DEBUG)" resolve the issue? The problem is that assertions are still activated by DEBUG, even when JS_DEBUG isn't defined.

Sean, line 535 in my workspace is below, which doesn't seem to make sense. Please provide the code section around the line you want me to change.

line 535: #ifdef JSGC_GENERATIONAL JS_PUBLIC_API(void) HeapCellPostBarrier(js::gc::Cell **cellp); JS_PUBLIC_API(void) HeapCellRelocate(js::gc::Cell **cellp); #endif

Patch in bug 949195 fixes the issue. Thanks.