From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id 1+ehNkk2sV82QwAAWB0awg (envelope-from ) for ; Sun, 15 Nov 2020 09:08:09 -0500 Received: by simark.ca (Postfix, from userid 112) id D28D61F08B; Sun, 15 Nov 2020 09:08:09 -0500 (EST) X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on simark.ca X-Spam-Level: X-Spam-Status: No, score=0.3 required=5.0 tests=MAILING_LIST_MULTI,RDNS_NONE autolearn=no autolearn_force=no version=3.4.2 Received: from sourceware.org (unknown [8.43.85.97]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPS id D46821E552 for ; Sun, 15 Nov 2020 09:08:08 -0500 (EST) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 331853857C65; Sun, 15 Nov 2020 14:08:08 +0000 (GMT) Received: from rock.gnat.com (rock.gnat.com [IPv6:2620:20:4000:0:a9e:1ff:fe9b:1d1]) by sourceware.org (Postfix) with ESMTP id BCFF13857C65 for ; Sun, 15 Nov 2020 14:08:05 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org BCFF13857C65 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=adacore.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=brobecker@adacore.com Received: from localhost (localhost.localdomain [127.0.0.1]) by filtered-rock.gnat.com (Postfix) with ESMTP id 528EE1162DD; Sun, 15 Nov 2020 09:08:05 -0500 (EST) X-Virus-Scanned: Debian amavisd-new at gnat.com Received: from rock.gnat.com ([127.0.0.1]) by localhost (rock.gnat.com [127.0.0.1]) (amavisd-new, port 10024) with LMTP id LJ8GFg7yvHP2; Sun, 15 Nov 2020 09:08:05 -0500 (EST) Received: from float.home (localhost.localdomain [127.0.0.1]) (using TLSv1.2 with cipher ADH-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by rock.gnat.com (Postfix) with ESMTPS id E7D311162DB; Sun, 15 Nov 2020 09:08:04 -0500 (EST) Received: by float.home (Postfix, from userid 1000) id 7340BA1870; Sun, 15 Nov 2020 18:07:59 +0400 (+04) Date: Sun, 15 Nov 2020 18:07:59 +0400 From: Joel Brobecker To: Andrew Burgess Subject: Re: [PATCH] gdb: user variables with components of dynamic type Message-ID: <20201115140759.GA498820@adacore.com> References: <20201022153238.1947197-1-andrew.burgess@embecosm.com> <20201106230422.GK2729@embecosm.com> <20201108105059.GC451505@adacore.com> <20201112160018.GR2729@embecosm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20201112160018.GR2729@embecosm.com> X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: gdb-patches@sourceware.org Errors-To: gdb-patches-bounces@sourceware.org Sender: "Gdb-patches" Hi Andrew, > So your third choice was the winner, the array has dynamic type and > includes a computed data location. Thanks for providing the additional details. > The problem in terms of implementation is that really everything is > either a real inline value, or a pointer. All the other words are > just language sugar on top of these two choices. Agreed. > When we look at C++ references (basically pointers + automatic > dereferencing), Thanks for sharing the experiment. This was actually a good refresher for me. > So we get the behaviour we might expect, the pointer value underlying > the reference is preserved, but the value pointed too is not. Yeah. What I took away from this experiment is that I think the syntax in printing the value gives some clues as to why some of the underlying value is not captured by the value history mechanism. Whether this is enough or not will likely depend on the level of the user, but I tend to think that a reasonably sharp user seeing @: can infer that the value printed from it is the current value at the address shown. So good enough for me. > > It's possible that the compromise you suggest (treat dynamic components > > the same as pointers) might be the most reasonable way out, but I think > > it'll invite confusion on the users' side, and probably bug reports. > > At the very least, I think we should warn users when we do this, so > > as to be sure to set expectations right, on the spot. > > Adding a warning would be reasonably simple, we can start with (in > value.c:set_internalvar): > > if (is_dynamic_type (value_type (new_data.value))) > warning ("some warning text here..."); > > There's two problems, the first is easy enough to solve: if the top > level value being captured is dynamic, then we do capture the _actual_ > value, it's only when a sub-component is dynamic that we have > problems. The above check will trigger if only the top-level value is > dynamic, so it warns in too many places. > > As a concrete example, given this Fortran type: > > type :: some_type > integer, allocatable :: array_one (:,:) > integer :: a_field > integer, allocatable :: array_two (:,:) > end type some_type > > type(some_type) :: some_var > > Then in GDB: > > (gdb) set $foo = some_var > > We capture the contents of the some_type struct, including the > pointers to the dynamic objects array_one and array_two. But if > instead we do: > > (gdb) set $bar = some_var%array_one > > Now we capture the full contents of array_one, there's no further > dynamic type resolution required. Changing 'some_var%array_one' will > not change the value of $bar, but the change would be see in $foo. > > The harder problem is, what warning do we print?? I initially went > with: > > components of dynamically typed values are not currently captured within internal variables > > despite being a bit long, it's not immediately clear if a user will > know what 'dynamically typed values' means? Maybe we end up needing a > language specific warning, so for Fortran: > > the values of allocatable fields are not currently captured within internal variables > > thoughts or suggestions are welcome... I admit I'm having minor headaches trying to wrap my head around the various scenarios and how we would want to deal with the situation. For instance, do we auto-dereference [1] the same way for all languages? For Ada, for instance, I think we always auto-derefrence, regardless if the data we're printing is top-level or whether it's part of an aggregate. It sounds like the same is True for Fortran as well. But it does look from your experiments that C++ only dereferences in some of the cases (e.g. when printing a struct with a field being a reference, the field's value is the address, without the actual underlying value). This question would have an impact on the warning, since we would warn only when there is auto-dereferencing. For the timing of the warning, at least, it seems clear to me that it would be printed when printing the value. Do we want to print it when accessing the value too? It would seem logical, I think, but are there scenarios where this could be counter-productive? There's also the question of avoiding to get the warning printed in the middle of the value itself. This might be a bit minor, but it's something I've always found to be distracting and hard to read, when a warning is lost with some other data being printed. > > Have you looked at how we handle components which are references? > > I wonder how well we handle those... > > As above we treat them as pointers, but guard against possible > confusion by displaying them as pointers. > > I would not like to change Fortran from displaying dynamnic types as > their actual value (and instead just display a pointer) as that seems > like a really bad change just to work around a limitation with > internal variables. 100% agreed. > What I think is super interesting is how this all interacts with > pretty-printers. So, if I start with this test program: Interesting indeed. > I wonder if this problem should just be solved (at least in the > short/medium term) by improving the documentation for internal > variables? For me, if I had to summarize my current thinking thanks to our discussions so far, I would say: (a) The case of fields whose type is dynamic is really very similar to the case of references. (b) Since we handle references the same as pointers, I think it's fine as a first instance to fix our immediate issue, which is the internal error, by providing the exact same kind of behavior for those dynamic fields as we provide for references. (c) Finding a way to make this clearer for users would be a nice enhancement, but this would be a general enhancement, not something required in the context of your patch. So you and I are converging towards the same solution in the code, and I agree that a documentation update might be useful as well. Going back to your original patch, I would need more research than what I have time for to determine whether I'd fix it the same way you did at the location you did. After much staring with the entire function's implementation as a context, the patch does make sense to me, especially if I ignore a bit the asserts for a minute. Hopefully others with a more complete knowledge of the area of value-saving for the value history can chime in. I wish I could be more help! :-/ -- Joel [1] I know your situation has to do with dynamic types rather than references, but I think both cases are in the same boat and could be treated the same.