+1 for limiting the scope of a variable as much as possible On Mon, Jun 18, 2018 at 7:57 AM Pavel Labath via lldb-dev < lldb-dev@lists.llvm.org> wrote:
> Thanks. I am going to submit the patch then. > > On Fri, 15 Jun 2018 at 19:56, Jim Ingham <jing...@apple.com> wrote: > > > On Jun 15, 2018, at 3:44 AM, Pavel Labath via lldb-dev < > lldb-dev@lists.llvm.org> wrote: > > > > > > Hello again, > > > > > > Just a quick update on the state of this. > > > > > > I've managed to move VersionTuple from clang to llvm. I've also > > > created <https://reviews.llvm.org/D47889> to switch over our version > > > handling to that class. > > > > > > Could I interest anyone in taking a quick look at the patch? > > > > > > Somehow I can’t log into Phabricator from home so I can’t comment right > now, but I took a look. > > > > In some of your changes in the SB files you do: > > > > if (PlatformSP platform_sp = GetSP()) > > version = platform_sp->GetOSVersion(); > > > > I don’t like putting initializers in if statements like this because I > always have to think twice about whether you meant “==“. Moreover, all of > the surrounding code does it differently: > > > > PlatformSP platform_sp = GetSP() > > if (platform_sp) > > version = platform_sp->GetOSVersion(); > > > > so switching to the other form in a couple of places only kinda forces > the double-take. But that’s a little nit. > > I've rechecked the llvm style guide. It doesn't say anything about > this particular issue, but this syntax is used throughout the examples > demonstrating other things. > > What I like about this syntax is that it makes it clear that the > variable has no meaning outside of the if block, which is the same > reason we declare variables inside the for() statement. But those are > microscopic details I'd leave to the discretion of whoever is writing > the code. > _______________________________________________ > lldb-dev mailing list > lldb-dev@lists.llvm.org > http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev >
_______________________________________________ lldb-dev mailing list lldb-dev@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev
