Closed
Bug 81385
Opened 24 years ago
Closed 9 years ago
extern vs. non extern warnings.
Categories
(Core :: XPConnect, defect)
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]
Comment 1•24 years ago
|
||
Looks like a bogus warning to me. This is shaver's code.
Assignee: jband → shaver
Comment 2•24 years ago
|
||
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]
Comment 3•24 years ago
|
||
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'?
Comment 4•24 years ago
|
||
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.
Comment 5•24 years ago
|
||
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.
Comment 6•23 years ago
|
||
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
Comment 8•22 years ago
|
||
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
Updated•22 years ago
|
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)
I commented on http://gcc.gnu.org/bugzilla/show_bug.cgi?id=8680 to see if
someone would resurrect it.
![]() |
||
Comment 11•19 years ago
|
||
Technique 1 seemed to compile on all our tinderboxes, including ports.
Updated•18 years ago
|
QA Contact: pschwartau → xpconnect
Updated•14 years ago
|
Whiteboard: [~1600 warnings all bad] → [build_warning][~1600 warnings all bad]
Comment 12•13 years ago
|
||
Ginn, Can you please share current build warning logs so that we can work on removing them?
Comment 13•13 years ago
|
||
Comment 14•13 years ago
|
||
Comment 15•9 years ago
|
||
Ginn: are these extern "C" warnings still an issue on SPARC?
Assignee: timeless → nobody
Status: ASSIGNED → NEW
Flags: needinfo?(ginn.chen)
Comment 16•9 years ago
|
||
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)
Comment 17•9 years ago
|
||
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.
Description
•