[PATCH] D69764: [clang-format] Add Left/Right Const (East/West , Before/After) fixer capability

Sam McCall via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Nov 11 05:11:10 PST 2019


sammccall added a comment.

(Sorry for arriving at this late)

At a strategic level, I have some concerns here: the fact that clang-format generally doesn't touch the token sequence isn't an accident.
e.g. formatting `int x;;` will insert a newline rather than deleting the redundant semicolon. Like the one in this patch, that would be a useful feature, but it's a path the clang-format authors deliberately decided to close off.

The conservative approach means that pseudo-parser gets things wrong, we get broken formatting, and ~never broken code.
This means:

- a simpler mental model. People are happier to blindly apply it, not to require review of formatting changes, etc
- the ability to require clang-formatting as a pre-commit hook, knowing that at worst people will have to badly format their code
- a lesser risk when rolling out new versions of clang-format from (our org does this from head weekly)

(There are exceptions, e.g. comment splitting, but they're pretty rare)

@klimek in case he has thoughts.



================
Comment at: clang/lib/Format/Format.cpp:768
   LLVMStyle.CommentPragmas = "^ IWYU pragma:";
+  LLVMStyle.ConstStyle = FormatStyle::CS_Leave;
   LLVMStyle.CompactNamespaces = false;
----------------
MyDeveloperDay wrote:
> lebedev.ri wrote:
> > MyDeveloperDay wrote:
> > > lebedev.ri wrote:
> > > > Based on code reviews, this should be `CS_West`.
> > > I would tend to agree for the LLVM project itself, but I really don't feel I can change this due to the huge number of projects which have BasedOnStyle: LLVM  (not to mention most other style inherit from LLVM like Google, Chromium etc...)
> > > 
> > > I think this needs to be an opt-in change in the .clang-format (at least for now)
> > There is already a number of stylistic LLVM-specific decisions in LLVM style
> > * 80-col wrap
> > * pointer alignment to the right
> > * spaces instead of tabs
> > * ???
> > I think it first and foremost is an LLVM style, and i don't
> > really recall any guarantees that it must not/will not change.
> Again I do agree, but I think those rules were established before clang-format really gained such popular usage, those coming to clang-format changed those probably straight away if they didn't agree with their established their style.
> 
> Changing something under everyone now feels like I'd have a different agenda (for west const to win), and that that is not my goal. I'm simply trying to facilitate compliance.
> 
> I think we can agree the east/west const rebellion is a contentious issue, (as where the previous formatting wars!), my aim here is to help try and remove the discussion from the debate and let people move on! but I think what it doesn't need is someone like clang-format being considered the bad guy by dictating a default style globally.
> 
> I did a quick search on github, the hits to "const int" and "int const" alone are too eye-watering. I really don't want to break the internet with this.
> 
For non-style reasons, I do think this should be CS_Leave in every built-in style, so that adjusting const would have to at least be explicitly in .clang-format.


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

https://reviews.llvm.org/D69764





More information about the cfe-commits mailing list