[PATCH] D113898: [NFC][clangd] cleaning up llvm-qualified-auto
Sam McCall via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Mon Nov 15 07:59:05 PST 2021
sammccall added a comment.
There is a reasonable style rule here, but:
- the suggested fixes spell out `const` in ways we generally prefer not to. (And the rule doesn't require).
- Strings are a special case. `auto*` for string literals makes no sense to me, the check should be modified to either accept `auto` or replace with `const char*`
================
Comment at: clang-tools-extra/clangd/AST.cpp:441
DeducedType = AT->getDeducedType();
- } else if (auto DT = dyn_cast<DecltypeType>(D->getReturnType())) {
+ } else if (const auto *DT = dyn_cast<DecltypeType>(D->getReturnType())) {
// auto in a trailing return type just points to a DecltypeType and
----------------
We usually use `auto*` rather than `const auto*` for locals like this.
For the same reason we don't `const` locals: it adds more noise than it helps.
(And if it's critical we spell out these things, we probably shouldn't be using auto at all)
When this check was introduced, there was a discussion about this and they agreed to have the check stop flagging `auto*`, but if it sees `auto` it will still suggest the const. So the automatic fixes generally aren't what we want, we need to drop const.
================
Comment at: clang-tools-extra/clangd/index/IndexAction.cpp:64
const auto FileID = SM.getFileID(Loc);
- const auto File = SM.getFileEntryForID(FileID);
+ const auto *const File = SM.getFileEntryForID(FileID);
auto URI = toURI(File);
----------------
This is a particularly confusing type.
The original code is atypical for clangd, I'd suggest changing to `auto* File`
================
Comment at: clang-tools-extra/clangd/index/dex/Iterator.cpp:209
OS << "(| ";
- auto Separator = "";
+ const auto *Separator = "";
for (const auto &Child : Children) {
----------------
`const char*` or `auto` or `StringRef` for strings. "Some kind of pointer" is technically correct but unhelpful.
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