Okay. I will do that. On Fri, 4 Jun 2021, 10:48 pm Gedare Bloom, <ged...@rtems.org> wrote:
> On Fri, Jun 4, 2021 at 2:57 PM Ida Delphine <idad...@gmail.com> wrote: > > > > Okay, I will take a look. > > > > Regarding me asking a question in the appropriate clang-format mailing > list should it be just regarding the parentheses and braces being aligned? > > > That would be the right question to ask, if you can't find a way to > align the closing parenthesis. > > You might also follow-up that old thread related to alignment of the > pointers. > > > On Fri, Jun 4, 2021 at 8:41 PM Joel Sherrill <j...@rtems.org> wrote: > >> > >> > >> > >> On Fri, Jun 4, 2021 at 12:39 PM Gedare Bloom <ged...@rtems.org> wrote: > >>> > >>> On Fri, Jun 4, 2021 at 8:47 AM Joel Sherrill <j...@rtems.org> wrote: > >>> > > >>> > > >>> > > >>> > On Fri, Jun 4, 2021 at 12:24 AM Ida Delphine <idad...@gmail.com> > wrote: > >>> >> > >>> >> Hello everyone, > >>> >> > >>> >> I applied the configuration Sebastian used and ran clang-format on > cpukit/score/src/threadqenque.c and so far these are the differences I > could notice... > >>> >> Below are some example areas in the code you can spot the > differences: > >>> >> > >>> >> In line 68, the ")" at the end of the parameter list needs to be in > a new row, but this doesn't seem to be supported in clang-format. > >>> > > >>> > If I understand correctly, clang-format does not like: > >>> > > >>> > https://git.rtems.org/rtems/tree/cpukit/score/src/threadqenqueue.c > >>> > > >>> > which has the first parameter on its one line but wants the first > parameter > >>> > after the open parenthesis? > >>> > > >>> > The RTEMS style would seem to correspond to AlignAfterOpenBracket > being > >>> > set to AlwaysBreak > >>> > > >>> >> > >>> >> In line 142, if the function call is split into multiple rows, the > ");" should always be in a new row. > >>> > > >>> > Having the closing parenthesis on its own line may end up being > something > >>> > we have to change the RTEMS style on. I do not see an option in their > >>> > documentation to do this. Unfortunate, since I like the symmetry > between > >>> > braces and parentheses. > >>> > > >>> > Could you file an issue with them and/or ask a question the > appropriate > >>> > mailing list? Please cc Gedara and me. Give them an example. Maybe > >>> > we are missing something. > >>> >> > >>> >> In line 201-202, we can see that the "*" of the pointers are not > aligned to the right. > >>> > > >>> > > >>> > This seems to be the issue Gedare mentioned which might have a patch. > >>> > Follow up on that. > >>> > > >>> > But I think we had previously discussed this as a point we may have > to > >>> > concede and change RTEMS style on. > >>> >> > >>> >> You can check out the formatted file here - > https://pastebin.com/nDBrSSCP > >>> > > >>> > > >>> > Is it just the website or are blank line differences? It may just be > an > >>> > illusion. I think the spacing between the numbered lines is greater > >>> > than in the git view. Just odd. > >>> > > >>> That's just the pastebin having more vertical padding between > consecutive lines. > >> > >> > >> That's what I thought but it did make the code look funny. > >> > >> Ida/Gedare.. does this mean there are only 3 style mismatch issues? Or > only > >> three in this file? > >> > >> Probably should try a few more files and see if there are other > discrepancies. > >> > >> And obviously work on the integration/automation of using the tools. :) > >> > >> --joel > >> > >>> > >>> > >>> > --joel > >>> >> > >>> >> > >>> >> > >>> >> On Tue, Jun 1, 2021 at 5:36 PM Gedare Bloom <ged...@rtems.org> > wrote: > >>> >>> > >>> >>> On Mon, May 31, 2021 at 2:59 PM Ida Delphine <idad...@gmail.com> > wrote: > >>> >>> > > >>> >>> > Hi Gedare, > >>> >>> > > >>> >>> > With regards to your comment on discord on me looking for a tool > that works on both patches and source files, it turns out clang-format has > that functionality already. Here's what I found - > https://clang.llvm.org/docs/ClangFormat.html#script-for-patch-reformatting > >>> >>> > > >>> >>> > Does it match what you have in mind? > >>> >>> > > >>> >>> Yes. I think we would want to not use the `-i` option but instead > pass > >>> >>> through and check the changes. I don't think we should rewrite the > >>> >>> patches themselves, but instead we want to use a tool that can be > used > >>> >>> to check and approve the style of submitted patches. You might > need to > >>> >>> write a modified version of the clang-format-diff.py to use as a > >>> >>> "checker" with ability to provide exceptions to the rules. > >>> >>> > >>> >>> Gedare > >>> >>> > >>> >>> > On Thu, May 13, 2021 at 3:49 PM Gedare Bloom <ged...@rtems.org> > wrote: > >>> >>> >> > >>> >>> >> On Wed, May 12, 2021 at 2:18 PM Ida Delphine <idad...@gmail.com> > wrote: > >>> >>> >> > > >>> >>> >> > Hello everyone, > >>> >>> >> > Still waiting for some feedback :) > >>> >>> >> > > >>> >>> >> > Cheers, > >>> >>> >> > Ida. > >>> >>> >> > > >>> >>> >> > On Mon, 10 May 2021, 5:59 am Ida Delphine, <idad...@gmail.com> > wrote: > >>> >>> >> >> > >>> >>> >> >> Hello everyone, > >>> >>> >> >> Went through some previous emails and it turns out Sebastian > already came up with a configuration for clang format which works well for > RTEMS except for the fact that some configurations haven't been implemented > into clang-format yet. Using > >>> >>> >> >> > >>> >>> >> >> AlignConsecutiveDeclarations: false > >>> >>> >> >> PointerAlignment: Right > >>> >>> >> >> > >>> >>> >> >> Doesn't seem to work. > >>> >>> >> >> For example in the cpukit/score/src/threadq.c file, > something like > >>> >>> >> >> > >>> >>> >> >> RTEMS_STATIC_ASSERT( > >>> >>> >> >> offsetof( Thread_queue_Syslock_queue, Queue.name ) > >>> >>> >> >> == offsetof( struct _Thread_queue_Queue, _name ), > >>> >>> >> >> THREAD_QUEUE_SYSLOCK_QUEUE_NAME > >>> >>> >> >> ); > >>> >>> >> >> > >>> >>> >> >> RTEMS_STATIC_ASSERT( > >>> >>> >> >> sizeof( Thread_queue_Syslock_queue ) > >>> >>> >> >> == sizeof( struct _Thread_queue_Queue ), > >>> >>> >> >> THREAD_QUEUE_SYSLOCK_QUEUE_SIZE > >>> >>> >> >> ); > >>> >>> >> >> > >>> >>> >> >> #if defined(RTEMS_SMP) > >>> >>> >> >> void _Thread_queue_Do_acquire_critical( > >>> >>> >> >> Thread_queue_Control *the_thread_queue, > >>> >>> >> >> ISR_lock_Context *lock_context > >>> >>> >> >> ) > >>> >>> >> >> { > >>> >>> >> >> _Thread_queue_Queue_acquire_critical( > >>> >>> >> >> &the_thread_queue->Queue, > >>> >>> >> >> &the_thread_queue->Lock_stats, > >>> >>> >> >> lock_context > >>> >>> >> >> ); > >>> >>> >> >> > >>> >>> >> >> becomes this after using the given configuration > >>> >>> >> >> > >>> >>> >> >> RTEMS_STATIC_ASSERT(sizeof(Thread_queue_Syslock_queue) == > >>> >>> >> >> sizeof(struct _Thread_queue_Queue), > >>> >>> >> >> THREAD_QUEUE_SYSLOCK_QUEUE_SIZE); > >>> >>> >> >> > >>> >>> >> >> #if defined(RTEMS_SMP) > >>> >>> >> >> void _Thread_queue_Do_acquire_critical(Thread_queue_Control > *the_thread_queue, > >>> >>> >> >> ISR_lock_Context *lock_context) { > >>> >>> >> >> _Thread_queue_Queue_acquire_critical( > >>> >>> >> >> &the_thread_queue->Queue, &the_thread_queue->Lock_stats, > lock_context); > >>> >>> >> >> > >>> >>> >> >> Everything seems manageable except for this alignment > issue... > >>> >>> >> >> This also throws more light on the changes using > clang-format ( > https://lists.rtems.org/pipermail/devel/2018-December/024145.html) > >>> >>> >> >> > >>> >>> >> I think we're willing to concede the pointer alignment. > However, it > >>> >>> >> would be worth spending some time to see if > >>> >>> >> https://reviews.llvm.org/D27651 can be made to work. The > current state > >>> >>> >> of the code would need to be compared to the patch on that > review > >>> >>> >> board. > >>> >>> >> > >>> >>> >> Beyond that, documenting the clang-format options to use is > next, and > >>> >>> >> then identifying a plan how to invoke clang-format during a git > >>> >>> >> workflow is needed. > >>> >>> >> > >>> >>> >> >> On Thu, May 6, 2021 at 8:05 PM Joel Sherrill <j...@rtems.org> > wrote: > >>> >>> >> >>> > >>> >>> >> >>> > >>> >>> >> >>> > >>> >>> >> >>> On Thu, May 6, 2021 at 12:47 PM Christian Mauderer < > o...@c-mauderer.de> wrote: > >>> >>> >> >>>> > >>> >>> >> >>>> Hello Ida and Gedare, > >>> >>> >> >>>> > >>> >>> >> >>>> On 06/05/2021 06:26, Gedare Bloom wrote: > >>> >>> >> >>>> > hi Ida, > >>> >>> >> >>>> > > >>> >>> >> >>>> > On Wed, May 5, 2021 at 3:21 PM Ida Delphine < > idad...@gmail.com> wrote: > >>> >>> >> >>>> >> > >>> >>> >> >>>> >> Hello everyone, > >>> >>> >> >>>> >> > >>> >>> >> >>>> >> Regarding this project ( > https://devel.rtems.org/ticket/3860) I went with clang-format as we all > agreed. I have tested it on some "score" files and it made some changes > which I don't think are very much in line with the RTEMS coding style. > However, it wasn't really clear if we will chage the RTEMS coding style or > try to make changes to clang-format to fit the style. > >>> >>> >> >>>> >> Please will love to know the best option. > >>> >>> >> >>>> >> > >>> >>> >> >>>> > We will likely need to consider our choices carefully. > If we can find > >>> >>> >> >>>> > a suitably close style that is already well-supported by > clang, and > >>> >>> >> >>>> > get consensus from the maintainers on a change, then > that might be the > >>> >>> >> >>>> > best route forward. > >>> >>> >> >>>> > >>> >>> >> >>>> +1 > >>> >>> >> >>>> > >>> >>> >> >>>> > I think the first thing to do is take the examples > >>> >>> >> >>>> > that have been shown by Sebastian that are "close" but > not quite > >>> >>> >> >>>> > perfect, and identify the cases where they differ with > RTEMS style in > >>> >>> >> >>>> > order to present for discussion here. If consensus can't > be reached to > >>> >>> >> >>>> > change the style, then we would need to have a plan for > how to improve > >>> >>> >> >>>> > the existing tools for what we have. > >>> >>> >> >>>> > >>> >>> >> >>>> I also found the following tool quite useful to play with > the clang > >>> >>> >> >>>> style config: > >>> >>> >> >>>> > >>> >>> >> >>>> https://zed0.co.uk/clang-format-configurator/ > >>> >>> >> >>>> > >>> >>> >> >>>> Maybe it can help a bit to find out what certain options > mean. > >>> >>> >> >>>> > >>> >>> >> >>>> > > >>> >>> >> >>>> > However, I think there is interest in doing less work on > the tool > >>> >>> >> >>>> > side, and more work on how to integrate it into our > workflows better. > >>> >>> >> >>>> > >>> >>> >> >>>> +1 > >>> >>> >> >>> > >>> >>> >> >>> > >>> >>> >> >>> I agree with all of this from the student perspective. But > we will have > >>> >>> >> >>> to come to some agreement on a machine producible format to > >>> >>> >> >>> be able to use the integration. A report on what doesn't > match would > >>> >>> >> >>> give us something to chew on while Ida works the > integration. > >>> >>> >> >>> > >>> >>> >> >>> --joel > >>> >>> >> >>>> > >>> >>> >> >>>> > >>> >>> >> >>>> > > >>> >>> >> >>>> >> Cheers, > >>> >>> >> >>>> >> Ida. > >>> >>> >> >>>> >> _______________________________________________ > >>> >>> >> >>>> >> devel mailing list > >>> >>> >> >>>> >> devel@rtems.org > >>> >>> >> >>>> >> http://lists.rtems.org/mailman/listinfo/devel > >>> >>> >> >>>> > _______________________________________________ > >>> >>> >> >>>> > devel mailing list > >>> >>> >> >>>> > devel@rtems.org > >>> >>> >> >>>> > http://lists.rtems.org/mailman/listinfo/devel > >>> >>> >> >>>> > > >>> >>> >> >>>> _______________________________________________ > >>> >>> >> >>>> devel mailing list > >>> >>> >> >>>> devel@rtems.org > >>> >>> >> >>>> http://lists.rtems.org/mailman/listinfo/devel >
_______________________________________________ devel mailing list devel@rtems.org http://lists.rtems.org/mailman/listinfo/devel