Re: "Documentation by paper"

[Felix Lee <felix dot 1 at canids dot net>]

kenner@vlsi1.ultra.nyu.edu (Richard Kenner):
> How could I possibly write documentation on code that I need documentation
> to understand?  Any documentation I could write would be totally useless!

why do you say that?  I think it's generally not that hard to
approach unfamiliar code and add documentation when I figure out
what it's doing.  it's often just tedious.  of course, my
understanding can be faulty, but that's what peer review is
supposed to be for.

as an experiment, I picked a random gcc file, genemit.c.  I've
never looked at it before, and I'm not really familiar with gcc
internals.  you added a function to it a couple years ago

    /* Write a function, `added_clobbers_hard_reg_p' this is given an insn_code
       number that needs clobbers and returns 1 if they include a clobber of a
       hard reg and 0 if they just clobber SCRATCH.  */

    static void
    output_added_clobbers_hard_reg_p ()

I don't have much idea what that comment means, mostly because
the function name 'added_clobbers_hard_reg_p' doesn't make sense
to me.  I don't know what the 'added' refers to.

so I look at who calls added_clobbers_hard_reg_p, which is
insn_invalid_p in recog.c.  that's helpful, and it suggests to me
that if the function were named something like
'adding_clobbers_would_clobber_hard_reg_p' then I'd have
understood the comment better, and maybe even understood the
function without a descriptive comment.

or maybe the word 'added' should just be deleted, since it
doesn't seem to me that where the function's used is relevant to
what it does.  if it were called just 'clobbers_hard_reg_p', then
I wouldn't have been confused by the comment.

but after looking at insn_invalid_p, I'm now unclear why the
added_clobbers_hard_reg_p function exists at all, since my
intuition is that a function named insn_invalid_p should be a
pure function, and the prologue comment doesn't give me any
reason to expect the function to have side effects.  so I'm
puzzled insn_invalid_p modifies the insn it's given.  it seems to
me the modification should happen somewhere earlier, but perhaps
there's a reason for doing it this way.

that was also a change you made.  so, starting from the
assumption that you're not an idiot, I can work on reconstructing
your logic, which involves looking at insn_invalid_p's callers,
digging into mailing list discussions, asking you, etc.  after
doing all that, I could submit patches that summarize what I
find, so other people don't have to go through the same
investigative process.  but all that's too much work right now.
I have other things I need to do.  maybe later.
--