Created attachment 71080 [details] [review]
patch

2006-08-26  Matthias Clasen  <mclasen@redhat.com>

	* glib/gutils.h: 
	* glib/gscanner.c: Fix some typos.  (#351741, Kjartan Maraas)

> -G_DEFINE_TYPE (GInitiallyUnowned, g_initially_unowned, G_TYPE_OBJECT);
> +G_DEFINE_TYPE (GInitiallyUnowned, g_initially_unowned, G_TYPE_OBJECT)

this is bogus, G_DEFINE_TYPE() macros are supposed to be ended by a ';' (as pretty much every other definition macro).
not doing this severely screws up documentation parses, indentation in editors, some linters and maybe other tools.
(backing out this change)

Look at the definition ... G_DEFINE_TYPE() ends with a function, so
can't have a ; after it .. that's not legal C.

And you certainly can't add some sort of hack to G_DEFINE_TYPE that
would make it *require* a trailing ;, there are thousands of uses
out there already without the ';'

I haven't seen any editors or other tools get fundamentally confused 
by the lack of a semicolon after G_DEFINE_TYPE ... 

Tim, 

G_DEFINE_TYPE is used without the semicolon throughout GTK+

(In reply to comment #4)
> Look at the definition ... G_DEFINE_TYPE() ends with a function, so
> can't have a ; after it .. that's not legal C.
> 
> And you certainly can't add some sort of hack to G_DEFINE_TYPE that
> would make it *require* a trailing ;, there are thousands of uses
> out there already without the ';'
> 
> I haven't seen any editors or other tools get fundamentally confused 
> by the lack of a semicolon after G_DEFINE_TYPE ... 

well, i have. emacs is one example, simply run c-indent-command (Tab) after such a statement.
doxygen is another, it also gets majorly confused by G_BEING_DECLS because it's used without a trailing ; in so many places.

note that there is no way to "fix" these tools/editors, because without a semicolon, C/C++ syntax simply becomes ambiguous in the absence of macro expansion. and macro expansion is usually not desirable for these cases (e.g. syntax indenting/highlighting in editors).

(In reply to comment #5)
> Tim, 
> 
> G_DEFINE_TYPE is used without the semicolon throughout GTK+

well, unfortunately then. being used in many places doesn't make this right though, the problems still persist.

if you intend to insist on removing these semicolons which most modern compilers ignore anyway, we can of course deprecate G_DEFINE_TYPE() in favour of a variant that has 'some sort of hack to G_DEFINE_TYPE that would make it *require* a trailing ;'
 ;-)

(In reply to comment #6)
> (In reply to comment #4)
> > Look at the definition ... G_DEFINE_TYPE() ends with a function, so
> > can't have a ; after it .. that's not legal C.
> > 
> > And you certainly can't add some sort of hack to G_DEFINE_TYPE that
> > would make it *require* a trailing ;, there are thousands of uses
> > out there already without the ';'
> > 
> > I haven't seen any editors or other tools get fundamentally confused 
> > by the lack of a semicolon after G_DEFINE_TYPE ... 
> 
> well, i have. emacs is one example, simply run c-indent-command (Tab) after
> such a statement.
> doxygen is another, it also gets majorly confused by G_BEING_DECLS because it's
> used without a trailing ; in so many places.

Yes, I've had this problem with 'indent' too.

> note that there is no way to "fix" these tools/editors, because without a
> semicolon, C/C++ syntax simply becomes ambiguous in the absence of macro
> expansion. and macro expansion is usually not desirable for these cases (e.g.
> syntax indenting/highlighting in editors).

Actually there is.  They simply can recognize an empty line as a separator.  No decorator usage I've seen uses an empty line in between.


> (In reply to comment #5)
> > Tim, 
> > 
> > G_DEFINE_TYPE is used without the semicolon throughout GTK+
> 
> well, unfortunately then. being used in many places doesn't make this right
> though, the problems still persist.

This was discussed a few months ago when kmaraas submitted patches to remove all the semi-colons.  The conclusion was that if there are compilers that don't like the semi-colon after function declaration, but accept double semi-colons, we can add a dummy prototype after G_DEFINE_TYPE and similar functions.

I don't think a compiler would be any more likely to accept a stray
semi-colon in one place than the other :-) Without referring to the 
C standard, I'm pretty sure both are equally invalid.

Emacs at least will be confused on the next line, but fine for
the rest of the file, so it isn't a big deal. I'm not a fan of running 
indent over entire files, so I can't comment there.

(In reply to comment #7)
> (In reply to comment #6)
> > note that there is no way to "fix" these tools/editors, because without a
> > semicolon, C/C++ syntax simply becomes ambiguous in the absence of macro
> > expansion. and macro expansion is usually not desirable for these cases (e.g.
> > syntax indenting/highlighting in editors).
> 
> Actually there is.  They simply can recognize an empty line as a separator.
> No decorator usage I've seen uses an empty line in between.

well, you don't want indent/doxygen/emacs/et all to generally treat empty lines as statement seperators though.
as i said, the problem is not to detect the 4 macro cases we currently have and work around them (and if it were, every C -like parsing tool had to special case our macros), but the ambiguity that is introduced into the syntax.
the proper fix here really is to use macros according to standard C/C++ syntax (i.e. with '()' at the end, not like G_BEGIN_DECLS, and delimited by ';').

> > (In reply to comment #5)
> > > Tim, 
> > > 
> > > G_DEFINE_TYPE is used without the semicolon throughout GTK+
> > 
> > well, unfortunately then. being used in many places doesn't make this right
> > though, the problems still persist.
> 
> This was discussed a few months ago when kmaraas submitted patches to remove
> all the semi-colons.

hm, sorry, i missed that.

>  The conclusion was that if there are compilers that don't
> like the semi-colon after function declaration, but accept double semi-colons,
> we can add a dummy prototype after G_DEFINE_TYPE and similar functions.

that is solving a different problem though.
if there's a discussion on the issue already though, it's worth resurrecting that, since there's more than just compilers out there that need the ability to parse C syntax (and in the absence of macro expansion).

(In reply to comment #8)
> I don't think a compiler would be any more likely to accept a stray
> semi-colon in one place than the other :-) Without referring to the 
> C standard, I'm pretty sure both are equally invalid.

i don't think that's accurate, i have a pretty strong memory that double semicolons _inside_ of function bodies are explicitely allowed (might be C99 specifying that or so).

> Emacs at least will be confused on the next line, but fine for
> the rest of the file, so it isn't a big deal.

i can't agree to this. i completely rely on emacs doing all my indentaiton for me, and i have key setups that properly reindent whole files for me. 
it's definitely not affordable for me for the syntax indenter to screw up in any place.

> I'm not a fan of running 
> indent over entire files, so I can't comment there.

well, you asked for programs that get screwed by this and these were just examples. there're surely others out there (mono also has a C parser somewhere iirc). e.g. to point out doxygen again, it usually annotates every first function of a C file with the return type "G_BEGIN_DECLS real_return_type".

basically, these issues can only be fixed by moving back to proper C compatible syntax, even so for G_BEGIN_DECLS*, regardless of how many files need fixing and how long it takes to go through a slow deprecation process.

(luckily the required changes are very easy to script)

(In reply to comment #10)
> (In reply to comment #8)
> > I don't think a compiler would be any more likely to accept a stray
> > semi-colon in one place than the other :-) Without referring to the 
> > C standard, I'm pretty sure both are equally invalid.
> 
> i don't think that's accurate, i have a pretty strong memory that double
> semicolons _inside_ of function bodies are explicitely allowed (might be C99
> specifying that or so).

Gcc says "ISO C does not allow extra ‘;’ outside of a function" for both, but only with -pedantic.


> > I'm not a fan of running 
> > indent over entire files, so I can't comment there.
> 
> well, you asked for programs that get screwed by this and these were just
> examples. there're surely others out there (mono also has a C parser somewhere
> iirc). e.g. to point out doxygen again, it usually annotates every first
> function of a C file with the return type "G_BEGIN_DECLS real_return_type".

Now that you sayd, I remember, c2man too.
 
> basically, these issues can only be fixed by moving back to proper C compatible
> syntax, even so for G_BEGIN_DECLS*, regardless of how many files need fixing
> and how long it takes to go through a slow deprecation process.

So, something like:

#define G_DECLS_BEGIN() extern "C" {/*}*/ void g_dummy_prototype(void)

(In reply to comment #11)
> Gcc says "ISO C does not allow extra ‘;’ outside of a function" for both,
> but only with -pedantic.

that's it, thanks.

> > basically, these issues can only be fixed by moving back to proper C compatible
> > syntax, even so for G_BEGIN_DECLS*, regardless of how many files need fixing
> > and how long it takes to go through a slow deprecation process.
> 
> So, something like:
> 
> #define G_DECLS_BEGIN() extern "C" {/*}*/ void g_dummy_prototype(void)

yeah, we have something like that already actually:
gthread.h:extern void glib_dummy_decl (void);