[PATCH] D113898: [NFC][clangd] cleaning up llvm-qualified-auto
Sam McCall via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Thu Nov 18 09:13:49 PST 2021
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
More information about the cfe-commits
mailing list