[PATCH] D69764: [clang-format] Add East/West Const fixer capability

Tue May 26 13:07:05 PDT 2020

MyDeveloperDay added a comment.

I really do appreciate the reviews and the comments especially regarding east/west/left/right/wrong ;-), I know there won't be 100% agreement, but I think most people who even consider clone the LLVM repo know what we mean by East/West. as such I'm going to leave the internal code that way for now. (and the name of the TokenAnalyzer Files)

However, I have added support for Left/Right and East/West in the config, but I'm going to refrain from adding Before/After otherwise it just gets silly having too many options.

Whilst I've been developing this I've tried both ways, to be honest I instinctively know what is meant by East Const because its the style I don't use personally (don't shoot me for that comment). But when taking in terms of Left/Right I feel I have to think about what it means quite hard. Especially as Right feels Wrong to me!

Let me reiterate my goal. For me the goal was to make east/west const conversations disappear in the same way that tab and whitespace conversations have disappeared (mostly) because I think those conversations are a waste of good conference time.

The answer should just be "use clang-format", and for me, this is part of my own feeling that we should "clang-format all the things". I feel the best compromise is to offer both, (I'll likely update the documentation to not have so much of a bias)

If we can agree to that, then I'll be happy in a year or so to do a poll of the .clang-format files in github.com and then change the internal code to match the majority, to me that would be the greatest measure of which should be the primary option.

Apart from that, I've still some work to do here, this case from @steveire is really stumping me. Every preprocessor branch causes a different iteration over the same original code and as such this is causing multiple replacements to be added as each pass reanalyses the original code and doesn't regenerate the code in between (super odd)

  verifyFormat("Foo<const Foo<int>> P;\n#if 0\n#else\n#endif", "Foo<Foo<int> const> P;\n#if 0\n#else\n#endif", Style);

I tried to only perform the replacement on the first pass, but alas that means if I put any declarations inside the preprocess clauses they actually don't get converted. (I'm not sure if anyone has seen this before), but its likely the fact that I'm using clang-format to create replacements.

In the background I've been testing this on LLVM itself (which isn't easy because the code isn't all clang-formatted itself, another pet peeve of mine), apart from the #if issue (which hits lib/Analyzer/CFG.cpp and a few other files) it seems to work pretty well (although I've not done a detailed walkthrough to see if it's missing much)

I would like to land this at some point, but cannot whilst I still have the #if/#else issue

Thanks for the help and the feedback, just wanted to give everyone a progress report.

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69764/new/

https://reviews.llvm.org/D69764