Closed Bug 81385 Opened 24 years ago Closed 9 years ago

extern vs. non extern warnings.

Categories

(Core :: XPConnect, defect)

Sun
Solaris
defect
Not set
normal

Tracking

()

RESOLVED WONTFIX
Future

People

(Reporter: timeless, Unassigned)

References

(Blocks 1 open bug)

Details

(Keywords: arch, helpwanted, Whiteboard: [build_warning][~1600 warnings all bad])

Attachments

(2 files)

"/tmp/mozilla/js/src/xpconnect/src/xpcstring.cpp", line 190: Warning (Anachronism): Formal argument finalizer of type extern "C" void(*)(JSContext*,JSString*) in call to JS_AddExternalStringFinalizer(extern "C" void(*)(JSContext*,JSString*)) is being passed void(*)(JSContext*,JSString*). "/tmp/mozilla/js/src/xpconnect/src/xpcstring.cpp", line 222: Warning (Anachronism): Formal argument etor of type extern "C" JSDHashOperator(*)(JSDHashTable*,JSDHashEntryHdr*,unsigned,void*) in call to JS_DHashTableEnumerate(JSDHashTable*, extern "C" JSDHashOperator(*)(JSDHashTable*,JSDHashEntryHdr*,unsigned,void*), void*) is being passed JSDHashOperator(*)(JSDHashTable*,JSDHashEntryHdr*,unsigned,void*). "/tmp/mozilla/js/src/xpconnect/src/xpcstring.cpp", line 226: Warning (Anachronism): Formal argument finalizer of type extern "C" void(*)(JSContext*,JSString*) in call to JS_RemoveExternalStringFinalizer(extern "C" void(*)(JSContext*,JSString*)) is being passed void(*)(JSContext*,JSString*). 3 Warning(s) detected. xpcthreadcontext.cpp this stuff happens a lot. I'm trying to get a tinderbox running so people can access my logs. [Solaris7Sparc Sun ForteC5 Mozilla:Xlib]
Looks like a bogus warning to me. This is shaver's code.
Assignee: jband → shaver
There are 1600 of these warnings and none of them are bogus. According to the C/C++ spec, if you say that a function is extern "C" and receives extern "C" functions, then everything you pass must be extern "C". All throughout Mozilla, we pass static C++ member functions to things that are not supposed to get them. Fixing this is a real monster, probably a post 1.0 issue. Every static C++ function that has this problem must be pulled out of the class, usually made a friend, and then put in an extern "C" block. This also means that they all become extern rather than static because you cannot have both (see http://bugzilla.mozilla.org/show_bug.cgi?id=74224). So we should mark this a future and keep it on the radar.
Keywords: arch, helpwanted
Whiteboard: [~1600 warnings all bad]
mkaply: OK, fine. They are not bogus. What you did not say is what is broken by this pattern. Does this cause some platform to not work? Is this only a correctness issue? What is the 'badnes'?
This could potentially break any platform where C and C++ had different calling conventions, which is explicitly allowed. Bill Law works on such a platform, I believe it's OS2 or perhaps AIX.
Although at this point it is totally a correctness issue. It was an OS/2 issue a long time ago, but we solved it differently. Rather than: extern "C" { void foo() } We did void PR_CALLBACK foo() which accomplished the same thing - getting code to compile, and having the right calling convention. We did this because we didn't understand that the core of the problem was the extern "C" mismatch, not the calling convention mismatch. However, even if the extern "C" is put in properly we would still need the PR_CALLBACK, though because for correctness you should put the calling convention on the API to guarantee things match. So we know of the problem, and when new stuff arises we want to do it the right way, but I don't know of anyone that wants to go through the trouble of fixing this when there are so many other things to worry about.
Handing off to timeless, so he can produce a list from his build and put it up somewhere for people to pay really close attention to. Like all our other warnings.
Assignee: shaver → timeless
I should have done this *before* I lost access to my sparcs. I wonder if I could use dcran's or just get their output.
Status: NEW → ASSIGNED
Target Milestone: --- → Future
Depends on: 92571
timeless, Sure thing, I have Forte 7.0 Fully licensed now, so All you need is to pull a branch, get in touch with me and Ill set you up ... dcran
Blocks: buildwarning
Looking at the C++ standard (the out-of-date 1998 version, I admit), as far as I can tell a conformant compiler is *required* to give a diagnostic for this. In particular, section 7.5 [dcl.link] clause 1 says that function types with different linkage are different types, section 4 [conv] does not describe any standard conversions that could convert between them, and section 5.2.2 [expr.call] clause 4 and section 8.5 [dcl.init] clauses 12 and 14 seem to require that such a conversion exist. It seems like there are two workarounds for this: 1) Especially when using static member functions as the callbacks, those functions can be declared using a typedef that is declared in a manner like: extern "C" typedef void (PR_CALLBACK FUNC)(); /* Note no "*". C++ makes a distinction between function types and pointer-to-function types, and provides for implicit conversion from the former to the latter. C doesn't seem to make that distinction (see C99 (ISO/IEC 9899:1999), section 6.3.2.1, clause 4). Our type declarations in C all seem to use the "*" form; I'm not sure what's allowed in C and what's portable (i.e., whether we could change it within C files) */ /* I hope that use of PR_CALLBACK works! */ as something like: static FUNC MyHandler; and then defined as: void PR_CALLBACK Class::MyHandler() { ... } or void PR_CALLBACK MyHandler() { ... } 2) (This doesn't work for static member functions.) The function could be initially declared within an extern "C" block, and then defined: extern "C" { PR_STATIC_CALLBACK(void) MyHandler(); } void PR_CALLBACK MyHandler() { ... } bz may test whether technique (1) is portable when he lands bug 294114 (see bug 294114 comment 11 and on)
Depends on: 294114
Technique 1 seemed to compile on all our tinderboxes, including ports.
QA Contact: pschwartau → xpconnect
Whiteboard: [~1600 warnings all bad] → [build_warning][~1600 warnings all bad]
Ginn, Can you please share current build warning logs so that we can work on removing them?
Ginn: are these extern "C" warnings still an issue on SPARC?
Assignee: timeless → nobody
Status: ASSIGNED → NEW
Flags: needinfo?(ginn.chen)
For Firefox/Thunderbird after 31.0, I've switched to GNU compiler. And I don't think warnings are priority for Solaris platform now.
Flags: needinfo?(ginn.chen)
Thanks, Ginn. I just wanted to make sure before I retire this bug.
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: