Making Covoar More C++

[Joel Sherrill]

On Wed, Mar 24, 2021, 7:28 PM Chris Johns <chrisj at rtems.org> wrote:

>
>
> On 25/3/21 6:54 am, Joel Sherrill wrote:
> >
> >
> > On Wed, Mar 24, 2021 at 2:42 PM Gedare Bloom <gedare at rtems.org
> > <mailto:gedare at rtems.org>> wrote:
> >
> >     On Wed, Mar 24, 2021 at 1:35 PM Joel Sherrill <joel at rtems.org
> >     <mailto:joel at rtems.org>> wrote:
> >     >
> >     > Hi
> >     >
> >     > There has been a lot of talk about making covoar use more C++
> >     > features. It seems to be an issue on every patch. I almost
> >     > replied to Gedare's comment at the bottom of a patch
> >     > but decided it needed another thread:
> >     >
> >     > "I still struggle reviewing this codebase, in part because it is
> C+C++
> >     > (TM) and in part because I'm not so proficient in C++ to make
> concrete
> >     > recommendations how to write this better. I think, if the goal is
> >     > eventually to make this more C++ like code, then new code coming in
> >     > should aim to move the needle in that direction rather than
> continue
> >     > to propagate the old ways of doing."
> >     >
> >     Thanks for taking this on.
>

Alex's willing to make some of the changes. It's just a matter of having a
baseline so when we make changes they can be tested thoroughly. We now have
about a half dozen architectures producing reports so it is a good baseline.

I was actually thinking that once all of these are merged, it might be a
good place to tag because any git bisect might want to come back to here.

> >
> >     > I personally do NOT want to see changes to C++ in one leaf class
> and
> >     > the other architectures not get the same changes. I would prefer
> to see
> >     > all these corrections and base changes in the same style with
> limited
> >     > changes to C-isms. I'm not opposed to the changes but let's take
> the
> >     > Target class one. There are multiple target classes. Changing one
> >     > independent of the others isn't a good idea.
> >     >
> >     This is reasonable to me.
> >
> >     > I'd like to see us get a working baseline in and then do something
> like
> >     > sweep std::string through Target* as a single patch. This is
> easier to
> >     > test and review since it would only be C string to std::string.
> Perhaps
> >     > switch to C++ output a file at a time. Redo the report output. Etc.
> >     > Discrete chunks instead of piecemeal.
> >     >
> >     > Covoar has spent years broken and some is from changing working
> >     > things to do things "a better way" with no baseline to check
> against.
> >     > We need to get a baseline.
> >     >
> >     > Please. Let's get a working baseline and then approach this more
> >     > methodically. No one is going to suffer from seeing a C string a
> little
> >     > while longer. :)
> >     >
> >     I'm fine, as long as there is a plan in place and some clear
> >     directions. It would help to have tickets to organize the path
> >     forward.
>

+1

One issue is the order of the changes. I at first thought about making the
string changes and then realized that we would end up having C printf and
have to temporarily add c_str a lot. It likely makes more sense to sweep
all the output first and then switch to C++ strings.

I have also asked Alex to put together some diagrams so we can discuss this
better. I think there is some data flow and delivered inheritance in this
program that is not being understood by everyone.

Plus Alex and I talked earlier this evening and think we have a reasonable
path to greatly speed it up without massive overhauls.

>
> >     I'm willing to oblige continued use of C'ism for now, but I want to
> >     know the plan and maybe a deadline of sorts by which I can start to
> be
> >     picky again :) I don't like to be in limbo.
> >
> >
> > I'd love to have a deadline but I can't guarantee how long Alex can
> > work on it. But unless he gets pulled, he can pick on this for a while.
>
> Joel, I pushed a number of changes to move covoar towards C++ back when I
> pushed
> in the ELF and DWARF support. The C++ nature of the interfaces I brought
> in from
> the tool kit required some refactoring. I have not seen much action since
> then
> so Gedare's question seems reasonable.
>

Yes you did and because we did not review the reports, there was a backlog
of issues that had to be fixed. Some from this change some from other
changes. No blame. I just want a better baseline this time.


> The need for this code to be C++ and not a mix comes down to what you want
> to
> see happen. There is no real need to move the code closer to C++ other than
> improving the technical quality and that is about the life of the code in
> the
> project. If you are willing to look after the code as is then I am fine
> with
> that. If you would like to see the code move to C++ and away from C then
> can
> also happen. It would be an interesting way to learn more about C++.
>
> I find the code hard to work on because I do not know if I am looking at C
> and
> needing to use C solutions or C++ and I should be pulling on C++ threads
> and
> where they go. I also see this is an issue for those working on the code.
> In
> some recent patches I pointed out new code being call from C++ code with
> C++
> objects that was written in C. I pointed this out and the next patch was
> fine.
>

There really isn't any straight C code in here. Just C-strings and C style
IO.


> > My guess is that C string to std::string is probably a good pass by
> > itself since method signatures may change.
>
> I think anything you feel can be changed and is in reach is welcome.
>
> > There isn't much file input but that could be a pass by itself
> > along the way,
> >
> > Then sweep output one file at a time leaving reporting for last as
> > a batch.
> >
> > Do you see an order?
>
> Maybe we organise a group session online where we all look over the code
> together and discuss various aspects of the architecture and what it means
> from
> a strictly C vs C++ point of view. Any change like this cannot happen in a
> vacuum and I believe C++ is more of a taught language than C. C styles are
> easier to pick up  by reading code. My introduction to C++ was in the
> mid-90's
> with a Rational training course and documentation. It takes time to
> relearn C
> solutions you know and have at hand in C++.
>

This sounds good.

>
> I have no idea what we would need to hold such a session and who would be
> interested but I am happy to be there and be part of it.
>

Probably just a Google meeting is fine

>
> Chris
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.rtems.org/pipermail/devel/attachments/20210324/3f929263/attachment.html>