935395 - [10.9] plugin_child_interpose.mm: [-Wmissing-declarations] `typedef struct Cursor` declaration does not declare anything

Status: RESOLVED FIXED

(Core Graveyard :: Plug-ins, defect)

Description

[Chris Peterson [:cpeterson]]

Attachment: Cursor-declaration.patch

When building a 32-bit debug build on OS X 10.9, I see the following warning:

dom/plugins/ipc/interpose/plugin_child_interpose.mm:40:1 [-Wmissing-declarations] `typedef struct Cursor` declaration does not declare anything

Comment 1

[Steven Michaud [:smichaud] (Retired)]

Did your patch compile?  I don't think it should.

Without "typedef struct Cursor;", how does the compiler know what a "Cursor" object is in the next line?

And even if your patch does compile on Mavericks, I suspect it won't on other versions of OS X.  You might want to do a tryserver run.

Comment 2

[Chris Peterson [:cpeterson]]

Yes, the patch compiles for me. The warning is complaining about combining a typedef with a forward declaration of a struct. My patch replaces the superfluous typedef with a normal forward declaration of `struct Cursor`.

Comment 3

[Steven Michaud [:smichaud] (Retired)]

Could you please start a Mac tryserver run for your patch?  "try: -b do -p macosx,macosx64 -u none" should do it.

Comment 4

[Chris Peterson [:cpeterson]]

Here's a green tryserver build:

https://tbpl.mozilla.org/?tree=Try&rev=a04edd59f7c1

Comment 5

[Steven Michaud [:smichaud] (Retired)]

OK, I've now done some tests of my own.  I found that your syntax works in C++ files, but fails (in the way I expected) in C files.

struct Blah;
extern void whatever(Blah blah);

int main(int argc, char *argv[])
{
  return 0;
}

Put this in a file called test.cpp or test.mm, and it compiles just fine with either gcc or clang.

But name the file test.c or test.m, and you get errors like the following:

test.c:2: error: expected ‘)’ before ‘blah’

test.c:2:22: error: must use 'struct' tag to refer to type 'Blah'
extern void whatever(Blah blah);
                     ^
                     struct 

I don't know the C and C++ standards well enough to know why this works in C++ but not in C.  But this behavior is so consistent (I tested gcc back to OS X 10.5) that I don't think it can be just a mistake or a bug.

Given that it's so unexpected, though (at least to an old C hack like me), I think you should add a comment to your patch explaining that your syntax, though legal in C++, is illegal in C.

Comment 6

[Steven Michaud [:smichaud] (Retired)]

Comment on attachment 827865 [details] [diff] [review]
Cursor-declaration.patch

Please add a comment explaining that this syntax is legal in C++ but illegal in C.

Comment 7

[Steven Michaud [:smichaud] (Retired)]

> I don't know the C and C++ standards well enough to know why this works in C++ but not in C.

If *you* know them well enough, please cite the specific rule (or rules) here (in this bug) and leave out the comment :-)

Comment 8

[Chris Peterson [:cpeterson]]

Thanks for double-checking! Both C and C++ support forward declarations of structs like `struct Cursor`, but C require struct types to be referenced using the full name `struct Cursor` like `void SetCursor(struct Cursor* cursor)`.

This code's `typedef struct Cursor` was trying to 1. forward declare `struct Cursor` and 2. typedef struct Cursor to just `Cursor` so C code could just use `void SetCursor(Cursor* cursor)`. If this code was in a header file that need to be compatible with both C and C++ (like the QuickdrawAPI.h header file this code was polyfilling), then a compatible, warning-free change would be `typedef struct Cursor Cursor;`.

Fortunately, this is an Objective-C++ .mm file, we no longer need to worry about C compatibility. <:)

Comment 9

[Chris Peterson [:cpeterson]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/c2ace0287514

Comment 10

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/c2ace0287514