Mailing List Archive

Re: [mythtv-commits] [MythTV/mythtv] 704f5e: android: Fix problem with frontend setup
On Mon, 27 Jan 2020 at 01:31, Peter Bennett <noreply@github.com> wrote:

> Branch: refs/heads/master
> Home: https://github.com/MythTV/mythtv
> Commit: 704f5ef3660792b80709d17ebe24724326de3b4a
>
> https://github.com/MythTV/mythtv/commit/704f5ef3660792b80709d17ebe24724326de3b4a
> Author: Peter Bennett <pbennett@mythtv.org>
> Date: 2020-01-26 (Sun, 26 Jan 2020)
>
> Changed paths:
> M mythtv/libs/libmyth/standardsettings.cpp
> M mythtv/libs/libmythui/mythdialogbox.cpp
>
> Log Message:
> -----------
> android: Fix problem with frontend setup
>
> using auto for a dynamic cast was not working. Changed to
> appropriate object type.
>
> Can this be an issue in more places in the code?

Klaas.
Re: [mythtv-commits] [MythTV/mythtv] 704f5e: android: Fix problem with frontend setup [ In reply to ]
On 1/27/20 1:47 AM, Klaas de Waal wrote:
>
>
> On Mon, 27 Jan 2020 at 01:31, Peter Bennett <noreply@github.com
> <mailto:noreply@github.com>> wrote:
>
>   Branch: refs/heads/master
>   Home: https://github.com/MythTV/mythtv
>   Commit: 704f5ef3660792b80709d17ebe24724326de3b4a
> https://github.com/MythTV/mythtv/commit/704f5ef3660792b80709d17ebe24724326de3b4a
>   Author: Peter Bennett <pbennett@mythtv.org
> <mailto:pbennett@mythtv.org>>
>   Date:   2020-01-26 (Sun, 26 Jan 2020)
>
>   Changed paths:
>     M mythtv/libs/libmyth/standardsettings.cpp
>     M mythtv/libs/libmythui/mythdialogbox.cpp
>
>   Log Message:
>   -----------
>   android: Fix problem with frontend setup
>
> using auto for a dynamic cast was not working. Changed to
> appropriate object type.
>
> Can this be an issue in more places in the code?
>
> Klaas.
>
>
Hi Klaas

I think it could be. This came up with Mark Spieth's packaging change
to  use a new compiler. I will look through the code to see if there are
many uses of "auto" and whether they are working on android. I am not
very familiar with "auto" and its usage, but in this case it seemed
unnecessary because you could see the type right there in the code.
Other places where they assign auto to the result of a complicated
expression may be more difficult to fix if they are not working. What
was particularly confusing was that the code seemed to use "auto" and
"auto *" interchangeably when creating a pointer. This was working in
linux and the old compiler but not the new compiler.

Peter
Re: [mythtv-commits] [MythTV/mythtv] 704f5e: android: Fix problem with frontend setup [ In reply to ]
On Mon, 2020-01-27 at 09:00 -0500, Peter Bennett wrote:
>
>
> On 1/27/20 1:47 AM, Klaas de Waal wrote:
> >
> > On Mon, 27 Jan 2020 at 01:31, Peter Bennett <noreply@github.com>
> > wrote:
> > > Branch: refs/heads/master
> > > Home: https://github.com/MythTV/mythtv
> > > Commit: 704f5ef3660792b80709d17ebe24724326de3b4a
> > >
> > > https://github.com/MythTV/mythtv/commit/704f5ef3660792b80709d17ebe24724326de3b4a
> > > Author: Peter Bennett <pbennett@mythtv.org>
> > > Date: 2020-01-26 (Sun, 26 Jan 2020)
> > >
> > > Changed paths:
> > > M mythtv/libs/libmyth/standardsettings.cpp
> > > M mythtv/libs/libmythui/mythdialogbox.cpp
> > >
> > > Log Message:
> > > -----------
> > > android: Fix problem with frontend setup
> > >
> > > using auto for a dynamic cast was not working. Changed to
> > > appropriate object type.
> > >
> > >
> >
> > Can this be an issue in more places in the code?
> >
> > Klaas.
> >
> >
> Hi Klaas
>
> I think it could be. This came up with Mark Spieth's packaging change
> to use a new compiler. I will look through the code to see if there
> are many uses of "auto" and whether they are working on android.

Hi Peter,

I noticed that your commit also included a change from dynamic_cast to
static_cast. Those are very different animals, and that's far more
likely the cause of the problem. The former happens at runtime and can
fail and return nullptr; the latter happens at compile time. Could
that fix be what made the code work again, not the auto changes?

> I am not very familiar with "auto" and its usage, but in this case
> it seemed unnecessary because you could see the type right there in
> the code.

