Re: [PATCH] gdbserver: convert program_args to a single string

[Simon Marchi <simark@simark.ca>]

On 2025-01-14 12:23, Tom Tromey wrote:
>>>>>> "Simon" == Simon Marchi <simark@simark.ca> writes:
> 
>>> /* Start a new process.
>>> PROGRAM is the program name.
>>> -   PROGRAM_ARGS is the vector containing the inferior's args.
>>> +   PROGRAM_ARGS is a string containing all the inferior's arguments.
>>> Returns the new PID on success, -1 on failure.  Registers the new
>>> process with the process list.  */
> 
> Simon> I think this comment should just be removed, there's no point in
> Simon> repeating the documentation from the base class.
> 
> FWIW I normally consider the "override" keyword to be sufficient
> documentation in these cases; though there's also some code with a
> comment pointing at the base class.

Speaking of this, I don't really know what to do in these situations, I'd like
if we could clarify what our style is.  Let's say you have:

-- base.h

struct base
{
  /* 1 */
  virtual void method ();
};

-- base.c

/* 2 */

void base::method ()
{
}

-- impl.c

struct impl : public base
{
  /* 3 */
  void method () override;
};

/* 4 */

void impl::method ()
{
}

Which comment should you have where?  We typically say "all declarations
and definitions should have a comment", but I feel like it's unnecessary here.

 - #1 is where you should have the documentation about the behavior of
   `method`
 - #2 would be the typical /* See base.h.  */, although I find these
   comments a bit useless
 - As you said, I think the `override` keyword is sufficient to make
   comment #3 unnecessary, unless you want to specify something specific
   to `impl`.  I don't see the need to point to the base class, that's
   just implied by how the language works.
 - I never know what to put for #4, you can't put /* See impl.c.  */,
   since you're already in impl.c.  Again, I feel like a "See whatever"
   comment is a bit useless: if you know C++, you know you need to go
   look at the declaration of struct/class `impl`.

Simon