[lldb-dev] clang::VersionTuple

[Zachary Turner via lldb-dev]

+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 at lists.llvm.org> wrote:

> Thanks. I am going to submit the patch then.
>
> On Fri, 15 Jun 2018 at 19:56, Jim Ingham <jingham at apple.com> wrote:
> > > On Jun 15, 2018, at 3:44 AM, Pavel Labath via lldb-dev <
> lldb-dev at 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 at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/lldb-dev/attachments/20180618/dbdc0896/attachment.html>