One of the reasons to use auto is that you don't have to repeat the
same type name in a single line of code. :-) I know for simple short
time names its just as easy to write the type twice, bit for long
complicated names its easier to write "auto".

> Other places where they assign auto to the result of a complicated
> expression may be more difficult to fix if they are not working. What
> was particularly confusing was that the code seemed to use "auto" and
> "auto *" interchangeably when creating a pointer.

Yes, that's a little confusing. If you use the "auto" keyword by
itself, it does figure out that that right side of the equals sign is a
pointer and will correctly use a pointer. "Auto *" deduces the exact
same type, but it reads easier to us humans. There's newer code in
clang-11 to ensure that it replaces duplicated type declarations with
"auto", "auto *", "auto &", or "const auto *" where appropriate. All
essentially work out the same to the compiler, but the latter three are
more readable to humans. I have a fix in my working branch that
updates a number of instance of "auto" to be "auto *". I was waiting
until after the v31 split to merge the changes, but I can cherry-pick
that fix into master if the group wants.

> This was working in linux and the old compiler but not the new
> compiler.

Sound like I need to update my setup for the new compiler and sdk/ndk.

David



_______________________________________________
mythtv-dev mailing list
mythtv-dev@mythtv.org
http://lists.mythtv.org/mailman/listinfo/mythtv-dev
http://wiki.mythtv.org/Mailing_List_etiquette
MythTV Forums: https://forum.mythtv.org
Re: [mythtv-commits] [MythTV/mythtv] 704f5e: android: Fix problem with frontend setup [ In reply to ]
On 1/27/20 11:04 AM, David Hampton wrote:
> On Mon, 2020-01-27 at 09:00 -0500, Peter Bennett wrote:
>>
>> On 1/27/20 1:47 AM, Klaas de Waal wrote:
>>> On Mon, 27 Jan 2020 at 01:31, Peter Bennett <noreply@github.com>
>>> wrote:
>>>> Branch: refs/heads/master
>>>> Home: https://github.com/MythTV/mythtv
>>>> Commit: 704f5ef3660792b80709d17ebe24724326de3b4a
>>>>
>>>> https://github.com/MythTV/mythtv/commit/704f5ef3660792b80709d17ebe24724326de3b4a
>>>> Author: Peter Bennett <pbennett@mythtv.org>
>>>> Date: 2020-01-26 (Sun, 26 Jan 2020)
>>>>
>>>> Changed paths:
>>>> M mythtv/libs/libmyth/standardsettings.cpp
>>>> M mythtv/libs/libmythui/mythdialogbox.cpp
>>>>
>>>> Log Message:
>>>> -----------
>>>> android: Fix problem with frontend setup
>>>>
>>>> using auto for a dynamic cast was not working. Changed to
>>>> appropriate object type.
>>>>
>>>>
>>> Can this be an issue in more places in the code?
>>>
>>> Klaas.
>>>
>>>
>> Hi Klaas
>>
>> I think it could be. This came up with Mark Spieth's packaging change
>> to use a new compiler. I will look through the code to see if there
>> are many uses of "auto" and whether they are working on android.
> Hi Peter,
>
> I noticed that your commit also included a change from dynamic_cast to
> static_cast. Those are very different animals, and that's far more
> likely the cause of the problem. The former happens at runtime and can
> fail and return nullptr; the latter happens at compile time. Could
> that fix be what made the code work again, not the auto changes?

Oops - mistake - I tried static_cast and then changed it back to
dynamic_cast (at least I thought I did). Something slipped through the
cracks. I will revisit that.  Note in this case static cast should not
cause a failure since the line above checks the type. However I will try
dynamic_cast again and see if I can sort that out correctly.
>> I am not very familiar with "auto" and its usage, but in this case
>> it seemed unnecessary because you could see the type right there in
>> the code.
> One of the reasons to use auto is that you don't have to repeat the
> same type name in a single line of code. :-) I know for simple short
> time names its just as easy to write the type twice, bit for long
> complicated names its easier to write "auto".


>> Other places where they assign auto to the result of a complicated
>> expression may be more difficult to fix if they are not working. What
>> was particularly confusing was that the code seemed to use "auto" and
>> "auto *" interchangeably when creating a pointer.
> Yes, that's a little confusing. If you use the "auto" keyword by
> itself, it does figure out that that right side of the equals sign is a
> pointer and will correctly use a pointer. "Auto *" deduces the exact
> same type, but it reads easier to us humans. There's newer code in
> clang-11 to ensure that it replaces duplicated type declarations with
> "auto", "auto *", "auto &", or "const auto *" where appropriate. All
> essentially work out the same to the compiler, but the latter three are
> more readable to humans. I have a fix in my working branch that
> updates a number of instance of "auto" to be "auto *". I was waiting
> until after the v31 split to merge the changes, but I can cherry-pick
> that fix into master if the group wants.
>
>> This was working in linux and the old compiler but not the new
>> compiler.
> Sound like I need to update my setup for the new compiler and sdk/ndk.
>
> David
>
>
>
> _______________________________________________
> mythtv-dev mailing list
> mythtv-dev@mythtv.org
> http://lists.mythtv.org/mailman/listinfo/mythtv-dev
> http://wiki.mythtv.org/Mailing_List_etiquette
> MythTV Forums: https://forum.mythtv.org

