[commit] language.h minor cleanup

[commit] language.h minor cleanup

[Joel Brobecker]

[-- Attachment #1: Type: text/plain, Size: 633 bytes --]

Hello,

This is something I noticed while working on the "struct parse_context"
task: There was one commented-out enum declaration that I decommented,
and also a couple of functions declarations were using struct language_defn
before that struct was defined. I moved these declarations past the
struct declaration.

2007-12-17  Joel Brobecker  <brobecker@adacore.com>

        * language.h (enum exp_opcode): Add forward declaration.
        (language_string_char_type, language_lookup_primitive_type_by_name):
        Move declaration past declaration of language_defn.

Tested on x86-linux by rebuilding GDB.
Checked in.

-- 
Joel

[-- Attachment #2: language.h.diff --]
[-- Type: text/plain, Size: 1508 bytes --]

Index: language.h
===================================================================
RCS file: /cvs/src/src/gdb/language.h,v
retrieving revision 1.43
diff -u -p -r1.43 language.h
--- language.h	4 Dec 2007 23:33:00 -0000	1.43
+++ language.h	17 Dec 2007 06:59:05 -0000
@@ -30,8 +30,7 @@ struct objfile;
 struct frame_info;
 struct expression;
 struct ui_file;
-
-/* enum exp_opcode;     ANSI's `wisdom' didn't include forward enum decls. */
+enum exp_opcode;
 
 /* This used to be included to configure GDB for one or more specific
    languages.  Now it is left out to configure for all of them.  FIXME.  */
@@ -129,13 +128,6 @@ struct language_arch_info
   struct type *string_char_type;
 };
 
-struct type *language_string_char_type (const struct language_defn *l,
-					struct gdbarch *gdbarch);
-
-struct type *language_lookup_primitive_type_by_name (const struct language_defn *l,
-						     struct gdbarch *gdbarch,
-						     const char *name);
-
 /* Structure tying together assorted information about a language.  */
 
 struct language_defn
@@ -314,6 +306,14 @@ extern enum language_mode
     language_mode_auto, language_mode_manual
   }
 language_mode;
+
+struct type *language_string_char_type (const struct language_defn *l,
+					struct gdbarch *gdbarch);
+
+struct type *language_lookup_primitive_type_by_name (const struct language_defn *l,
+						     struct gdbarch *gdbarch,
+						     const char *name);
+
 \f
 /* These macros define the behaviour of the expression 
    evaluator.  */

Re: [commit] language.h minor cleanup

[Mark Kettenis]

> Date: Mon, 17 Dec 2007 11:03:01 +0400
> From: Joel Brobecker <brobecker@adacore.com>
> 
> Hello,
> 
> This is something I noticed while working on the "struct parse_context"
> task: There was one commented-out enum declaration that I decommented,

As far as I know the comment:

> -/* enum exp_opcode;     ANSI's `wisdom' didn't include forward enum decls. */

is right, so you shouldn't have comitted this :(.  Perhaps it is now a
GCC extension to allow these, but as far as I remember older versions
of GCC didn't accept this.

In that pas we simply moved these enums to defs.h, which should always
be included before any other headers.

Mark

Re: [commit] language.h minor cleanup

[Joel Brobecker]

> As far as I know the comment:
> 
> > -/* enum exp_opcode;     ANSI's `wisdom' didn't include forward enum decls. */
> 
> is right, so you shouldn't have comitted this :(.  Perhaps it is now a
> GCC extension to allow these, but as far as I remember older versions
> of GCC didn't accept this.

I thought that we changed the requirement to C90 which should accept this.
I was comforted in this direction by the fact that we already use this
paradigm elsewhere so I thought it was OK.

-- 
Joel

Re: [commit] language.h minor cleanup

[Daniel Jacobowitz]

On Mon, Dec 17, 2007 at 02:45:47PM +0400, Joel Brobecker wrote:
> > As far as I know the comment:
> > 
> > > -/* enum exp_opcode;     ANSI's `wisdom' didn't include forward enum decls. */
> > 
> > is right, so you shouldn't have comitted this :(.  Perhaps it is now a
> > GCC extension to allow these, but as far as I remember older versions
> > of GCC didn't accept this.
> 
> I thought that we changed the requirement to C90 which should accept this.
> I was comforted in this direction by the fact that we already use this
> paradigm elsewhere so I thought it was OK.

Nope, sorry.

C99, 6.7.2.3#2:

A type specifier of the form
       enum identifier
without an enumerator list shall only appear after the type it
specifies is complete.

drow@caradoc:~% gcc -c q.c -Wall -std=c99 -pedantic
q.c:1: warning: ISO C forbids forward references to `enum' types

If we use it elsewhere, we should stop.

-- 
Daniel Jacobowitz
CodeSourcery

Re: [commit] language.h minor cleanup

[Joel Brobecker]

[-- Attachment #1: Type: text/plain, Size: 845 bytes --]

> q.c:1: warning: ISO C forbids forward references to `enum' types
> If we use it elsewhere, we should stop.

Humpf, sorry about that. I have reverted this part of my patch
as follow:

2007-12-17  Joel Brobecker  <brobecker@adacore.com>

        * language.h (enum exp_opcode): Comment out this declaration,
        undoing the previous change.

Re-tested on x86-linux with a full rebuild. The situation is not
completely fixed yet, but we can discuss our options.

In terms of other places where it is used, I thought there were
several but a quick grep revealed only two:
  - language.h (our case, enum exp_opcode)
  - symtab.h (enum language)

Unless my egrep command is wrong, I must have some delusional episodes:
        % grep -E -e 'enum +[_0-9a-zA-Z]+;' **/*.h

I will send some patches ASAP.

-- 
Joel (UTC+4 for the next 5 weeks :-).

[-- Attachment #2: language.h.diff --]
[-- Type: text/plain, Size: 609 bytes --]

Index: language.h
===================================================================
RCS file: /cvs/src/src/gdb/language.h,v
retrieving revision 1.44
diff -u -p -r1.44 language.h
--- language.h	17 Dec 2007 07:00:49 -0000	1.44
+++ language.h	17 Dec 2007 14:04:59 -0000
@@ -30,7 +30,8 @@ struct objfile;
 struct frame_info;
 struct expression;
 struct ui_file;
-enum exp_opcode;
+
+/* enum exp_opcode;     ANSI's `wisdom' didn't include forward enum decls. */
 
 /* This used to be included to configure GDB for one or more specific
    languages.  Now it is left out to configure for all of them.  FIXME.  */