I had a quick look at clang-format again and came across the following: 1. Case labels can’t be half-indented We can probably live with this one, but it means that case labels use the same indentation as the switch statement or they get indented 4 spaces (together with their code block).
2. It breaks BitUnion macros We could probably rewrite the macros to look like something clang recognises. This’d probably help other tools as well. 3. It’s very aggressive at changing alignment There are cases where we (well, at least I) break initialisation lists and parameter lists to make logical groups of parameters. For example, initialisation of the various sync parameters in the HDLCD controller looks like this: : ... v_sync(0), v_back_porch(0), v_data(0), v_front_porch(0), h_sync(0), h_back_porch(0), h_data(0), h_front_porch(0), The code formatting above wouldn’t be preserved. 4. It insists on putting short initialisation lists on the same line as the constructor’s parameter lists For example: FrameBuffer::FrameBuffer() : _width(0), _height(0) We never define this in the style guide, so I guess it’s correct behaviour... 5. It can’t check function/variable naming conventions I think I could live with most of the issues above (assuming BitUnions can be fixed), but it wouldn’t completely remove the need for custom Python scripts to check for things like header ordering. //Andreas On 04/02/2016, 22:06, "gem5-dev on behalf of nathan binkert" <[email protected] on behalf of [email protected]> wrote: >Things may have advanced in the last year, but my recommendation is to >just >change the style (or submit a change to llvm) so that clang-format can be >used. Its awesome. > > Nate > >On Sun, Jan 31, 2016 at 5:23 AM, Andreas Sandberg ><[email protected]> >wrote: > >> Nilay posted a configuration file [1] about a year ago. I independently >> had a look around that time as well. It seemed like a really good tool, >> but it wasn’t possible to configure it to adhere to the gem5 style guide >> at the time. >> >> IIRC, one of the problems at the time was that there were a few things >>in >> the gem5 style guide that clang-format couldn’t handle. Judging by my >> comment on RB, the issue was to do with indentation in switch statements >> and I think there may have been issues with class declarations as well. >> >> //Andreas >> >> [1] http://reviews.gem5.org/r/2372/ >> >> >> On 29/01/2016, 18:47, "gem5-dev on behalf of nathan binkert" >> <[email protected] on behalf of [email protected]> wrote: >> >> >Have you guys played with clang-format? It's awesome and very >> >configurable. You could consider at least running that on the whole >> >codebase and then adding a prechangegroup hook on the server to reject >> >pushes once it's done. >> > >> > Nate >> > >> >On Thu, Jan 28, 2016 at 10:14 AM, Steve Reinhardt <[email protected]> >> >wrote: >> > >> >> On Thu, Jan 28, 2016 at 9:35 AM Andreas Sandberg >> >><[email protected] >> >> > >> >> wrote: >> >> >> >> > >> >> > You¹re probably right, rewrite is probably a better description. I >> >> suspect >> >> > we¹ll have to create some abstraction layer that allows us to >>extract >> >> > diffs from both Mercurial and git if we want to keep the ability to >> >>apply >> >> > the style checker to a subset of a file. However, I¹m not sure if >> >>that¹s >> >> > desirable. I have had issues with the style checker (false >>positives >> >>and >> >> > negatives) due to the partial view in the past, so it might make >> >>sense to >> >> > apply it to the entire files instead of individual changes. >> >> > >> >> >> >> Yes, doing whole files makes sense to me. We need to make a pass and >> >>clean >> >> up all the existing issues before we do that though. I'm planning >>to do >> >> that anyway. >> >> >> >> Steve >> >> _______________________________________________ >> >> gem5-dev mailing list >> >> [email protected] >> >> http://m5sim.org/mailman/listinfo/gem5-dev >> >> >> >_______________________________________________ >> >gem5-dev mailing list >> >[email protected] >> >http://m5sim.org/mailman/listinfo/gem5-dev >> >> IMPORTANT NOTICE: The contents of this email and any attachments are >> confidential and may also be privileged. If you are not the intended >> recipient, please notify the sender immediately and do not disclose the >> contents to any other person, use it for any purpose, or store or copy >>the >> information in any medium. Thank you. >> _______________________________________________ >> gem5-dev mailing list >> [email protected] >> http://m5sim.org/mailman/listinfo/gem5-dev >> >_______________________________________________ >gem5-dev mailing list >[email protected] >http://m5sim.org/mailman/listinfo/gem5-dev IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you. _______________________________________________ gem5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/gem5-dev
