[llvm-commits] [PATCH] Coding Standards: Doxygen guidelines

[Chandler Carruth]

On Fri, Sep 14, 2012 at 11:12 AM, Andrew Trick <atrick at apple.com> wrote:

>
> On Sep 14, 2012, at 10:44 AM, Dmitri Gribenko <gribozavr at gmail.com> wrote:
>
> > On Fri, Sep 14, 2012 at 8:32 PM, Andrew Trick <atrick at apple.com> wrote:
> >>
> >> On Sep 14, 2012, at 8:27 AM, Dmitri Gribenko <gribozavr at gmail.com>
> wrote:
> >>
> >>> Hello,
> >>>
> >>> @Chris, Chandler: CC'ing you since this an addition to the policies
> >>> and I feel like I need your review.
> >>>
> >>> The attached patch documents current Doxygen use practices in Coding
> >>> Standards.  Mostly it is obvious stuff and most new code being
> >>> committed conforms to that.  Some old code does not; this might cause
> >>> confusion and this is the motivation to document the correct
> >>> guidelines.
> >>>
> >>> Please review.
> >>
> >> +Don't duplicate the documentation comment in header file and in
> implementation
> >> +file.  If the API being documented is not module-private,
> documentation should
> >> +go to the header file.
> >>
> >> Sorry, this wasn't obvious to me, so I may be violating it in the short
> term until I understand the policy.
> >
> > This is to document the current practice rather than to introduce
> > something new, so I brought it up for discussion.
> >
> >> I'm definitely going to add function-level comments in the .cpp file.
> So should those detailed comments be doxygen style? The header comments are
> for people who need to understand the API. The out-of-line comments are for
> people hacking on the implementation. I often start by duplicating the
> short comment and refine it from there.
> >
> > I didn't really understand you.  Do you mean that for a given function
> > you put interface explanations in .h and implementation comments in
> > .cpp right before the function?  It would be nice if you could give a
> > pointer to some example.
>
>
> <MachineScheduler.h>
>
>   /// Implement ScheduleDAGInstrs interface for scheduling a sequence of
>   /// reorderable instructions.
>   virtual void schedule();
>
> <MachineScheduler.cpp>
>
> /// schedule - Called back from MachineScheduler::runOnMachineFunction
> /// after setting up the current scheduling region. [RegionBegin,
> RegionEnd)
> /// only includes instructions that have DAG nodes, not scheduling
> boundaries.
> ///
> /// This is a skeletal driver, with all the functionality pushed into
> helpers,
> /// so that it can be easilly extended by experimental schedulers.
> Generally,
> /// implementing MachineSchedStrategy should be sufficient to implement a
> new
> /// scheduling algorithm. However, if a scheduler further subclasses
> /// ScheduleDAGMI then it will want to override this virtual method in
> order to
> /// update any specialized state.
> void ScheduleDAGMI::schedule()
>
> I admit that most of the time I just duplicate the short comment and have
> nothing more to say. It's nice to have a place to tack on FIXME's and
> TODO's though.
>

I'm actually very comfortable with the only comments on a function's
implementation being FIXME and TODO entries... =] More comfortable if they
decrease over time.

I'm also very comfortable with functions whose implementation is blindingly
obvious to the reader, and there is really nothing to say, having no
comment in the source code. We shouldn't add comments just to have that
block of text at the top. ;]


>
> I never realized that duplicating the short comment was a problem. It's no
> more irritating to update than the function name and type, though less
> often caught. It seems by Chandler's logic we should omit variable names
> from the function prototype too.
>

I wouldn't go that far. The names in the prototype help serve as part of
the documentation, and in fact allow writing much of the doxygen interface
comments. Omitting them would be counterproductive in that since.

Also, for whatever reason, I've heard almost no complaints about these
getting out of sync (whether or not it happens). I don't have any real
logical argument as to why, but that's the observation I make from watching
the development and listening to quite a few new developers ramping up.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20120914/c113c406/attachment.html>