This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [V4 02/18] vla: make dynamic fortran arrays functional.


> 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


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]