aaron.ballman added inline comments.
================
Comment at: clang/lib/Format/Format.cpp:1699
ChromiumStyle.DerivePointerAlignment = false;
+ ChromiumStyle.InsertBraces = true;
if (Language == FormatStyle::LK_ObjC)
----------------
MyDeveloperDay wrote:
> owenpan wrote:
> > aaron.ballman wrote:
> > > HazardyKnusperkeks wrote:
> > > > MyDeveloperDay wrote:
> > > > > This is an code modifying feature, we agreed that all code modifying
> > > > > features would be off by default, opt in only
> > > > Now the question arises if //default// simply only applies to
> > > > `LLVMStyle`, since that's the //default// when nothing is stated, or if
> > > > other styles are free to enable such features in their style //by
> > > > default//.
> > > >
> > > > I'd say if chromium wants to do that, they should be allowed to.
> > > The community reacted pretty strongly to clang-format mutating code in
> > > ways that may change the meaning of code unless there is an explicit
> > > opt-in. The reason for that is because the opt-in + documentation is what
> > > informs the user that the feature may break their code, so removing that
> > > opt-in for the Chromium style means those users have no idea about the
> > > dangers. (In general, users take a dim view of a formatting tool that
> > > breaks code.)
> > >
> > > Personally, I think if the Chromium *project* wants that to be the
> > > default, they can use .clang-format files in their version control to
> > > make it so, but I don't think the Chromium *style* built into
> > > clang-format should allow it by default because that may be used by a
> > > wider audience than just Chromium developers. Basically, I think we want
> > > to be conservative with formatting features that can potentially break
> > > code (once we start breaking user code with a formatting tool, that tool
> > > gets pulled out of affected people's CI pipelines pretty quickly, which I
> > > think we generally want to avoid).
> > I'm with @MyDeveloperDay and @aaron.ballman on this.
> I personally would feel quite uncomfortable about going against what we
> agreed in the RFC.
That sounds like we've got consensus amongst code owners not to proceed with
this patch. However, we still appreciate the patch and the discussion!
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D147969/new/
https://reviews.llvm.org/D147969
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
