[lldb-dev] LLDB-MI cleanups

[Ilia K]

Hello Bruce,

+lldb-dev

1. Not all ctor/dtor comments are worthless. Of course we can remove
something like:
```
 51 //++
------------------------------------------------------------------------------------
 52 // Details: CMICmdArgValFile destructor.
 53 // Type:    Overridden.
 54 // Args:    None.
 55 // Return:  None.
 56 // Throws:  None.
 57 //--
```
But some of them contain useful information, like this:
```
 37 //++
------------------------------------------------------------------------------------
 38 // Details: CMICmdArgValFile constructor.
 39 // Type:    Method.
 40 // Args:    vrArgName       - (R) Argument's name to search by.
 41 //          vbMandatory     - (R) True = Yes must be present, false =
optional argument.
 42 //          vbHandleByCmd   - (R) True = Command processes *this
option, false = not handled.
 43 // Return:  None.
 44 // Throws:  None.
 45 //--
```

I think we can remove "Throws" because lldb doesn't use exceptions at all
and it always is "None". Also we should remove "Return" and "Args" too in
case of "None" because of useless. As for me, the only plus of having
"Type" field is to know whether a method is static or not, but lldb doesn't
store such details in .cpp files, so we can fully remove it.

Also we already have discussed that we shouldn't use
Gotchas/Authors/Changes fields in header files, and perhaps we like to
remove them too. In addition, Environment/Copyright fields don't have a
sense and should be removed also.
```
 15 // Environment: Compilers:  Visual C++ 12.
 16 //                          gcc (Ubuntu/Linaro 4.8.1-10ubuntu9) 4.8.1
 17 //              Libraries:  See MIReadmetxt.
 18 //
 19 // Copyright:   None.
```

In my opinion, all other comments are OK because they improve a code
readability:
```
 47 class CMICmnMIValue : public CMICmnBase



 48 {
 49     // Methods:
 50   public:
 51     /* ctor */ CMICmnMIValue(void);
 52     //
 53     const CMIUtilString &GetString(void) const;



 54
 55     // Overridden:
 56   public:
 57     // From CMICmnBase
 58     /* dtor */ virtual ~CMICmnMIValue(void);



 59
 60     // Attributes:
 61   protected:
 62     CMIUtilString m_strValue;
 63     bool m_bJustConstructed; // True = *this just constructed with no
value, false = *this has had value added to it


 64 };
```

2. Ok. -- that's about "void" in case of empty list of arguments
3. I think it will be ok, but perhaps in some cases we shouldn't return
anything. Please do that in a separate patch to have a chance to discuss.
4. Yes, it's the general rule - to have empty ctors/dtors for determinism,
and thus avoiding their autogeneration. I'm not sure about the necessity of
that but it's widely used in lldb.

Thanks,
Ilia

On Tue, Jul 21, 2015 at 2:38 PM, Bruce Mitchener <bruce.mitchener at gmail.com>
wrote:

> Hello Ilia,
>
> I'd like to do a couple of bulk cleanups in a single pass to try and make
> some stuff look more like typical LLDB sources.
>
> One of these passes would be to remove the ctor and dtor comments from
> header files.
>
> Another would be to remove (void) parameter lists and just leave them as
> () as one normally does in C++. (I'm aware of the difference in C.)
>
> I'd also like to see MIstatus::success and MIstatus::failure die ... and a
> bunch of functions which never do any error handling shouldn't actually
> return bool. (Like the 'Add' methods in MICmnMIValue classes.)
>
> Another thing is whether or not we actually need empty destructors on so
> many classes. Can we safely remove the constructors which aren't actually
> doing anything?
>
> Thanks for your time,
>
>  - Bruce
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/lldb-dev/attachments/20150722/45eedac1/attachment.html>