[llvm-dev] [cfe-dev] [RFC] Adding support for clang-format making further code modifying changes (original) (raw)
David Blaikie via llvm-dev llvm-dev at lists.llvm.org
Mon Aug 9 15:34:13 PDT 2021
- Previous message: [llvm-dev] [cfe-dev] [RFC] Adding support for clang-format making further code modifying changes
- Next message: [llvm-dev] [cfe-dev] [RFC] Adding support for clang-format making further code modifying changes
- Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]
On Mon, Aug 9, 2021 at 3:23 PM Sam McCall via cfe-dev < cfe-dev at lists.llvm.org> wrote:
I'm cautiously +1 on const reordering, having previously opposed it and been convinced. I think anyone who's worked on a large shared codebase both before and after clang-format can understand the value here, so I'll focus mostly on the risks and why I think they're acceptable.
*Risk: *clang-format will become a grab-bag of features with no clear line - just anything implemented on top of its pseudo-AST. Clang-format's brand is low-level formatting details and I think it's important to preserve this. Const order fits here in users' minds. (So does brace addition/removal). Risk: The feature will break code and clang-format will no longer be (seen as) reliable. This can make it harder socially or technically to deploy, and cause real damage. I think we need to work hard on mitigating this: - the feature needs careful design and extra scrutiny, like security-critical code - it should be clearly and temporarily marked as experimental, with opt-in required - it should be clearly and permanently marked as "makes assumptions about coding style", with opt-in required. - bugs need to be thoughtfully addressed From what I can see MyDeveloperDay is serious about doing all of this. Risk: clang-format will be overtaken by the complexity of such features, which will outweigh the benefits (if few use them), hurting maintenance, causing bugs etc. However this isn't different from other optional features. Editing tokens tends to be done as a separate pass which is relatively easy to isolate (compared to something like supporting a new language). With complexity isolated, this is mostly just about how maintainers prioritize their time/attention, which must be left up to them. Regarding include-ordering: I think this is a valuable feature if you follow a coding style that allows it to be correct, and it fits well in clang-format's brand. However it wasn't clearly labeled to emphasize its caveats, and in hindsight it shouldn't have been made part of the Google style without further opt-in required.
Thanks for the write up! & for the record I'm pretty comfortable with what Sam's said here. (not that I've got strong weight in clang-format development, but to make sure my other comments on the thread aren't treated as standing criticisms/concerns)
- Dave -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20210809/5a3a80d0/attachment.html>
- Previous message: [llvm-dev] [cfe-dev] [RFC] Adding support for clang-format making further code modifying changes
- Next message: [llvm-dev] [cfe-dev] [RFC] Adding support for clang-format making further code modifying changes
- Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]