Request: Please reconsider adding tag in SDL_Event

Hi,

Patch in question:

— include/SDL_events.h 20 Aug 2004 18:57:01 -0000 1.11
+++ include/SDL_events.h 19 Jan 2006 17:35:09 -0000
@@ -214,7 +214,7 @@
} SDL_SysWMEvent;

/* General event structure */
-typedef union {
+typedef union SDL_Event {
Uint8 type;
SDL_ActiveEvent active;
SDL_KeyboardEvent key;

Reasoning:----------

  1. Allows forward declaration of the SDL_Event union in C++. Please
    note that in plain C it is possible to forward declare it.

  2. Forward declaration is good because it allows encapsulation. It hides
    the specific implementation and does not necessarily exposes SDL staff
    to my appication’s namespace

  3. It can’t harm plain C because tags are living in a different namespace
    than typenames

  4. It is already done like this in other places in SDL. Check for example
    SDL_KeySym, and SDL_.*Event structures.

  5. Right now I have to include SDL/event.h from a C++ header file. See 2)

short FAQ:

Q1: The code as it is currently does not compile?
A: No it compiles fine. I just don’t want to include it in my public header
if I can avoid it.

Consider the following minimal example. It is valid
C but not valid C++.

---------- cut here -----------
/* this is a forward declaration */
union Foo;

/* and here is the complete type */
typedef union {
int i;
float f;
double d;
} Foo;

int main()
{
return 0;
}
---------- cut here -----------

The error coming from g++ is
fdecl.c:7: error: conflicting declaration 'typedef union Foo Foo’
fdecl.c:1: error: ‘union Foo’ has a previous declaration as ‘union Foo’

If you change it to
typedef union Foo {

} Foo;

It compiles fine under g++ 4.0.1 (Debian sarge)
------------------

Q2: Are you sure you know the difference between type and tag in C? in C++?
A: I think so. Types and tags are different animals in C, but C++ treats them
the same. Problem is: in C++ you forward declare a previously typedefed type
unless it has a tag associated with it (gross oversimplification).
------------------

Q3: I don’t see a problem here.
A: Please then do it for uniformity and consistency. Consider the following
fragment from SDL sources

/* Mouse button event structure /
typedef struct SDL_MouseButtonEvent {
Uint8 type; /
SDL_MOUSEBUTTONDOWN or SDL_MOUSEBUTTONUP /
Uint8 which; /
The mouse device index /
Uint8 button; /
The mouse button index /
Uint8 state; /
SDL_PRESSED or SDL_RELEASED /
Uint16 x, y; /
The X/Y coordinates of the mouse at press time */
} SDL_MouseButtonEvent;

Here both the type and the tag have the same name SDL_MouseButtonEvent. Why not
also the SDL_Event union?
------------------

Q4: For such a small patch, do you actually need all these arguments?
A: Well, I think it’s the right thing to do and I am trying to prove it by
technical arguments. Since this the second time I try it I would like to have
done my best effort to be understandable, before the SDL developers shot me down.
------------------

 .bill

Patch in question:

— include/SDL_events.h 20 Aug 2004 18:57:01 -0000 1.11
+++ include/SDL_events.h 19 Jan 2006 17:35:09 -0000
@@ -214,7 +214,7 @@
} SDL_SysWMEvent;

/* General event structure */
-typedef union {
+typedef union SDL_Event {
Uint8 type;
SDL_ActiveEvent active;
SDL_KeyboardEvent key;

Thanks! This is in CVS.

-Sam Lantinga, Senior Software Engineer, Blizzard Entertainment

Thanks! This is in CVS.

The guy wrote a FAQ for a one-line patch. I feel bad about not
committing it to CVS before. :slight_smile:

–ryan.