From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 8401 invoked by alias); 22 Feb 2015 04:23:17 -0000 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 Received: (qmail 8392 invoked by uid 89); 22 Feb 2015 04:23:16 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.8 required=5.0 tests=AWL,BAYES_00 autolearn=ham version=3.3.2 X-HELO: rock.gnat.com Received: from rock.gnat.com (HELO rock.gnat.com) (205.232.38.15) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-SHA encrypted) ESMTPS; Sun, 22 Feb 2015 04:23:15 +0000 Received: from localhost (localhost.localdomain [127.0.0.1]) by filtered-rock.gnat.com (Postfix) with ESMTP id 06746D30B3; Sat, 21 Feb 2015 23:23:13 -0500 (EST) 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 FOv9czGd9vtd; Sat, 21 Feb 2015 23:23:12 -0500 (EST) Received: from joel.gnat.com (localhost.localdomain [127.0.0.1]) by rock.gnat.com (Postfix) with ESMTP id 88BEBD30B2; Sat, 21 Feb 2015 23:23:12 -0500 (EST) Received: by joel.gnat.com (Postfix, from userid 1000) id 1A2E440EAD; Sun, 22 Feb 2015 08:23:15 +0400 (RET) Date: Sun, 22 Feb 2015 04:23:00 -0000 From: Joel Brobecker To: Keven Boell Cc: Keven Boell , gdb-patches@sourceware.org Subject: Re: [V4 02/18] vla: make dynamic fortran arrays functional. Message-ID: <20150222042315.GK23529@adacore.com> References: <1421243390-24015-1-git-send-email-keven.boell@intel.com> <1421243390-24015-3-git-send-email-keven.boell@intel.com> <20150209070931.GB15579@adacore.com> <54E4B768.1030906@linux.intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <54E4B768.1030906@linux.intel.com> User-Agent: Mutt/1.5.21 (2010-09-15) X-SW-Source: 2015-02/txt/msg00620.txt.bz2 > Basically the whole support for Fortran dynamic arrays is covered in > this patch, not only the support allocated/associated states. That's just too much for me to grok, and I think that's also going to make it harder later on to figure out why a certain piece of your changes is needed or not. When testing is fairly complete and it is affecting a mainstream language such as C, commenting the code out can help figure things out. But this is still sub- optimal, and nearly out of reach for less available languages such as Fortran. To give you an example showing that this is very necessary, we are currently trying to figure out, at AdaCore, why reference types are considered dynamic if their target type is dynamic. If I had asked that question earlier on, we wouldn't be trying to find the info a posteriori. > I thought it would be good to have one patch, which enables the > dynamic array support completely and not having individual small > patches, which might not functional at all. All the tests following > this patch are testing various functionality in the area of dynamic > arrays. Here's a suggestion of how I sometimes approach that problem. Start from a clean checkout and build it. Then use a program that has the simplest situation of an associated array, and start by adding the code that reads the DWARF info, and then keep adding code until GDB is able to print "" for that array. If you need additional changes to be able to print the correct value of associated arrays, leave those for later. We can deal deal with that as a separate patch (assuming no regression elsewhere, but I don't see how this could happen). Once we have that, we can start working on your testcase, and test the feature that you just introduced, within the limitations that your simplified patch currently have. Make that one patch, and send it. Once we have the first patch reviewed, and pushed to the GDB repository, we can claim that as first progress, and show that we have something for our efforts. Then we iterate, piece by piece, feature by feature, special-case by special-case. Each time, we need to handle the bare minimum, and have a corresponding new set of tests. Note that the new tests do not need to be in a new testcase - it is perfectly fine to add tests to a pre-existing testcase too. I apologize if it seems a little painful for you to be working this way, but I truly believe that it'll make things a lot easier to review, and as a result, I believe that it'll actually take less time to get that code accepted. -- Joel