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

[Andrew Trick]

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 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.

No strong feelings here, just trying to understand the intent.

-Andy