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.
> --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
