[Lldb-commits] [PATCH] D25099: Refactor Args a different way

[Adrian McCarthy via lldb-commits]

amccarth added a comment.

Just a drive by.



> Args.h:449
>    //------------------------------------------------------------------
>    // Classes that inherit from Args can see and modify these
>    //------------------------------------------------------------------

This comment is no longer true given the change from protected to private just above.

> Args.cpp:96
> +ParseSingleArgument(llvm::StringRef command) {
> +  // Argument can be split into multiple discontiguous pieces, // for example:
>    //  "Hello ""World"

Minor formatting glitch with the `\\` in the comment.

> Args.cpp:192
> +//----------------------------------------------------------------------
> +// We have to be very careful on the copy constructor of this class
> +// to make sure we copy all of the string values, but we can't copy the

This says "copy constructor" but it seems to be documenting the copy assignment operator.

> Args.cpp:195
> +// rhs.m_argv into m_argv since it will point to the "const char *" c
> +// strings in rhs.m_args. We need to copy the string list and update our
> +// own m_argv appropriately.

You got right of m_args, so maybe this whole comment needs a rewrite.

> Args.cpp:282
> +
> +  // Now m_argv might be out of date with m_args, so we need to fix that.
> +  // This happens because getopt_long_only may permute the order of the

`m_args` is gone.

https://reviews.llvm.org/D25099