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

Reply via email to