<div dir="ltr">Hello Bruce,<div><br></div><div>+lldb-dev</div><div><br></div><div>1. Not all ctor/dtor comments are worthless. Of course we can remove something like:</div><div>```</div><div><div> 51 //++ ------------------------------------------------------------------------------------</div><div> 52 // Details: CMICmdArgValFile destructor.</div><div> 53 // Type: Overridden.</div><div> 54 // Args: None.</div><div> 55 // Return: None.</div><div> 56 // Throws: None.</div><div> 57 //--</div></div><div>```</div><div>But some of them contain useful information, like this:</div><div>```</div><div><div> 37 //++ ------------------------------------------------------------------------------------</div><div> 38 // Details: CMICmdArgValFile constructor.</div><div> 39 // Type: Method.</div><div> 40 // Args: vrArgName - (R) Argument's name to search by.</div><div> 41 // vbMandatory - (R) True = Yes must be present, false = optional argument.</div><div> 42 // vbHandleByCmd - (R) True = Command processes *this option, false = not handled.</div><div> 43 // Return: None.</div><div> 44 // Throws: None.</div><div> 45 //--</div></div><div>```</div><div><br></div><div>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.</div><div><br></div><div>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.<br></div><div>```</div><div><div> 15 // Environment: Compilers: Visual C++ 12.</div><div> 16 // gcc (Ubuntu/Linaro 4.8.1-10ubuntu9) 4.8.1</div><div> 17 // Libraries: See MIReadmetxt.</div><div> 18 //</div><div> 19 // Copyright: None.</div></div><div>```</div><div><br></div><div>In my opinion, all other comments are OK because they improve a code readability:</div><div>```</div><div> 47 class CMICmnMIValue : public CMICmnBase </div><div> 48 { </div><div> 49 // Methods:</div><div> 50 public:</div><div> 51 /* ctor */ CMICmnMIValue(void); </div><div> 52 //</div><div> 53 const CMIUtilString &GetString(void) const; </div><div> 54 </div><div> 55 // Overridden:</div><div> 56 public:</div><div> 57 // From CMICmnBase </div><div> 58 /* dtor */ virtual ~CMICmnMIValue(void); </div><div> 59 </div><div> 60 // Attributes:</div><div> 61 protected:</div><div> 62 CMIUtilString m_strValue;</div><div> 63 bool m_bJustConstructed; // True = *this just constructed with no value, false = *this has had value added to it </div><div> 64 };</div><div>``` </div><div><br></div><div>2. Ok. -- that's about "void" in case of empty list of arguments<br></div><div>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.<br></div><div>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.<br></div><div><br></div><div>Thanks,</div><div>Ilia</div></div><div class="gmail_extra"><br><div class="gmail_quote">On Tue, Jul 21, 2015 at 2:38 PM, Bruce Mitchener <span dir="ltr"><<a href="mailto:bruce.mitchener@gmail.com" target="_blank">bruce.mitchener@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">Hello Ilia,<div><br></div><div>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.</div><div><br></div><div>One of these passes would be to remove the ctor and dtor comments from header files.</div><div><br></div><div>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.)</div><div><br></div><div>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.)</div><div><br></div><div>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?</div><div><br></div><div>Thanks for your time,</div><div><br></div><div> - Bruce</div><div><br></div></div>
</blockquote></div><br></div>