sammccall added a comment. In D113898#3140320 <https://reviews.llvm.org/D113898#3140320>, @kuhnel wrote:
> When trying to revert some of the changes, I just noticed there are a couple > of `if (const auto* ...` in `CodeComplete.cpp` and `AST.cpp` (and maybe other > files as well). So some folks seem to be using this right now. If we want to > be consistent, we would have to remove these `const` as well. > > I'm not sure what the right way forward would be. > > @sammccall WDYT? Yeah, we don't really have a consistent pattern for this after all. In current code, where `Foo` is local and can be const: - we favor `Foo` over `const Foo` - we favor `const Foo*` over `Foo*` (the language often forces this) - `auto *Foo` vs `const auto *Foo` is a mix Most of the time, I think the const is noise in contexts where `auto` is appropriate. But not all the time! "This variable is intended for mutation" would be a useful signal, but marking every *other* variable as const is an ineffective and expensive way to do it. And we have no good way to keep that consistent. So I'd suggest: - if applying bulk fixes like this and you don't want to look through case-by-case, prefer `auto*` - if a human has written `const auto*`, then there may or may not be a good reason. To "clean these up" would require two people to think about whether const communicates something important in each location, and I don't personally think that's a great use of time. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D113898/new/ https://reviews.llvm.org/D113898 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