_______________________________________________
mythtv-dev mailing list
mythtv-dev@mythtv.org
http://lists.mythtv.org/mailman/listinfo/mythtv-dev
http://wiki.mythtv.org/Mailing_List_etiquette
MythTV Forums: https://forum.mythtv.org
Re: [mythtv-commits] [MythTV/mythtv] 704f5e: android: Fix problem with frontend setup [ In reply to ]
On Mon, Jan 27, 2020 at 11:04:21AM -0500, David Hampton wrote:
> On Mon, 2020-01-27 at 09:00 -0500, Peter Bennett wrote:
> >
> >
> > On 1/27/20 1:47 AM, Klaas de Waal wrote:
> > >
> > > On Mon, 27 Jan 2020 at 01:31, Peter Bennett <noreply@github.com>
> > > wrote:
> > > > Branch: refs/heads/master
> > > > Home: https://github.com/MythTV/mythtv
> > > > Commit: 704f5ef3660792b80709d17ebe24724326de3b4a
> > > >
> > > > https://github.com/MythTV/mythtv/commit/704f5ef3660792b80709d17ebe24724326de3b4a
> > > > Author: Peter Bennett <pbennett@mythtv.org>
> > > > Date: 2020-01-26 (Sun, 26 Jan 2020)
> > > >
> > > > Changed paths:
> > > > M mythtv/libs/libmyth/standardsettings.cpp
> > > > M mythtv/libs/libmythui/mythdialogbox.cpp
> > > >
> > > > Log Message:
> > > > -----------
> > > > android: Fix problem with frontend setup
> > > >
> > > > using auto for a dynamic cast was not working. Changed to
> > > > appropriate object type.
> > > >
> > > >
> > >
> > > Can this be an issue in more places in the code?
> > >
> > > Klaas.
> > >
> > >
> > Hi Klaas
> >
> > I think it could be. This came up with Mark Spieth's packaging change
> > to use a new compiler. I will look through the code to see if there
> > are many uses of "auto" and whether they are working on android.
>
> Hi Peter,
>
> I noticed that your commit also included a change from dynamic_cast to
> static_cast. Those are very different animals, and that's far more
> likely the cause of the problem. The former happens at runtime and can
> fail and return nullptr; the latter happens at compile time. Could
> that fix be what made the code work again, not the auto changes?
>
> > I am not very familiar with "auto" and its usage, but in this case
> > it seemed unnecessary because you could see the type right there in
> > the code.
>
> One of the reasons to use auto is that you don't have to repeat the
> same type name in a single line of code. :-) I know for simple short
> time names its just as easy to write the type twice, bit for long
> complicated names its easier to write "auto".
>
> > Other places where they assign auto to the result of a complicated
> > expression may be more difficult to fix if they are not working. What
> > was particularly confusing was that the code seemed to use "auto" and
> > "auto *" interchangeably when creating a pointer.
>
> Yes, that's a little confusing. If you use the "auto" keyword by
> itself, it does figure out that that right side of the equals sign is a
> pointer and will correctly use a pointer. "Auto *" deduces the exact
> same type, but it reads easier to us humans. There's newer code in
> clang-11 to ensure that it replaces duplicated type declarations with
> "auto", "auto *", "auto &", or "const auto *" where appropriate. All
> essentially work out the same to the compiler, but the latter three are
> more readable to humans. I have a fix in my working branch that
> updates a number of instance of "auto" to be "auto *". I was waiting
> until after the v31 split to merge the changes, but I can cherry-pick
> that fix into master if the group wants.
>
> > This was working in linux and the old compiler but not the new
> > compiler.
>
> Sound like I need to update my setup for the new compiler and sdk/ndk.

Has anyone tried for Linux with clang, perticularly the same version
used on Android? That might help to isolate the problem to clang or
Android.

David
--
David Engel
david@istwok.net
_______________________________________________
mythtv-dev mailing list
mythtv-dev@mythtv.org
http://lists.mythtv.org/mailman/listinfo/mythtv-dev
http://wiki.mythtv.org/Mailing_List_etiquette
MythTV Forums: https://forum.mythtv.org