MyDeveloperDay added inline comments.
================ Comment at: clang/lib/Format/Format.cpp:1699 ChromiumStyle.DerivePointerAlignment = false; + ChromiumStyle.InsertBraces = true; if (Language == FormatStyle::LK_ObjC) ---------------- 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. 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