Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix debug build #5

Merged
merged 2 commits into from Sep 4, 2013
Merged

Fix debug build #5

merged 2 commits into from Sep 4, 2013

Conversation

@kmcallister
Copy link
Contributor

kmcallister commented Sep 4, 2013

I tried to do this within Servo's mk/config.mk but CFLAGS_mozjs there seems to have no effect.

kmcallister added 2 commits Sep 4, 2013
Should cut down compile time a little bit.
SpiderMonkey defines 'jsid' to be a struct type in debug builds, which changes
the mangled names of some functions, producing a linker error.

This feature serves only to catch mistakes in the SpiderMonkey C++ code, so
for Servo we just disable it.
@metajack

This comment has been minimized.

Copy link

metajack commented on bac8c7f Sep 4, 2013

Good idea!

@metajack

This comment has been minimized.

Copy link

metajack commented on js/src/Makefile.in in 5aef769 Sep 4, 2013

This seems odd. What kind of linker error? Should this be a FIXME?

This comment has been minimized.

Copy link
Owner Author

kmcallister replied Sep 4, 2013

Consider

js::BaseProxyHandler::hasOwn(JSContext*, JSObject*, jsid id, bool*)

In a release build, jsid is a typedef for ptrdiff_t which is a typedef for long, so the symbol name for this function is the mangling of

js::BaseProxyHandler::hasOwn(JSContext*, JSObject*, long, bool*)

In a debug build it's a struct and the name mentions jsid. But the glue in rust-mozjs/jsglue.cpp expects only the former.

Presumably there's a way to make the rust-mozjs build see the same defines such that it all works out in both build modes. But I didn't want to do more Makefile yak-shaving to preserve a debug feature that's of no use to us anyway.

@metajack

This comment has been minimized.

Copy link

metajack commented on 5aef769 Sep 4, 2013

r+

metajack added a commit that referenced this pull request Sep 4, 2013
@metajack metajack merged commit 15c494b into servo:master Sep 4, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants
You can’t perform that action at this time.