Re: [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@lists.llvm.org> wrote:

> Thanks. I am going to submit the patch then.
>
> On Fri, 15 Jun 2018 at 19:56, Jim Ingham  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  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

Re: [lldb-dev] clang::VersionTuple

[Pavel Labath via lldb-dev]

Thanks. I am going to submit the patch then.

On Fri, 15 Jun 2018 at 19:56, Jim Ingham  wrote:
> > On Jun 15, 2018, at 3:44 AM, Pavel Labath via lldb-dev 
> >  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  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

Re: [lldb-dev] clang::VersionTuple

[Jim Ingham via lldb-dev]

> On Jun 15, 2018, at 3:44 AM, Pavel Labath via lldb-dev 
>  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  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.

Given that clang::VersionTuple::tryParse does what the hand-parsing in place 
(which I’m assuming it does or you wouldn’t have used it) this looks fine and a 
nice cleanup to me.

Jim


> 
> pl
> 
> On Wed, 9 May 2018 at 08:49, Pavel Labath  wrote:
>> 
>> Thank you all for the feedback. I'll get on with this when I find some
>> spare time. I will send a patch which will show the final look of the code,
>> before I start moving VersionTuple into llvm.
>> 
>> cheers,
>> pl
>> On Tue, 8 May 2018 at 19:46, Frédéric Riss via lldb-dev <
>> lldb-dev@lists.llvm.org> wrote:
>> 
>> 
>> 
>>> On May 8, 2018, at 11:37 AM, Greg Clayton  wrote:
>> 
>>> I was referring to the Swift and Apple internal branches. They tend to
>> lock down against older llvm and clang repositories so when we put changes
>> in llvm or clang that are required for LLDB, it makes merging a bit tougher
>> in those cases. Again, I am not affected by this, just trying to watch out
>> for you guys.
>> 
>> 
>>> I understand and I appreciate it, I was just worried that I’m missing
>> something obvious. We branch LLDB at the same time as LLVM so that’s not
>> actually an issue. Of course, it might cause merge conflicts or make it
>> harder to cherry-pick patches, but that's just living downstream.
>> 
>>> Fred
>> 
>>> Greg
>> 
>> 
>>> On May 8, 2018, at 11:35 AM, Greg Clayton  wrote:
>> 
>>> I'm good if Apple is good.
>> 
>>> On May 8, 2018, at 11:31 AM, Frédéric Riss  wrote:
>> 
>> 
>> 
>>> On May 8, 2018, at 10:04 AM, Greg Clayton via lldb-dev <
>> lldb-dev@lists.llvm.org> wrote:
>> 
>> 
>> 
>>> On May 8, 2018, at 9:47 AM, Zachary Turner  wrote:
>> 
>>> We don’t want the lowest levels of lldb to depend on clang. If this is
>> useful we should move it from clang to llvm and use llvm::VersionTuple
>> 
>> 
>>> I agree, though this move will cause merging issues for many that have
>> repositories that link against older llvm/clang. Doesn't affect me anymore,
>> but Apple will be affected.
>> 
>> 
>>> I’m not sure I understand what issues you’re referring to, we don’t link
>> new LLDBs to old clangs (and even if we did, it wouldn’t be something the
>> that drives community decisions).
>> 
>>> Fred
>> 
>>> Greg
>> 
>>> On Tue, May 8, 2018 at 9:26 AM Greg Clayton via lldb-dev <
>> lldb-dev@lists.llvm.org> wrote:
>> 
 No issues from me.
>> 
> On May 8, 2018, at 9:11 AM, Pavel Labath via lldb-dev <
>> lldb-dev@lists.llvm.org> wrote:
> 
> While moving Args around, I noticed that we have a bunch of
> functions/classes that pass/store version numbers as a triplet of
>> integers
> (e.g. Platform::GetOSVersion). I got halfway into creating a wrapper
>> class
> for that when I noticed clang::VersionTuple, which is pretty much what
>> I
> wanted out of the box.
> 
> Now there are small differences between this class, and what we have
>> now:
> it has an extra fourth "build" field, and it uses only 31 bits to
>> represent
> the values. None of these seem to matter (particularly as we are
> converting our representation into this struct in some places) that
>> much,
> but before I go through the trouble of pulling this class into llvm
> (although technically possible, it seems wrong to pull a clang
>> dependency
> at such a low level), I wanted to make sure we are able to use it.
> 
> Do you see any reason why we could not replace our version triplets
>> with
> clang::VersionTuple ?
> 
> cheers,
> pl
> ___
> 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
>> 
>> 
>>> ___
>>> lldb-dev mailing list
>>> lldb-dev@lists.llvm.org
>>> http://lists.llvm.org/cgi-bin/mailman/

Re: [lldb-dev] clang::VersionTuple

[Pavel Labath via lldb-dev]

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  to switch over our version
handling to that class.

Could I interest anyone in taking a quick look at the patch?

pl

On Wed, 9 May 2018 at 08:49, Pavel Labath  wrote:
>
> Thank you all for the feedback. I'll get on with this when I find some
> spare time. I will send a patch which will show the final look of the code,
> before I start moving VersionTuple into llvm.
>
> cheers,
> pl
> On Tue, 8 May 2018 at 19:46, Frédéric Riss via lldb-dev <
> lldb-dev@lists.llvm.org> wrote:
>
>
>
> > On May 8, 2018, at 11:37 AM, Greg Clayton  wrote:
>
> > I was referring to the Swift and Apple internal branches. They tend to
> lock down against older llvm and clang repositories so when we put changes
> in llvm or clang that are required for LLDB, it makes merging a bit tougher
> in those cases. Again, I am not affected by this, just trying to watch out
> for you guys.
>
>
> > I understand and I appreciate it, I was just worried that I’m missing
> something obvious. We branch LLDB at the same time as LLVM so that’s not
> actually an issue. Of course, it might cause merge conflicts or make it
> harder to cherry-pick patches, but that's just living downstream.
>
> > Fred
>
> > Greg
>
>
> > On May 8, 2018, at 11:35 AM, Greg Clayton  wrote:
>
> > I'm good if Apple is good.
>
> > On May 8, 2018, at 11:31 AM, Frédéric Riss  wrote:
>
>
>
> > On May 8, 2018, at 10:04 AM, Greg Clayton via lldb-dev <
> lldb-dev@lists.llvm.org> wrote:
>
>
>
> > On May 8, 2018, at 9:47 AM, Zachary Turner  wrote:
>
> > We don’t want the lowest levels of lldb to depend on clang. If this is
> useful we should move it from clang to llvm and use llvm::VersionTuple
>
>
> > I agree, though this move will cause merging issues for many that have
> repositories that link against older llvm/clang. Doesn't affect me anymore,
> but Apple will be affected.
>
>
> > I’m not sure I understand what issues you’re referring to, we don’t link
> new LLDBs to old clangs (and even if we did, it wouldn’t be something the
> that drives community decisions).
>
> > Fred
>
> > Greg
>
> > On Tue, May 8, 2018 at 9:26 AM Greg Clayton via lldb-dev <
> lldb-dev@lists.llvm.org> wrote:
>
> >> No issues from me.
>
> >> > On May 8, 2018, at 9:11 AM, Pavel Labath via lldb-dev <
> lldb-dev@lists.llvm.org> wrote:
> >> >
> >> > While moving Args around, I noticed that we have a bunch of
> >> > functions/classes that pass/store version numbers as a triplet of
> integers
> >> > (e.g. Platform::GetOSVersion). I got halfway into creating a wrapper
> class
> >> > for that when I noticed clang::VersionTuple, which is pretty much what
> I
> >> > wanted out of the box.
> >> >
> >> > Now there are small differences between this class, and what we have
> now:
> >> > it has an extra fourth "build" field, and it uses only 31 bits to
> represent
> >> >  the values. None of these seem to matter (particularly as we are
> >> > converting our representation into this struct in some places) that
> much,
> >> > but before I go through the trouble of pulling this class into llvm
> >> > (although technically possible, it seems wrong to pull a clang
> dependency
> >> > at such a low level), I wanted to make sure we are able to use it.
> >> >
> >> > Do you see any reason why we could not replace our version triplets
> with
> >> > clang::VersionTuple ?
> >> >
> >> > cheers,
> >> > pl
> >> > ___
> >> > 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
>
>
> > ___
> > 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
___
lldb-dev mailing list
lldb-dev@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev

Re: [lldb-dev] clang::VersionTuple

[Pavel Labath via lldb-dev]

Thank you all for the feedback. I'll get on with this when I find some
spare time. I will send a patch which will show the final look of the code,
before I start moving VersionTuple into llvm.

cheers,
pl
On Tue, 8 May 2018 at 19:46, Frédéric Riss via lldb-dev <
lldb-dev@lists.llvm.org> wrote:



> On May 8, 2018, at 11:37 AM, Greg Clayton  wrote:

> I was referring to the Swift and Apple internal branches. They tend to
lock down against older llvm and clang repositories so when we put changes
in llvm or clang that are required for LLDB, it makes merging a bit tougher
in those cases. Again, I am not affected by this, just trying to watch out
for you guys.


> I understand and I appreciate it, I was just worried that I’m missing
something obvious. We branch LLDB at the same time as LLVM so that’s not
actually an issue. Of course, it might cause merge conflicts or make it
harder to cherry-pick patches, but that's just living downstream.

> Fred

> Greg


> On May 8, 2018, at 11:35 AM, Greg Clayton  wrote:

> I'm good if Apple is good.

> On May 8, 2018, at 11:31 AM, Frédéric Riss  wrote:



> On May 8, 2018, at 10:04 AM, Greg Clayton via lldb-dev <
lldb-dev@lists.llvm.org> wrote:



> On May 8, 2018, at 9:47 AM, Zachary Turner  wrote:

> We don’t want the lowest levels of lldb to depend on clang. If this is
useful we should move it from clang to llvm and use llvm::VersionTuple


> I agree, though this move will cause merging issues for many that have
repositories that link against older llvm/clang. Doesn't affect me anymore,
but Apple will be affected.


> I’m not sure I understand what issues you’re referring to, we don’t link
new LLDBs to old clangs (and even if we did, it wouldn’t be something the
that drives community decisions).

> Fred

> Greg

> On Tue, May 8, 2018 at 9:26 AM Greg Clayton via lldb-dev <
lldb-dev@lists.llvm.org> wrote:

>> No issues from me.

>> > On May 8, 2018, at 9:11 AM, Pavel Labath via lldb-dev <
lldb-dev@lists.llvm.org> wrote:
>> >
>> > While moving Args around, I noticed that we have a bunch of
>> > functions/classes that pass/store version numbers as a triplet of
integers
>> > (e.g. Platform::GetOSVersion). I got halfway into creating a wrapper
class
>> > for that when I noticed clang::VersionTuple, which is pretty much what
I
>> > wanted out of the box.
>> >
>> > Now there are small differences between this class, and what we have
now:
>> > it has an extra fourth "build" field, and it uses only 31 bits to
represent
>> >  the values. None of these seem to matter (particularly as we are
>> > converting our representation into this struct in some places) that
much,
>> > but before I go through the trouble of pulling this class into llvm
>> > (although technically possible, it seems wrong to pull a clang
dependency
>> > at such a low level), I wanted to make sure we are able to use it.
>> >
>> > Do you see any reason why we could not replace our version triplets
with
>> > clang::VersionTuple ?
>> >
>> > cheers,
>> > pl
>> > ___
>> > 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


> ___
> 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
___
lldb-dev mailing list
lldb-dev@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev

Re: [lldb-dev] clang::VersionTuple

[Frédéric Riss via lldb-dev]

> On May 8, 2018, at 11:37 AM, Greg Clayton  wrote:
> 
> I was referring to the Swift and Apple internal branches. They tend to lock 
> down against older llvm and clang repositories so when we put changes in llvm 
> or clang that are required for LLDB, it makes merging a bit tougher in those 
> cases. Again, I am not affected by this, just trying to watch out for you 
> guys.

I understand and I appreciate it, I was just worried that I’m missing something 
obvious. We branch LLDB at the same time as LLVM so that’s not actually an 
issue. Of course, it might cause merge conflicts or make it harder to 
cherry-pick patches, but that's just living downstream.

Fred 

> Greg
> 
> 
>> On May 8, 2018, at 11:35 AM, Greg Clayton > > wrote:
>> 
>> I'm good if Apple is good. 
>> 
>>> On May 8, 2018, at 11:31 AM, Frédéric Riss >> > wrote:
>>> 
>>> 
>>> 
 On May 8, 2018, at 10:04 AM, Greg Clayton via lldb-dev 
 mailto:lldb-dev@lists.llvm.org>> wrote:
 
 
 
> On May 8, 2018, at 9:47 AM, Zachary Turner  > wrote:
> 
> We don’t want the lowest levels of lldb to depend on clang. If this is 
> useful we should move it from clang to llvm and use llvm::VersionTuple
 
 I agree, though this move will cause merging issues for many that have 
 repositories that link against older llvm/clang. Doesn't affect me 
 anymore, but Apple will be affected.
>>> 
>>> I’m not sure I understand what issues you’re referring to, we don’t link 
>>> new LLDBs to old clangs (and even if we did, it wouldn’t be something the 
>>> that drives community decisions).
>>> 
>>> Fred
>>> 
 Greg
 
> On Tue, May 8, 2018 at 9:26 AM Greg Clayton via lldb-dev 
> mailto:lldb-dev@lists.llvm.org>> wrote:
> No issues from me.
> 
> > On May 8, 2018, at 9:11 AM, Pavel Labath via lldb-dev 
> > mailto:lldb-dev@lists.llvm.org>> wrote:
> > 
> > While moving Args around, I noticed that we have a bunch of
> > functions/classes that pass/store version numbers as a triplet of 
> > integers
> > (e.g. Platform::GetOSVersion). I got halfway into creating a wrapper 
> > class
> > for that when I noticed clang::VersionTuple, which is pretty much what I
> > wanted out of the box.
> > 
> > Now there are small differences between this class, and what we have 
> > now:
> > it has an extra fourth "build" field, and it uses only 31 bits to 
> > represent
> >  the values. None of these seem to matter (particularly as we are
> > converting our representation into this struct in some places) that 
> > much,
> > but before I go through the trouble of pulling this class into llvm
> > (although technically possible, it seems wrong to pull a clang 
> > dependency
> > at such a low level), I wanted to make sure we are able to use it.
> > 
> > Do you see any reason why we could not replace our version triplets with
> > clang::VersionTuple ?
> > 
> > cheers,
> > pl
> > ___
> > 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 
> 
 
 ___
 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

Re: [lldb-dev] clang::VersionTuple

[Greg Clayton via lldb-dev]

I was referring to the Swift and Apple internal branches. They tend to lock 
down against older llvm and clang repositories so when we put changes in llvm 
or clang that are required for LLDB, it makes merging a bit tougher in those 
cases. Again, I am not affected by this, just trying to watch out for you guys.

Greg


> On May 8, 2018, at 11:35 AM, Greg Clayton  wrote:
> 
> I'm good if Apple is good. 
> 
>> On May 8, 2018, at 11:31 AM, Frédéric Riss > > wrote:
>> 
>> 
>> 
>>> On May 8, 2018, at 10:04 AM, Greg Clayton via lldb-dev 
>>> mailto:lldb-dev@lists.llvm.org>> wrote:
>>> 
>>> 
>>> 
 On May 8, 2018, at 9:47 AM, Zachary Turner >>> > wrote:
 
 We don’t want the lowest levels of lldb to depend on clang. If this is 
 useful we should move it from clang to llvm and use llvm::VersionTuple
>>> 
>>> I agree, though this move will cause merging issues for many that have 
>>> repositories that link against older llvm/clang. Doesn't affect me anymore, 
>>> but Apple will be affected.
>> 
>> I’m not sure I understand what issues you’re referring to, we don’t link new 
>> LLDBs to old clangs (and even if we did, it wouldn’t be something the that 
>> drives community decisions).
>> 
>> Fred
>> 
>>> Greg
>>> 
 On Tue, May 8, 2018 at 9:26 AM Greg Clayton via lldb-dev 
 mailto:lldb-dev@lists.llvm.org>> wrote:
 No issues from me.
 
 > On May 8, 2018, at 9:11 AM, Pavel Labath via lldb-dev 
 > mailto:lldb-dev@lists.llvm.org>> wrote:
 > 
 > While moving Args around, I noticed that we have a bunch of
 > functions/classes that pass/store version numbers as a triplet of 
 > integers
 > (e.g. Platform::GetOSVersion). I got halfway into creating a wrapper 
 > class
 > for that when I noticed clang::VersionTuple, which is pretty much what I
 > wanted out of the box.
 > 
 > Now there are small differences between this class, and what we have now:
 > it has an extra fourth "build" field, and it uses only 31 bits to 
 > represent
 >  the values. None of these seem to matter (particularly as we are
 > converting our representation into this struct in some places) that much,
 > but before I go through the trouble of pulling this class into llvm
 > (although technically possible, it seems wrong to pull a clang dependency
 > at such a low level), I wanted to make sure we are able to use it.
 > 
 > Do you see any reason why we could not replace our version triplets with
 > clang::VersionTuple ?
 > 
 > cheers,
 > pl
 > ___
 > 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 
 
>>> 
>>> ___
>>> 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

Re: [lldb-dev] clang::VersionTuple

[Greg Clayton via lldb-dev]

I'm good if Apple is good. 

> On May 8, 2018, at 11:31 AM, Frédéric Riss  wrote:
> 
> 
> 
>> On May 8, 2018, at 10:04 AM, Greg Clayton via lldb-dev 
>> mailto:lldb-dev@lists.llvm.org>> wrote:
>> 
>> 
>> 
>>> On May 8, 2018, at 9:47 AM, Zachary Turner >> > wrote:
>>> 
>>> We don’t want the lowest levels of lldb to depend on clang. If this is 
>>> useful we should move it from clang to llvm and use llvm::VersionTuple
>> 
>> I agree, though this move will cause merging issues for many that have 
>> repositories that link against older llvm/clang. Doesn't affect me anymore, 
>> but Apple will be affected.
> 
> I’m not sure I understand what issues you’re referring to, we don’t link new 
> LLDBs to old clangs (and even if we did, it wouldn’t be something the that 
> drives community decisions).
> 
> Fred
> 
>> Greg
>> 
>>> On Tue, May 8, 2018 at 9:26 AM Greg Clayton via lldb-dev 
>>> mailto:lldb-dev@lists.llvm.org>> wrote:
>>> No issues from me.
>>> 
>>> > On May 8, 2018, at 9:11 AM, Pavel Labath via lldb-dev 
>>> > mailto:lldb-dev@lists.llvm.org>> wrote:
>>> > 
>>> > While moving Args around, I noticed that we have a bunch of
>>> > functions/classes that pass/store version numbers as a triplet of integers
>>> > (e.g. Platform::GetOSVersion). I got halfway into creating a wrapper class
>>> > for that when I noticed clang::VersionTuple, which is pretty much what I
>>> > wanted out of the box.
>>> > 
>>> > Now there are small differences between this class, and what we have now:
>>> > it has an extra fourth "build" field, and it uses only 31 bits to 
>>> > represent
>>> >  the values. None of these seem to matter (particularly as we are
>>> > converting our representation into this struct in some places) that much,
>>> > but before I go through the trouble of pulling this class into llvm
>>> > (although technically possible, it seems wrong to pull a clang dependency
>>> > at such a low level), I wanted to make sure we are able to use it.
>>> > 
>>> > Do you see any reason why we could not replace our version triplets with
>>> > clang::VersionTuple ?
>>> > 
>>> > cheers,
>>> > pl
>>> > ___
>>> > 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 
>>> 
>> 
>> ___
>> 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

Re: [lldb-dev] clang::VersionTuple

[Frédéric Riss via lldb-dev]

> On May 8, 2018, at 10:04 AM, Greg Clayton via lldb-dev 
>  wrote:
> 
> 
> 
>> On May 8, 2018, at 9:47 AM, Zachary Turner > > wrote:
>> 
>> We don’t want the lowest levels of lldb to depend on clang. If this is 
>> useful we should move it from clang to llvm and use llvm::VersionTuple
> 
> I agree, though this move will cause merging issues for many that have 
> repositories that link against older llvm/clang. Doesn't affect me anymore, 
> but Apple will be affected.

I’m not sure I understand what issues you’re referring to, we don’t link new 
LLDBs to old clangs (and even if we did, it wouldn’t be something the that 
drives community decisions).

Fred

> Greg
> 
>> On Tue, May 8, 2018 at 9:26 AM Greg Clayton via lldb-dev 
>> mailto:lldb-dev@lists.llvm.org>> wrote:
>> No issues from me.
>> 
>> > On May 8, 2018, at 9:11 AM, Pavel Labath via lldb-dev 
>> > mailto:lldb-dev@lists.llvm.org>> wrote:
>> > 
>> > While moving Args around, I noticed that we have a bunch of
>> > functions/classes that pass/store version numbers as a triplet of integers
>> > (e.g. Platform::GetOSVersion). I got halfway into creating a wrapper class
>> > for that when I noticed clang::VersionTuple, which is pretty much what I
>> > wanted out of the box.
>> > 
>> > Now there are small differences between this class, and what we have now:
>> > it has an extra fourth "build" field, and it uses only 31 bits to represent
>> >  the values. None of these seem to matter (particularly as we are
>> > converting our representation into this struct in some places) that much,
>> > but before I go through the trouble of pulling this class into llvm
>> > (although technically possible, it seems wrong to pull a clang dependency
>> > at such a low level), I wanted to make sure we are able to use it.
>> > 
>> > Do you see any reason why we could not replace our version triplets with
>> > clang::VersionTuple ?
>> > 
>> > cheers,
>> > pl
>> > ___
>> > 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 
>> 
> 
> ___
> 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

Re: [lldb-dev] clang::VersionTuple

[Davide Italiano via lldb-dev]

Fred is probably the person who can answer this question



On Tue, May 8, 2018 at 11:06 AM, Zachary Turner  wrote:
> +davide, +aprantl for the Apple perspective.
>
>
> On Tue, May 8, 2018 at 10:04 AM Greg Clayton  wrote:
>>
>>
>> On May 8, 2018, at 9:47 AM, Zachary Turner  wrote:
>>
>> We don’t want the lowest levels of lldb to depend on clang. If this is
>> useful we should move it from clang to llvm and use llvm::VersionTuple
>>
>>
>> I agree, though this move will cause merging issues for many that have
>> repositories that link against older llvm/clang. Doesn't affect me anymore,
>> but Apple will be affected.
>>
>> Greg
>>
>> On Tue, May 8, 2018 at 9:26 AM Greg Clayton via lldb-dev
>>  wrote:
>>>
>>> No issues from me.
>>>
>>> > On May 8, 2018, at 9:11 AM, Pavel Labath via lldb-dev
>>> >  wrote:
>>> >
>>> > While moving Args around, I noticed that we have a bunch of
>>> > functions/classes that pass/store version numbers as a triplet of
>>> > integers
>>> > (e.g. Platform::GetOSVersion). I got halfway into creating a wrapper
>>> > class
>>> > for that when I noticed clang::VersionTuple, which is pretty much what
>>> > I
>>> > wanted out of the box.
>>> >
>>> > Now there are small differences between this class, and what we have
>>> > now:
>>> > it has an extra fourth "build" field, and it uses only 31 bits to
>>> > represent
>>> >  the values. None of these seem to matter (particularly as we are
>>> > converting our representation into this struct in some places) that
>>> > much,
>>> > but before I go through the trouble of pulling this class into llvm
>>> > (although technically possible, it seems wrong to pull a clang
>>> > dependency
>>> > at such a low level), I wanted to make sure we are able to use it.
>>> >
>>> > Do you see any reason why we could not replace our version triplets
>>> > with
>>> > clang::VersionTuple ?
>>> >
>>> > cheers,
>>> > pl
>>> > ___
>>> > 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
>>
>>
>
___
lldb-dev mailing list
lldb-dev@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev

Re: [lldb-dev] clang::VersionTuple

[Zachary Turner via lldb-dev]

+davide, +aprantl for the Apple perspective.

On Tue, May 8, 2018 at 10:04 AM Greg Clayton  wrote:

>
> On May 8, 2018, at 9:47 AM, Zachary Turner  wrote:
>
> We don’t want the lowest levels of lldb to depend on clang. If this is
> useful we should move it from clang to llvm and use llvm::VersionTuple
>
>
> I agree, though this move will cause merging issues for many that have
> repositories that link against older llvm/clang. Doesn't affect me anymore,
> but Apple will be affected.
>
> Greg
>
> On Tue, May 8, 2018 at 9:26 AM Greg Clayton via lldb-dev <
> lldb-dev@lists.llvm.org> wrote:
>
>> No issues from me.
>>
>> > On May 8, 2018, at 9:11 AM, Pavel Labath via lldb-dev <
>> lldb-dev@lists.llvm.org> wrote:
>> >
>> > While moving Args around, I noticed that we have a bunch of
>> > functions/classes that pass/store version numbers as a triplet of
>> integers
>> > (e.g. Platform::GetOSVersion). I got halfway into creating a wrapper
>> class
>> > for that when I noticed clang::VersionTuple, which is pretty much what I
>> > wanted out of the box.
>> >
>> > Now there are small differences between this class, and what we have
>> now:
>> > it has an extra fourth "build" field, and it uses only 31 bits to
>> represent
>> >  the values. None of these seem to matter (particularly as we are
>> > converting our representation into this struct in some places) that
>> much,
>> > but before I go through the trouble of pulling this class into llvm
>> > (although technically possible, it seems wrong to pull a clang
>> dependency
>> > at such a low level), I wanted to make sure we are able to use it.
>> >
>> > Do you see any reason why we could not replace our version triplets with
>> > clang::VersionTuple ?
>> >
>> > cheers,
>> > pl
>> > ___
>> > 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
>>
>
>
___
lldb-dev mailing list
lldb-dev@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev

Re: [lldb-dev] clang::VersionTuple

[Greg Clayton via lldb-dev]

> On May 8, 2018, at 9:47 AM, Zachary Turner  wrote:
> 
> We don’t want the lowest levels of lldb to depend on clang. If this is useful 
> we should move it from clang to llvm and use llvm::VersionTuple

I agree, though this move will cause merging issues for many that have 
repositories that link against older llvm/clang. Doesn't affect me anymore, but 
Apple will be affected.

Greg

> On Tue, May 8, 2018 at 9:26 AM Greg Clayton via lldb-dev 
> mailto:lldb-dev@lists.llvm.org>> wrote:
> No issues from me.
> 
> > On May 8, 2018, at 9:11 AM, Pavel Labath via lldb-dev 
> > mailto:lldb-dev@lists.llvm.org>> wrote:
> > 
> > While moving Args around, I noticed that we have a bunch of
> > functions/classes that pass/store version numbers as a triplet of integers
> > (e.g. Platform::GetOSVersion). I got halfway into creating a wrapper class
> > for that when I noticed clang::VersionTuple, which is pretty much what I
> > wanted out of the box.
> > 
> > Now there are small differences between this class, and what we have now:
> > it has an extra fourth "build" field, and it uses only 31 bits to represent
> >  the values. None of these seem to matter (particularly as we are
> > converting our representation into this struct in some places) that much,
> > but before I go through the trouble of pulling this class into llvm
> > (although technically possible, it seems wrong to pull a clang dependency
> > at such a low level), I wanted to make sure we are able to use it.
> > 
> > Do you see any reason why we could not replace our version triplets with
> > clang::VersionTuple ?
> > 
> > cheers,
> > pl
> > ___
> > 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 
> 

___
lldb-dev mailing list
lldb-dev@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev

Re: [lldb-dev] clang::VersionTuple

[Zachary Turner via lldb-dev]

We don’t want the lowest levels of lldb to depend on clang. If this is
useful we should move it from clang to llvm and use llvm::VersionTuple
On Tue, May 8, 2018 at 9:26 AM Greg Clayton via lldb-dev <
lldb-dev@lists.llvm.org> wrote:

> No issues from me.
>
> > On May 8, 2018, at 9:11 AM, Pavel Labath via lldb-dev <
> lldb-dev@lists.llvm.org> wrote:
> >
> > While moving Args around, I noticed that we have a bunch of
> > functions/classes that pass/store version numbers as a triplet of
> integers
> > (e.g. Platform::GetOSVersion). I got halfway into creating a wrapper
> class
> > for that when I noticed clang::VersionTuple, which is pretty much what I
> > wanted out of the box.
> >
> > Now there are small differences between this class, and what we have now:
> > it has an extra fourth "build" field, and it uses only 31 bits to
> represent
> >  the values. None of these seem to matter (particularly as we are
> > converting our representation into this struct in some places) that much,
> > but before I go through the trouble of pulling this class into llvm
> > (although technically possible, it seems wrong to pull a clang dependency
> > at such a low level), I wanted to make sure we are able to use it.
> >
> > Do you see any reason why we could not replace our version triplets with
> > clang::VersionTuple ?
> >
> > cheers,
> > pl
> > ___
> > 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
>
___
lldb-dev mailing list
lldb-dev@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev

Re: [lldb-dev] clang::VersionTuple

[Greg Clayton via lldb-dev]

No issues from me.

> On May 8, 2018, at 9:11 AM, Pavel Labath via lldb-dev 
>  wrote:
> 
> While moving Args around, I noticed that we have a bunch of
> functions/classes that pass/store version numbers as a triplet of integers
> (e.g. Platform::GetOSVersion). I got halfway into creating a wrapper class
> for that when I noticed clang::VersionTuple, which is pretty much what I
> wanted out of the box.
> 
> Now there are small differences between this class, and what we have now:
> it has an extra fourth "build" field, and it uses only 31 bits to represent
>  the values. None of these seem to matter (particularly as we are
> converting our representation into this struct in some places) that much,
> but before I go through the trouble of pulling this class into llvm
> (although technically possible, it seems wrong to pull a clang dependency
> at such a low level), I wanted to make sure we are able to use it.
> 
> Do you see any reason why we could not replace our version triplets with
> clang::VersionTuple ?
> 
> cheers,
> pl
> ___
> 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

[lldb-dev] clang::VersionTuple

[Pavel Labath via lldb-dev]

While moving Args around, I noticed that we have a bunch of
functions/classes that pass/store version numbers as a triplet of integers
(e.g. Platform::GetOSVersion). I got halfway into creating a wrapper class
for that when I noticed clang::VersionTuple, which is pretty much what I
wanted out of the box.

Now there are small differences between this class, and what we have now:
it has an extra fourth "build" field, and it uses only 31 bits to represent
  the values. None of these seem to matter (particularly as we are
converting our representation into this struct in some places) that much,
but before I go through the trouble of pulling this class into llvm
(although technically possible, it seems wrong to pull a clang dependency
at such a low level), I wanted to make sure we are able to use it.

Do you see any reason why we could not replace our version triplets with
clang::VersionTuple ?

cheers,
pl
___
lldb-dev mailing list
lldb-dev@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev