From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 25884 invoked by alias); 28 Feb 2006 06:43:11 -0000 Received: (qmail 25871 invoked by uid 22791); 28 Feb 2006 06:43:08 -0000 X-Spam-Check-By: sourceware.org Received: from xproxy.gmail.com (HELO xproxy.gmail.com) (66.249.82.201) by sourceware.org (qpsmtpd/0.31) with ESMTP; Tue, 28 Feb 2006 06:43:04 +0000 Received: by xproxy.gmail.com with SMTP id s13so685777wxc for ; Mon, 27 Feb 2006 22:43:02 -0800 (PST) Received: by 10.70.63.1 with SMTP id l1mr304424wxa; Mon, 27 Feb 2006 22:43:02 -0800 (PST) Received: by 10.70.125.17 with HTTP; Mon, 27 Feb 2006 22:43:02 -0800 (PST) Message-ID: <8f2776cb0602272243y25582156x16534fd14028898c@mail.gmail.com> Date: Tue, 28 Feb 2006 13:53:00 -0000 From: "Jim Blandy" To: "Gaius Mulley" Subject: Re: Enhanced language support for Modula-2 Cc: gdb-patches@sources.redhat.com In-Reply-To: <87mzgc9rug.fsf@j228-gm.comp.glam.ac.uk> MIME-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable Content-Disposition: inline References: <87vevg9puv.fsf@j228-gm.comp.glam.ac.uk> <8f2776cb0602151619w5fd8f043u3e7227e27f3567a9@mail.gmail.com> <20060220150513.GB14155@nevyn.them.org> <8f2776cb0602201322o3791841dv2916e53181e9f308@mail.gmail.com> <87lkw5qaa9.fsf@j228-gm.comp.glam.ac.uk> <8f2776cb0602211055t572223ecs93e37d5575a31ce@mail.gmail.com> <87wtfjfwmu.fsf@j228-gm.comp.glam.ac.uk> <8f2776cb0602252137q7e716ffaibd7b88eeb0d16e43@mail.gmail.com> <87mzgc9rug.fsf@j228-gm.comp.glam.ac.uk> X-IsSubscribed: yes Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2006-02/txt/msg00505.txt.bz2 On 28 Feb 2006 01:47:51 +0000, Gaius Mulley wrote: > sure, ok. Here is the patch again with all issues addressed from the > last set of email responses. Hi, Gaius. Thanks for the revisions. - It's true that there are a few regression failures that are intermittent; these are an annoyance that we haven't gotten around to fixing for a long time. The way I generally look for regressions is to run the test suite before and after, and compare the two gdb.sum files using diff -u. I ignore differences in the thread tests, or in the signal-handling tests in the annotation test scripts. If you find other differences and you're not sure if they're regressions or not, post them here. - You need to post a ChangeLog entry along with your patch. I have some futher comments mixed in with the patch below. > --- latest-cvs-gdb/src-cvs/gdb/m2-typeprint.c 2005-12-17 22:34:01.00000= 0000 +0000 > +++ latest-cvs-gdb/src-m2/gdb/m2-typeprint.c 2006-02-27 23:21:59.00000= 0000 +0000 ... > + case TYPE_CODE_ENUM: > + m2_type_print_modifier (type, stream, 0, 1); > + /* HP C supports sized enums */ > + if (deprecated_hp_som_som_object_present) Adding new references to deprecated variables or functions is frowned on. These are bits of interface that we've decided we'd like to get rid of over time; every new use is another case that someone trying to remove the variable/function needs to consider and understand. Also, this code looks like it's cut-and-paste from c-typeprint.c; does HP C (also known as aCC) generate Modula-2 code? Can't all this just go away? In general, I'm surprised at the structure of the type printing code.=20 All this varspec prefix and varspec suffix crud is there to deal with C's weird pseudo-algebraic type syntax, where the parse tree is the reverse of the natural type tree. Surely Modula-2 doesn't imitate that --- does it? Couldn't Modula's types be printed with a straightforward tree walk? Apologies if you've explained this before. > --- latest-cvs-gdb/src-cvs/gdb/dwarf2read.c 2006-02-09 18:18:41.00000= 0000 +0000 > +++ latest-cvs-gdb/src-m2/gdb/dwarf2read.c 2006-02-27 23:55:12.00000= 0000 +0000 > @@ -4728,10 +4743,17 @@ > code =3D TYPE_CODE_FLT; > break; > case DW_ATE_signed: > - case DW_ATE_signed_char: > break; > case DW_ATE_unsigned: > - case DW_ATE_unsigned_char: > + type_flags |=3D TYPE_FLAG_UNSIGNED; > + break; > + case DW_ATE_signed_char: > + if (cu->language =3D=3D language_m2) > + code =3D TYPE_CODE_CHAR; > + break; > + case DW_ATE_unsigned_char: > + if (cu->language =3D=3D language_m2) > + code =3D TYPE_CODE_CHAR; > type_flags |=3D TYPE_FLAG_UNSIGNED; > break; > default: This is what I had in mind --- thanks. > @@ -6146,6 +6168,10 @@ > { > switch (lang) > { > + case DW_LANG_Ada83: > + case DW_LANG_Ada95: > + cu->language =3D language_ada; > + break; > case DW_LANG_C89: > case DW_LANG_C: > cu->language =3D language_c; If you want to rearrange things, please submit that as a separate patch. If Modula-2 patch simply improves the Modula-2 support, and nothing else, that makes it easier to review. > @@ -6153,25 +6179,29 @@ > case DW_LANG_C_plus_plus: > cu->language =3D language_cplus; > break; > + case DW_LANG_Cobol74: > + cu->language =3D DW_LANG_Cobol74; > + break; > + case DW_LANG_Cobol85: > + cu->language =3D DW_LANG_Cobol85; > + break; Was this code here before? I'm sorry I didn't catch it the first time. You can't use DWARF language constants as values for cu->language; that's GDB's internal 'enum language'. They're completely different enums. > @@ -7337,7 +7371,8 @@ > struct die_info *parent; > > if (cu->language !=3D language_cplus > - && cu->language !=3D language_java) > + && cu->language !=3D language_java > + && cu->language !=3D language_pascal) Pascal? Does this belong here? > --- latest-cvs-gdb/src-cvs/gdb/doc/gdb.texinfo 2006-02-10 03:54:33.00000= 0000 +0000 > +++ latest-cvs-gdb/src-m2/gdb/doc/gdb.texinfo 2006-02-27 23:44:38.00000= 0000 +0000 > +Currently @value{GDBN} can print the following data types in Modula-2 > +syntax: array types, record types, set types, pointer types, procedure > +types, enumerated types, subrange types and base types. Values of > +these types may also be printed. Here's another use of the passive voice: "@value{GDBN} can also print values of these types." > +It is advisable to use the following compile flags on the @code{gm2} > +command line @option{-gdwarf-2}. Eli pointed this out before. I think you mean, "We recommend that you pass the @option{-gdwarf-2} option to @code{gm2} when compiling code you intend to debug with @value{GDBN}. Also note that the array handling is > +not yet complete and although the type is printed correctly, the > +expression handling still assumes that all arrays have a lower bound > +of zero and not @code{-10} in the case above. Unbounded arrays are > +also not yet recognised in @value{GDBN}. Here follows some more type "Here follow" > +related Modula-2 examples: > + > +@example > +TYPE > + colour =3D (blue, red, yellow, green) ; > + t =3D [blue..yellow] ; > +VAR > + s: t ; > +BEGIN > + s :=3D blue ; > +@end example > + > +The @value{GDBN} interaction shows that the correct value and type > +can be obtained. Eli has approved this, so what I say here is just a suggestion: The way this is written gives me the impression that GDB used to handle this poorly, but now handles it correctly. While that's great, the GDB manual isn't the place to describe the progression of bug fixes; it should simply describe the way GDB works now, as for a reader encountering GDB for the first time. (You can describe important bug fixes in gdb/NEWS.) Should this example of type printing simply be appended to the preceding series? The notes about using -gdwarf-2 could go to the end of the page, or perhaps be moved to @node Compilation, along with similar advice for other languages. > +In this example a Modula-2 array is declared and its contents > +displayed. Observe that the contents are written in the same > +way as their @code{C} counterparts. More use of the passive voice. Passive voice sentences have the form "X is Y-ed by Z", with the "by Z" part often omitted. You can rephrase these, almost mechanically, as "Z Y-s X." When you've omitted Z in the original, you have to make a Z up for use in the rephrased sentence. > +More complex types are also handled by @value{GDBN}, in this example > +we combine array types, record types, pointer types and subrange > +types. This is another use of the passive voice. Also, that first comma should be a period, and "in this example" should be the start of a new sentence.