[PATCH] D113898: [NFC][clangd] cleaning up llvm-qualified-auto

This revision was not accepted when it landed; it landed in state "Needs Review". This revision was automatically updated to reflect the committed changes. Closed by commit rGc0e3c893aa09: [NFC][clangd] cleaning up llvm-qualified-auto (authored by kuhnel). Changed prior to commit:

[PATCH] D113898: [NFC][clangd] cleaning up llvm-qualified-auto

kuhnel added a comment. TODO: check for remaining `const` then submit. Comment at: clang-tools-extra/clangd/unittests/FileIndexTests.cpp:252 TEST(FileIndexTest, TemplateParamsInLabel) { - auto Source = R"cpp( + const auto *Source = R"cpp( template remove

[PATCH] D113898: [NFC][clangd] cleaning up llvm-qualified-auto

kuhnel marked 3 inline comments as done. kuhnel added inline comments. Comment at: clang-tools-extra/clangd/unittests/HoverTests.cpp:2550 auto AST = TU.build(); - for (auto Comment : {"doc1", "doc2", "doc3"}) { + for (const auto *Comment : {"doc1", "doc2", "doc3"}) {

[PATCH] D113898: [NFC][clangd] cleaning up llvm-qualified-auto

kuhnel updated this revision to Diff 393894. kuhnel added a comment. more reverts of consts Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D113898/new/ https://reviews.llvm.org/D113898 Files: clang-tools-extra/clangd/AST.cpp

[PATCH] D113898: [NFC][clangd] cleaning up llvm-qualified-auto

kuhnel updated this revision to Diff 393892. kuhnel added a comment. fixed more const ClangdTests.cpp Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D113898/new/ https://reviews.llvm.org/D113898 Files: clang-tools-extra/clangd/AST.cpp

[PATCH] D113898: [NFC][clangd] cleaning up llvm-qualified-auto

kuhnel updated this revision to Diff 393890. kuhnel added a comment. reverted the `const` as advised in Sam's comment Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D113898/new/ https://reviews.llvm.org/D113898 Files:

[PATCH] D113898: [NFC][clangd] cleaning up llvm-qualified-auto

kuhnel added a comment. In D113898#3167215 , @kbobyrev wrote: > In D113898#3167050 , @kuhnel wrote: > >> Looking at the documentation, this looks like a bug in the clang-tidy check: >>

[PATCH] D113898: [NFC][clangd] cleaning up llvm-qualified-auto

kbobyrev added a comment. In D113898#3167050 , @kuhnel wrote: > Looking at the documentation, this looks like a bug in the clang-tidy check: > https://releases.llvm.org/13.0.0/tools/clang/tools/extra/docs/clang-tidy/checks/readability-qualified-auto.html

[PATCH] D113898: [NFC][clangd] cleaning up llvm-qualified-auto

kuhnel added a comment. Looking at the documentation, this looks like a bug in the clang-tidy check: https://releases.llvm.org/13.0.0/tools/clang/tools/extra/docs/clang-tidy/checks/readability-qualified-auto.html The rule `llvm-qualified-auto` should not add a `const` qualifier. Repository:

[PATCH] D113898: [NFC][clangd] cleaning up llvm-qualified-auto

Quuxplusone added a comment. In 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

[PATCH] D113898: [NFC][clangd] cleaning up llvm-qualified-auto

sammccall added a comment. In 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

[PATCH] D113898: [NFC][clangd] cleaning up llvm-qualified-auto

kuhnel added a comment. 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

[PATCH] D113898: [NFC][clangd] cleaning up llvm-qualified-auto

sammccall added a comment. This check was written specifically for the LLVM monorepo, I doubt there'll be consensus to disable it. https://reviews.llvm.org/D72217 has some previous discussion - some people want the consts spelled out, but we don't do this in clangd. And I don't particularly

[PATCH] D113898: [NFC][clangd] cleaning up llvm-qualified-auto

kuhnel added a comment. Based on Sam's comments and our offline discussion, I would propose: 1. abandon this patch 2. start a discussion on the mailing list to disable this check for all of the llvm monorepo. Other thoughts? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D113898: [NFC][clangd] cleaning up llvm-qualified-auto

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

[PATCH] D113898: [NFC][clangd] cleaning up llvm-qualified-auto

kuhnel created this revision. Herald added subscribers: carlosgalvezp, usaxena95, kadircet, arphaman, javed.absar. kuhnel added subscribers: sammccall, adamcz, kbobyrev. kuhnel published this revision for review. Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov. Herald added a