[-Wdocumentation][RFC] \param with multiple identifiers

I'm working on bug 14335 [1] and have a proof-of-concept patch. This
patch allows a single \param command with multiple identifiers. For
example:

/** Sets the position.
* @param [in] x,y,z Coordinates of the position in 3D space.
* @param [in] t The timestamp of the position.
*/
void setPosition(double x, double y, double z, double t);

Generates AST output like:

-ParagraphComment 0x560955761db0 <line:1291:4, line:1292:4>
>-TextComment 0x560955761d60 <line:1291:4, col:22> Text=" Sets the position."
`-TextComment 0x560955761d80 <line:1292:3, col:4> Text=" "
-ParamCommandComment 0x560955761dd0 <col:5, line:1293:4> [in] explicitly Param="x,y,z"
`-ParagraphComment 0x560955761ed0 <line:1292:22, line:1293:4>
  >-TextComment 0x560955761e80 <line:1292:22, col:63> Text=" Coordinates of the position in 3D space. "
  `-TextComment 0x560955761ea0 <line:1293:3, col:4> Text=" "

`-ParamCommandComment 0x560955761ef0 <col:5, col:52> [in] explicitly Param="t" ParamIndex=3
  `-ParagraphComment 0x560955761f80 <col:18, col:52>
    `-TextComment 0x560955761f50 <col:18, col:52> Text=" The timestamp of the position."

So it works, but I wonder what I should do about the ParamIndex. This
variable is an usigned in ParamCommandComment class.

1.
I can change ParamIndex to a SmallVector and change the AST output to
  [in] explicitly Param="x,y,z" ParamIndex=1 ParamIndex=2 ParamIndex=3
I wonder whether that would make the users of the AST output happy.
Suddenly there can be multiple ParamIndex 'fields'.

2.
I can also add a new magic number MultipleArgParamIndex and add an
isMultipleArgParam function and change the AST output to
  [in] explicitly Param="x,y,z" MultipleArg

3.
Alternatively I change nothing and the `ParamIndex` remains set to
`InvalidParamIndex`. (This is what happens now.)

I personally lean towards option 2, but I'm not sure about the inpact of
these changes of the consumers of the AST output. Note this only changes
the output when multiple arguments are used in a \param command. When
using a single argument is used nothing changes.

[1] https://bugs.llvm.org/show_bug.cgi?id=14335

Kind regards,
Mark de Wever

Hi Mark,

Thank you for working on this bug!

1.
I can change ParamIndex to a SmallVector and change the AST output to
  [in] explicitly Param="x,y,z" ParamIndex=1 ParamIndex=2 ParamIndex=3
I wonder whether that would make the users of the AST output happy.
Suddenly there can be multiple ParamIndex 'fields'.

Note however that AST classes are allocated with a BumpPtrAllocator, and
that therefore their destructor is not executed. They should therefore be
trivially destructible. This is not currently enforced but I have put two
patches for review (D66646 and D66722) which enforce this with a static_assert.

What you can do instead is either:

1) If you need to have the flexibility to change the size of the array,
   or if 2) is too complicated, just store a pointer to the array and
   allocate the array with the allocator (for an example see
   FunctionDecl::setParams).

2) Or if the number of elements in the array is fixed and known when
   the AST node is created, store them just after the node in memory
   with llvm::TrailingObjects (plenty of examples, just grep for TrailingObjects).
   This is especially easy if the class is a leaf class.

Bruno

You're totally right. I should have written ArrayRef. Thanks for your
patches, I already had a look at D66722.

Cheers,
Mark

Hi Dmitri,