From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 8220 invoked by alias); 23 Jan 2008 23:55:34 -0000 Received: (qmail 8207 invoked by uid 22791); 23 Jan 2008 23:55:32 -0000 X-Spam-Check-By: sourceware.org Received: from ics.u-strasbg.fr (HELO ics.u-strasbg.fr) (130.79.112.250) by sourceware.org (qpsmtpd/0.31) with ESMTP; Wed, 23 Jan 2008 23:55:05 +0000 Received: from ICSMULLER (unknown [130.79.244.149]) by ics.u-strasbg.fr (Postfix) with ESMTP id E249618701B; Thu, 24 Jan 2008 01:01:27 +0100 (CET) From: "Pierre Muller" To: "'Joel Brobecker'" Cc: "'Eli Zaretskii'" , References: <002d01c85849$ef420f80$cdc62e80$@u-strasbg.fr> <002401c85c1a$b1997b30$14cc7190$@u-strasbg.fr> <20080123182514.GB3979@adacore.com> <001b01c85e10$48a8aa40$d9f9fec0$@u-strasbg.fr> <20080123230929.GC3979@adacore.com> In-Reply-To: <20080123230929.GC3979@adacore.com> Subject: RE: [RFA] Handle BINOP_INTDIV in valarith.c Date: Wed, 23 Jan 2008 23:55:00 -0000 Message-ID: <001d01c85e1b$5e280c70$1a782550$@u-strasbg.fr> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit X-Mailer: Microsoft Office Outlook 12.0 Content-Language: en-us Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2008-01/txt/msg00571.txt.bz2 OK, I get the idea, but the problem is that this leads to lots of code copy... And errors are duplicated at several places: See for instance my bug report: http://sourceware.org/ml/gdb/2008-01/msg00209.html 'ptyp 3 / 2.0' which returns 'int' type while 'print 3 / 2.0' returns '1.5' which is not an int! I suspect that the code in Ada shows the same bugs: > -----Original Message----- > From: gdb-patches-owner@sourceware.org [mailto:gdb-patches- > owner@sourceware.org] On Behalf Of Joel Brobecker > Sent: Thursday, January 24, 2008 12:09 AM > To: Pierre Muller > Cc: 'Eli Zaretskii'; gdb-patches@sourceware.org > Subject: Re: [RFA] Handle BINOP_INTDIV in valarith.c > > > I wrote the floatdiv version, which allows me to easily force the > > conversion to double formats for left and right operand: > > Right, but you are doing the conversion inside what I call "core-gdb", > which is a language-independent file. As a result, you ended up needing > a new operator. But the creation of this new operator would not be > necessary if you handled BINOP_DIV in a pascal-specific way. > > > maybe > > || (op == BINOP_DIV && current_language = language_pascal)) > > That's not the proper way of doing this. What you need to do is to > setup your language to have a pascal-specific expression evaluator. > Right now, it's set to use the "standard" expression evaluator which > (IIRC) is the C evaluator. Your pascal expression evaluator will > defer back to the standard expression evaluator most of the time, > except for the case when the binary operator is FLOAT_DIV. In this > case, you'll do the division yourself. > > You can have a look at ada-lang.c:ada_evaluate_subexp(). It's a large > routine because of a lot of stuff is special to Ada, but you'll also > see: > > op = exp->elts[pc].opcode; > [...] > switch (op) > { > default: > *pos -= 1; > arg1 = evaluate_subexp_standard (expect_type, exp, pos, noside); > [...] > case BINOP_DIV: > arg1 = evaluate_subexp (NULL_TYPE, exp, pos, noside); > arg2 = evaluate_subexp (NULL_TYPE, exp, pos, noside); > if (noside == EVAL_SKIP) > goto nosideret; > else if (noside == EVAL_AVOID_SIDE_EFFECTS > && (op == BINOP_DIV || op == BINOP_REM || op == > BINOP_MOD)) > return value_zero (value_type (arg1), not_lval); which means that 'ptyp' which sets noside to EVAL_AVOID_SIDE_EFFECTS will not see the fixed to double conversions below and will also return 'fixed', while the result of print will be a 'double'. Furthermore, you can clearly see that this code was copied from the C evaluator without removing the unnecessary parts: the check > && (op == BINOP_DIV || op == BINOP_REM || op == > BINOP_MOD)) is clearly redundant here as we are inside case BINOP_DIV. The same unnecessary test is also done in the BINOP_MOD-BINOP_REM case. > else > { > if (ada_is_fixed_point_type (value_type (arg1))) > arg1 = cast_from_fixed_to_double (arg1); > if (ada_is_fixed_point_type (value_type (arg2))) > arg2 = cast_from_fixed_to_double (arg2); > return ada_value_binop (arg1, arg2, op); > } > > If you look at the "default:" case, you'll see that we let > the standard evaluator do the work for us. But we treat the > BINOP_DIV case specially: First we evaluate each side of our > division, and then perform the division and return the result. > > This ada_evaluate_subexp routine is setup in the language through > the exp_descriptor structure. I agree with you that writing a pascal specific evaluate_subexp() function is better that explicitly checking the language of the currently parsed expression, but my idea was really to have on the contrary something usable for other languages, and thus not inside pascal specific code... On the other hand, no one said that another language has the same view on the '/' operator as pascal... Whihch validates your view that a new BINOP is not necessary... So I will probably try to write a pascal_evaluate_subexp function as you suggested. Thanks for the feedback. Pierre Muller