From: Joel Brobecker <brobecker@adacore.com>
To: Keven Boell <keven.boell@linux.intel.com>
Cc: Keven Boell <keven.boell@intel.com>, gdb-patches@sourceware.org
Subject: Re: [V4 02/18] vla: make dynamic fortran arrays functional.
Date: Sun, 22 Feb 2015 04:23:00 -0000 [thread overview]
Message-ID: <20150222042315.GK23529@adacore.com> (raw)
In-Reply-To: <54E4B768.1030906@linux.intel.com>
> 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 "<unassociated>"
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
next prev parent reply other threads:[~2015-02-22 4:23 UTC|newest]
Thread overview: 36+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-01-14 13:49 [V4 00/21] Fortran dynamic array support Keven Boell
2015-01-14 13:49 ` [V4 01/18] vla: introduce allocated/associated flags Keven Boell
2015-02-09 6:52 ` Joel Brobecker
2015-02-09 23:37 ` Doug Evans
2015-02-11 8:44 ` Joel Brobecker
2015-02-11 17:36 ` Doug Evans
2015-02-18 15:54 ` Keven Boell
2015-01-14 13:49 ` [V4 04/18] test: evaluate dynamic arrays using Fortran primitives Keven Boell
2015-01-14 13:49 ` [V4 07/18] test: evaluating allocation/association status Keven Boell
2015-01-14 13:50 ` [V4 15/18] vla: get dynamic array corner cases to work Keven Boell
2015-01-14 13:50 ` [V4 03/18] test: basic tests for dynamic array evaluations in Fortran Keven Boell
2015-01-14 13:50 ` [V4 16/18] vla: Fortran dynamic string support Keven Boell
2015-01-14 13:50 ` [V4 05/18] test: dynamic arrays passed to subroutines Keven Boell
2015-01-14 13:50 ` [V4 12/18] test: add archer dynamic other frame test Keven Boell
2015-01-14 13:50 ` [V4 06/18] test: correct ptype of dynamic arrays in Fortran Keven Boell
2015-01-14 13:50 ` [V4 18/18] vla: add NEWS entry for dynamic array support Keven Boell
2015-01-14 18:01 ` Eli Zaretskii
2015-01-14 13:50 ` [V4 09/18] test: accessing dynamic array history values Keven Boell
2015-01-14 13:50 ` [V4 14/18] vla: use value constructor instead of raw-buffer manipulation Keven Boell
2015-01-14 13:50 ` [V4 10/18] test: basic MI test for the dynamic array support Keven Boell
2015-01-14 13:50 ` [V4 17/18] vla: add stride support to fortran arrays Keven Boell
2015-01-14 13:50 ` [V4 08/18] test: dynamic arrays passed to functions Keven Boell
2015-01-14 13:50 ` [V4 11/18] test: test sizeof for dynamic fortran arrays Keven Boell
2015-01-14 13:50 ` [V4 02/18] vla: make dynamic fortran arrays functional Keven Boell
2015-02-09 7:09 ` Joel Brobecker
2015-02-18 16:02 ` Keven Boell
2015-02-22 4:23 ` Joel Brobecker [this message]
2015-01-14 13:50 ` [V4 13/18] vla: reconstruct value to compute bounds of target type Keven Boell
2015-05-28 20:36 ` [V4 00/21] Fortran dynamic array support Jan Kratochvil
2015-05-28 20:52 ` Joel Brobecker
2015-06-14 8:15 ` Jan Kratochvil
2015-06-17 11:42 ` Boell, Keven
2015-07-01 12:43 ` Keven Boell
2016-07-07 8:30 ` Jan Kratochvil
2016-07-07 9:16 ` Bernhard Heckel
2016-07-07 9:23 ` Jan Kratochvil
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20150222042315.GK23529@adacore.com \
--to=brobecker@adacore.com \
--cc=gdb-patches@sourceware.org \
--cc=keven.boell@intel.com \
--cc=keven.boell@linux.intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox