From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 14042 invoked by alias); 22 Oct 2008 22:32:27 -0000 Received: (qmail 14027 invoked by uid 22791); 22 Oct 2008 22:32:25 -0000 X-Spam-Check-By: sourceware.org Received: from mtagate4.de.ibm.com (HELO mtagate4.de.ibm.com) (195.212.29.153) by sourceware.org (qpsmtpd/0.31) with ESMTP; Wed, 22 Oct 2008 22:31:44 +0000 Received: from d12nrmr1607.megacenter.de.ibm.com (d12nrmr1607.megacenter.de.ibm.com [9.149.167.49]) by mtagate4.de.ibm.com (8.13.8/8.13.8) with ESMTP id m9MMVcJC109172 for ; Wed, 22 Oct 2008 22:31:38 GMT Received: from d12av02.megacenter.de.ibm.com (d12av02.megacenter.de.ibm.com [9.149.165.228]) by d12nrmr1607.megacenter.de.ibm.com (8.13.8/8.13.8/NCO v9.1) with ESMTP id m9MMVccu1495244 for ; Thu, 23 Oct 2008 00:31:38 +0200 Received: from d12av02.megacenter.de.ibm.com (loopback [127.0.0.1]) by d12av02.megacenter.de.ibm.com (8.12.11.20060308/8.13.3) with ESMTP id m9MMVbm3020572 for ; Thu, 23 Oct 2008 00:31:37 +0200 Received: from tuxmaker.boeblingen.de.ibm.com (tuxmaker.boeblingen.de.ibm.com [9.152.85.9]) by d12av02.megacenter.de.ibm.com (8.12.11.20060308/8.12.11) with SMTP id m9MMVb4P020569; Thu, 23 Oct 2008 00:31:37 +0200 Message-Id: <200810222231.m9MMVb4P020569@d12av02.megacenter.de.ibm.com> Received: by tuxmaker.boeblingen.de.ibm.com (sSMTP sendmail emulation); Thu, 23 Oct 2008 00:31:37 +0200 Subject: Re: [RFC] [patch] 'p->x' vs. 'p.x' and 'print object on' To: ppluzhnikov@google.com (Paul Pluzhnikov) Date: Wed, 22 Oct 2008 22:32:00 -0000 From: "Ulrich Weigand" Cc: tromey@redhat.com (Tom Tromey), gdb-patches@sourceware.org In-Reply-To: <8ac60eac0808191104r38efbd1fi28db42c05ae04d1d@mail.gmail.com> from "Paul Pluzhnikov" at Aug 19, 2008 11:04:42 AM X-Mailer: ELM [version 2.5 PL2] MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: 7bit 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-10/txt/msg00562.txt.bz2 Paul Pluzhnikov wrote: > gdb/ChangeLog > > 2008-08-19 Paul Pluzhnikov > > Changes to treat 'p.x' and 'p->x' the same. > * eval.c (value_static_or_dynamic_member): New. > (evaluate_subexp_standard): Call it. > > gdb/testsuite/ChangeLog > > 2008-08-19 Paul Pluzhnikov > > * gdb.cp/class3.exp: New test case. > * gdb.cp/class3.cc: New source file. Sorry for the late review. Tom just pointed me to this patch in the context of a different discussion ... In general, I think the approach looks right to me. However, the current patch does seem to have a couple of issues: > +static struct value * > +value_static_or_dynamic_member(struct value *arg, char *string, > + char *name, enum noside noside) > +{ > + struct type *type = value_type (arg); > + struct value *temp; > + > + if (noside == EVAL_AVOID_SIDE_EFFECTS) > + return value_zero (lookup_struct_elt_type (type, string, 0), > + lval_memory); > + temp = arg; > + { > + volatile struct gdb_exception except; > + struct value *v = NULL; > + TRY_CATCH (except, RETURN_MASK_ERROR) > + { > + v = value_struct_elt (&temp, NULL, string, NULL, name); > + } > + if (v) > + return v; > + } > + > + /* If we got here, value_struct_elt() above must have thrown, > + and there is no field 'name' in 'type'. > + Try dynamic type of 'arg' instead. */ > + > + if (TYPE_TARGET_TYPE(type) > + && (TYPE_CODE (TYPE_TARGET_TYPE (type)) == TYPE_CODE_CLASS)) > + { > + struct type *real_type; > + int full, top, using_enc; > + real_type = value_rtti_target_type (arg, &full, &top, &using_enc); > + if (real_type) > + { > + if (TYPE_CODE (type) == TYPE_CODE_PTR) > + real_type = lookup_pointer_type (real_type); > + else > + real_type = lookup_reference_type (real_type); > + > + temp = arg = value_cast (real_type, arg); > + } > + } > + return value_struct_elt (&temp, NULL, string, NULL, name); > +} You're treating looking up for type (EVAL_AVOID_SIDE_EFFECTS) differently from looking up for value; in the first case, you *never* look at the dynamic type -- that doesn't seem right to me (and in fact differs from what the original code does). Maybe you should add some "ptype p->x" tests to exercise this path as well. Also, you're calling value_static_or_dynamic_member both for class types *and* for pointer types -- but the whole dynamic type lookup as written only works for pointer types (starting with the if (TYPE_TARGET_TYPE)). If you really want to treat p.x and p->x equivalent in every case, why don't you call value_static_or_dynamic_member *always* with the class type (using a value_ind in the STRUCTOP_PTR case)? Then you'd use value_rtti_type instead of value_rtti_target_type. Bye, Ulrich -- Dr. Ulrich Weigand GNU Toolchain for Linux on System z and Cell BE Ulrich.Weigand@de.ibm.com