[Lldb-commits] [lldb] r255598 - Fix Clang-tidy modernize-use-nullptr and readability-simplify-boolean-expr warnings in some files in source/Target/.

[Jim Ingham via lldb-commits]

I see the value of replacing NULL with nullptr, but I don't see the value of these sorts of changes:

> On Dec 14, 2015, at 5:33 PM, Eugene Zelenko via lldb-commits <lldb-commits at lists.llvm.org> wrote:
> 
> @@ -118,19 +111,13 @@ ThreadPlanStepInstruction::IsPlanStale (
>     StackID cur_frame_id = m_thread.GetStackFrameAtIndex(0)->GetStackID();
>     if (cur_frame_id == m_stack_id)
>     {
> -        if (m_thread.GetRegisterContext()->GetPC(0) != m_instruction_addr)
> -            return true;
> -        else
> -            return false;
> +        return (m_thread.GetRegisterContext()->GetPC(0) != m_instruction_addr);
>     }

To my eye the second version though more dense, is actually harder to read.  I tend to write returns this way because I like it this way.  It mirrors the thought process - I'm testing something, then doing something, and makes the various bits of code that do these tests have some uniformity, rather than burying them behind an assignment or return.  Moreover, I don't see the point of going through and changing code you didn't write to fit your or clang tidy's preferences.  It just sprinkles more random changes into the history that folks doing archeology now have to look past, and makes the code look like it wasn't written by whoever wrote it.  I don't see the value in either of these.

Jim