393364 - mozilla-central: build fails in js/src, "uninitialized const 'DoubleHashOps'" (C/C++ incompatibility)

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Jason Orendorff [:jorendorff]]

Attachment: v1

In trunk, this is mostly harmless: it means you can't build JS with a C++ compiler.  In mozilla-central, it breaks the build.  It should be fixed in trunk, though, and merged downstream.

cvs-trunk-mirror/js/src$ make CC=g++ -f Makefile.ref
...
g++ -o Darwin_DBG.OBJ/jsatom.o -c -Wall -Wno-format -g -DXP_UNIX -DSVR4 -DSYSV -D_BSD_SOURCE -DPOSIX_SOURCE -DDARWIN -DX86_LINUX  -DDEBUG -DDEBUG_jason -DEDITLINE -IDarwin_DBG.OBJ  jsatom.c
jsatom.c:244: error: uninitialized const 'DoubleHashOps'
jsatom.c:245: error: uninitialized const 'StringHashOps'
jsatom.c:317: error: redefinition of 'const JSDHashTableOps DoubleHashOps'
jsatom.c:244: error: 'const JSDHashTableOps DoubleHashOps' previously declared here
jsatom.c:328: error: redefinition of 'const JSDHashTableOps StringHashOps'
jsatom.c:245: error: 'const JSDHashTableOps StringHashOps' previously declared here
make[1]: *** [Darwin_DBG.OBJ/jsatom.o] Error 1
make[1]: Leaving directory `/Users/jason/dev/am/cvs-trunk-mirror/js/src'
make: *** [all] Error 2

I think C++ forbids this as a consequence of the One-Definition Rule.  In any case, rearranging things a little seems to appease the compiler.

Comment 1

[Igor Bukanov]

Comment on attachment 277869 [details] [diff] [review]
v1

>-static const JSDHashTableOps DoubleHashOps;
>-static const JSDHashTableOps StringHashOps;
>+JS_STATIC_DLL_CALLBACK(JSDHashNumber)
>+HashDouble(JSDHashTable *table, const void *key);
>+
>+JS_STATIC_DLL_CALLBACK(JSBool)
>+MatchDouble(JSDHashTable *table, const JSDHashEntryHdr *hdr, const void *key);
>+
>+static const JSDHashTableOps DoubleHashOps = {
>+    JS_DHashAllocTable,
>+    JS_DHashFreeTable,
>+    HashDouble,
>+    MatchDouble,
>+    JS_DHashMoveEntryStub,
>+    JS_DHashClearEntryStub,
>+    JS_DHashFinalizeStub,
>+    NULL
>+};
>+
>+JS_STATIC_DLL_CALLBACK(JSDHashNumber)
>+HashString(JSDHashTable *table, const void *key);
>+
>+JS_STATIC_DLL_CALLBACK(JSBool)
>+MatchString(JSDHashTable *table, const JSDHashEntryHdr *hdr, const void *key);

Nit: move HashString and MatchString before DoubleHashOps to group similar-purpose code together. r+ with that fixed.

Comment 2

[Jason Orendorff [:jorendorff]]

Attachment: v1a

Sorry, knee-jerk OOP on my part.  :)

Comment 3

[Jason Orendorff [:jorendorff]]

Igor, I'm not a CVS committer; could you please check this in (presuming it's approval1.9+)?

Thanks for the speedy review.

Comment 4

[Brendan Eich [:brendan]]

Comment on attachment 277900 [details] [diff] [review]
v1a

Pre-approving, letting Igor mark and land. Thanks,

/be

Comment 5

[Igor Bukanov]

I checked in the patch from comment 2 to the trunk:

http://bonsai.mozilla.org/cvsquery.cgi?module=PhoenixTinderbox&branch=HEAD&cvsroot=%2Fcvsroot&date=explicit&mindate=1187895720&maxdate=1187895780&who=igor%mir2.org