<div dir="auto"><div><br><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Wed, Mar 24, 2021, 7:28 PM Chris Johns <<a href="mailto:chrisj@rtems.org" target="_blank" rel="noreferrer">chrisj@rtems.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><br>
<br>
On 25/3/21 6:54 am, Joel Sherrill wrote:<br>
> <br>
> <br>
> On Wed, Mar 24, 2021 at 2:42 PM Gedare Bloom <<a href="mailto:gedare@rtems.org" rel="noreferrer noreferrer" target="_blank">gedare@rtems.org</a><br>
> <mailto:<a href="mailto:gedare@rtems.org" rel="noreferrer noreferrer" target="_blank">gedare@rtems.org</a>>> wrote:<br>
> <br>
> On Wed, Mar 24, 2021 at 1:35 PM Joel Sherrill <<a href="mailto:joel@rtems.org" rel="noreferrer noreferrer" target="_blank">joel@rtems.org</a><br>
> <mailto:<a href="mailto:joel@rtems.org" rel="noreferrer noreferrer" target="_blank">joel@rtems.org</a>>> wrote:<br>
> ><br>
> > Hi<br>
> ><br>
> > There has been a lot of talk about making covoar use more C++<br>
> > features. It seems to be an issue on every patch. I almost<br>
> > replied to Gedare's comment at the bottom of a patch<br>
> > but decided it needed another thread:<br>
> ><br>
> > "I still struggle reviewing this codebase, in part because it is C+C++<br>
> > (TM) and in part because I'm not so proficient in C++ to make concrete<br>
> > recommendations how to write this better. I think, if the goal is<br>
> > eventually to make this more C++ like code, then new code coming in<br>
> > should aim to move the needle in that direction rather than continue<br>
> > to propagate the old ways of doing."<br>
> ><br>
> Thanks for taking this on.<br></blockquote></div></div><div dir="auto"><br></div><div dir="auto">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.</div><div dir="auto"><br></div><div dir="auto">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.</div><div dir="auto"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
> <br>
> > I personally do NOT want to see changes to C++ in one leaf class and<br>
> > the other architectures not get the same changes. I would prefer to see<br>
> > all these corrections and base changes in the same style with limited<br>
> > changes to C-isms. I'm not opposed to the changes but let's take the<br>
> > Target class one. There are multiple target classes. Changing one<br>
> > independent of the others isn't a good idea.<br>
> ><br>
> This is reasonable to me.<br>
> <br>
> > I'd like to see us get a working baseline in and then do something like<br>
> > sweep std::string through Target* as a single patch. This is easier to<br>
> > test and review since it would only be C string to std::string. Perhaps<br>
> > switch to C++ output a file at a time. Redo the report output. Etc.<br>
> > Discrete chunks instead of piecemeal.<br>
> ><br>
> > Covoar has spent years broken and some is from changing working<br>
> > things to do things "a better way" with no baseline to check against.<br>
> > We need to get a baseline.<br>
> ><br>
> > Please. Let's get a working baseline and then approach this more<br>
> > methodically. No one is going to suffer from seeing a C string a little<br>
> > while longer. :)<br>
> ><br>
> I'm fine, as long as there is a plan in place and some clear<br>
> directions. It would help to have tickets to organize the path<br>
> forward.<br></blockquote></div></div><div dir="auto"><br></div><div dir="auto">+1</div><div dir="auto"><br></div><div dir="auto">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.</div><div dir="auto"><br></div><div dir="auto">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.</div><div dir="auto"><br></div><div dir="auto">Plus Alex and I talked earlier this evening and think we have a reasonable path to greatly speed it up without massive overhauls.</div><div dir="auto"><br></div><div dir="auto"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
> <br>
> I'm willing to oblige continued use of C'ism for now, but I want to<br>
> know the plan and maybe a deadline of sorts by which I can start to be<br>
> picky again :) I don't like to be in limbo.<br>
> <br>
> <br>
> I'd love to have a deadline but I can't guarantee how long Alex can<br>
> work on it. But unless he gets pulled, he can pick on this for a while.<br>
<br>
Joel, I pushed a number of changes to move covoar towards C++ back when I pushed<br>
in the ELF and DWARF support. The C++ nature of the interfaces I brought in from<br>
the tool kit required some refactoring. I have not seen much action since then<br>
so Gedare's question seems reasonable.<br></blockquote></div></div><div dir="auto"><br></div><div dir="auto">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.</div><div dir="auto"><br></div><div dir="auto"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
The need for this code to be C++ and not a mix comes down to what you want to<br>
see happen. There is no real need to move the code closer to C++ other than<br>
improving the technical quality and that is about the life of the code in the<br>
project. If you are willing to look after the code as is then I am fine with<br>
that. If you would like to see the code move to C++ and away from C then can<br>
also happen. It would be an interesting way to learn more about C++.<br>
<br>
I find the code hard to work on because I do not know if I am looking at C and<br>
needing to use C solutions or C++ and I should be pulling on C++ threads and<br>
where they go. I also see this is an issue for those working on the code. In<br>
some recent patches I pointed out new code being call from C++ code with C++<br>
objects that was written in C. I pointed this out and the next patch was fine.<br></blockquote></div></div><div dir="auto"><br></div><div dir="auto">There really isn't any straight C code in here. Just C-strings and C style IO. </div><div dir="auto"><br></div><div dir="auto"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
> My guess is that C string to std::string is probably a good pass by<br>
> itself since method signatures may change. <br>
<br>
I think anything you feel can be changed and is in reach is welcome.<br>
<br>
> There isn't much file input but that could be a pass by itself<br>
> along the way,<br>
> <br>
> Then sweep output one file at a time leaving reporting for last as<br>
> a batch.<br>
> <br>
> Do you see an order?<br>
<br>
Maybe we organise a group session online where we all look over the code<br>
together and discuss various aspects of the architecture and what it means from<br>
a strictly C vs C++ point of view. Any change like this cannot happen in a<br>
vacuum and I believe C++ is more of a taught language than C. C styles are<br>
easier to pick up by reading code. My introduction to C++ was in the mid-90's<br>
with a Rational training course and documentation. It takes time to relearn C<br>
solutions you know and have at hand in C++.<br></blockquote></div></div><div dir="auto"><br></div><div dir="auto">This sounds good. </div><div dir="auto"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
I have no idea what we would need to hold such a session and who would be<br>
interested but I am happy to be there and be part of it.<br></blockquote></div></div><div dir="auto"><br></div><div dir="auto">Probably just a Google meeting is fine</div><div dir="auto"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
Chris<br>
</blockquote></div></div></div>