[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