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

2022-01-25 Thread Christian Kühnel via Phabricator via cfe-commits 
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:
  https://reviews.llvm.org/D113898?vs=393894&id=402869#toc

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
  clang-tools-extra/clangd/CodeComplete.cpp
  clang-tools-extra/clangd/ExpectedTypes.cpp
  clang-tools-extra/clangd/FindSymbols.cpp
  clang-tools-extra/clangd/HeaderSourceSwitch.cpp
  clang-tools-extra/clangd/Hover.cpp
  clang-tools-extra/clangd/TUScheduler.cpp
  clang-tools-extra/clangd/index/IndexAction.cpp
  clang-tools-extra/clangd/index/SymbolCollector.cpp
  clang-tools-extra/clangd/index/dex/Iterator.cpp
  clang-tools-extra/clangd/refactor/tweaks/AnnotateHighlightings.cpp
  clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp
  clang-tools-extra/clangd/refactor/tweaks/DumpAST.cpp
  clang-tools-extra/clangd/refactor/tweaks/ExpandMacro.cpp
  clang-tools-extra/clangd/unittests/ClangdTests.cpp
  clang-tools-extra/clangd/unittests/FileIndexTests.cpp
  clang-tools-extra/clangd/unittests/HoverTests.cpp
  clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
  clang-tools-extra/clangd/unittests/tweaks/DefineInlineTests.cpp

Index: clang-tools-extra/clangd/unittests/tweaks/DefineInlineTests.cpp
===
--- clang-tools-extra/clangd/unittests/tweaks/DefineInlineTests.cpp
+++ clang-tools-extra/clangd/unittests/tweaks/DefineInlineTests.cpp
@@ -192,7 +192,7 @@
 }
 
 TEST_F(DefineInlineTest, TransformNestedNamespaces) {
-  auto Test = R"cpp(
+  auto *Test = R"cpp(
 namespace a {
   void bar();
   namespace b {
@@ -220,7 +220,7 @@
   b::c::aux();
   a::b::c::aux();
 })cpp";
-  auto Expected = R"cpp(
+  auto *Expected = R"cpp(
 namespace a {
   void bar();
   namespace b {
@@ -252,7 +252,7 @@
 }
 
 TEST_F(DefineInlineTest, TransformUsings) {
-  auto Test = R"cpp(
+  auto *Test = R"cpp(
 namespace a { namespace b { namespace c { void aux(); } } }
 
 void foo();
@@ -263,7 +263,7 @@
   using c::aux;
   namespace d = c;
 })cpp";
-  auto Expected = R"cpp(
+  auto *Expected = R"cpp(
 namespace a { namespace b { namespace c { void aux(); } } }
 
 void foo(){
@@ -278,7 +278,7 @@
 }
 
 TEST_F(DefineInlineTest, TransformDecls) {
-  auto Test = R"cpp(
+  auto *Test = R"cpp(
 void foo();
 void f^oo() {
   class Foo {
@@ -293,7 +293,7 @@
   enum class EnClass { Zero, One };
   EnClass y = EnClass::Zero;
 })cpp";
-  auto Expected = R"cpp(
+  auto *Expected = R"cpp(
 void foo(){
   class Foo {
   public:
@@ -312,7 +312,7 @@
 }
 
 TEST_F(DefineInlineTest, TransformTemplDecls) {
-  auto Test = R"cpp(
+  auto *Test = R"cpp(
 namespace a {
   template  class Bar {
   public:
@@ -329,7 +329,7 @@
   bar>.bar();
   aux>();
 })cpp";
-  auto Expected = R"cpp(
+  auto *Expected = R"cpp(
 namespace a {
   template  class Bar {
   public:
@@ -350,7 +350,7 @@
 }
 
 TEST_F(DefineInlineTest, TransformMembers) {
-  auto Test = R"cpp(
+  auto *Test = R"cpp(
 class Foo {
   void foo();
 };
@@ -358,7 +358,7 @@
 void Foo::f^oo() {
   return;
 })cpp";
-  auto Expected = R"cpp(
+  auto *Expected = R"cpp(
 class Foo {
   void foo(){
   return;
@@ -395,7 +395,7 @@
 }
 
 TEST_F(DefineInlineTest, TransformDependentTypes) {
-  auto Test = R"cpp(
+  auto *Test = R"cpp(
 namespace a {
   template  class Bar {};
 }
@@ -409,7 +409,7 @@
   Bar B;
   Bar> q;
 })cpp";
-  auto Expected = R"cpp(
+  auto *Expected = R"cpp(
 namespace a {
   template  class Bar {};
 }
@@ -511,7 +511,7 @@
 }
 
 TEST_F(DefineInlineTest, TransformTypeLocs) {
-  auto Test = R"cpp(
+  auto *Test = R"cpp(
 namespace a {
   template  class Bar {
   public:
@@ -528,7 +528,7 @@
   Foo foo;
   a::Bar>::Baz> q;
 })cpp";
-  auto Expected = R"cpp(
+  auto *Expected = R"cpp(
 namespace a {
   template  class Bar {
   public:
@@ -549,7 +549,7 @@
 }
 
 TEST_F(DefineInlineTest, TransformDeclRefs) {
-  auto Test = R"cpp(
+  auto *Test = R"cpp(
 namespace a {
   template  class Bar {
   public:
@@ -575,7 +575,7 @@
   bar();
   a::test();
 })cpp";
-  auto Expected = R"cpp(
+  auto *Expected = R"cpp(
 namespace a {
   template  class Bar {
   public:
@@ -605,12 +605,12 @@
 }
 
 TEST_F(DefineInlineTest, StaticMembers) {
-  auto Test = R"cpp(
+  auto *Test = R"cpp(
 namespace ns { class X { static void foo(); void bar(); }; }
 void ns::X::b^ar() {
   foo();
 })cpp";
-  auto Expected = R"cpp(
+  auto *Expected = R"cpp(
 namespace ns

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

2022-01-18 Thread Christian Kühnel via Phabricator via cfe-commits 
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 const here


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

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

2021-12-13 Thread Christian Kühnel via Phabricator via cfe-commits 
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"}) {
 for (const auto &P : T.points(Comment)) {

while clang-tidy added a const here, I would keep it for consistency with the 
next line...


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

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

2021-12-13 Thread Christian Kühnel via Phabricator via cfe-commits 
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
  clang-tools-extra/clangd/CodeComplete.cpp
  clang-tools-extra/clangd/ExpectedTypes.cpp
  clang-tools-extra/clangd/FindSymbols.cpp
  clang-tools-extra/clangd/HeaderSourceSwitch.cpp
  clang-tools-extra/clangd/Hover.cpp
  clang-tools-extra/clangd/TUScheduler.cpp
  clang-tools-extra/clangd/index/IndexAction.cpp
  clang-tools-extra/clangd/index/SymbolCollector.cpp
  clang-tools-extra/clangd/index/dex/Iterator.cpp
  clang-tools-extra/clangd/refactor/tweaks/AnnotateHighlightings.cpp
  clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp
  clang-tools-extra/clangd/refactor/tweaks/DumpAST.cpp
  clang-tools-extra/clangd/refactor/tweaks/ExpandMacro.cpp
  clang-tools-extra/clangd/unittests/ClangdTests.cpp
  clang-tools-extra/clangd/unittests/FileIndexTests.cpp
  clang-tools-extra/clangd/unittests/HoverTests.cpp
  clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
  clang-tools-extra/clangd/unittests/tweaks/DefineInlineTests.cpp

Index: clang-tools-extra/clangd/unittests/tweaks/DefineInlineTests.cpp
===
--- clang-tools-extra/clangd/unittests/tweaks/DefineInlineTests.cpp
+++ clang-tools-extra/clangd/unittests/tweaks/DefineInlineTests.cpp
@@ -192,7 +192,7 @@
 }
 
 TEST_F(DefineInlineTest, TransformNestedNamespaces) {
-  auto Test = R"cpp(
+  auto *Test = R"cpp(
 namespace a {
   void bar();
   namespace b {
@@ -220,7 +220,7 @@
   b::c::aux();
   a::b::c::aux();
 })cpp";
-  auto Expected = R"cpp(
+  auto *Expected = R"cpp(
 namespace a {
   void bar();
   namespace b {
@@ -252,7 +252,7 @@
 }
 
 TEST_F(DefineInlineTest, TransformUsings) {
-  auto Test = R"cpp(
+  auto *Test = R"cpp(
 namespace a { namespace b { namespace c { void aux(); } } }
 
 void foo();
@@ -263,7 +263,7 @@
   using c::aux;
   namespace d = c;
 })cpp";
-  auto Expected = R"cpp(
+  auto *Expected = R"cpp(
 namespace a { namespace b { namespace c { void aux(); } } }
 
 void foo(){
@@ -278,7 +278,7 @@
 }
 
 TEST_F(DefineInlineTest, TransformDecls) {
-  auto Test = R"cpp(
+  auto *Test = R"cpp(
 void foo();
 void f^oo() {
   class Foo {
@@ -293,7 +293,7 @@
   enum class EnClass { Zero, One };
   EnClass y = EnClass::Zero;
 })cpp";
-  auto Expected = R"cpp(
+  auto *Expected = R"cpp(
 void foo(){
   class Foo {
   public:
@@ -312,7 +312,7 @@
 }
 
 TEST_F(DefineInlineTest, TransformTemplDecls) {
-  auto Test = R"cpp(
+  auto *Test = R"cpp(
 namespace a {
   template  class Bar {
   public:
@@ -329,7 +329,7 @@
   bar>.bar();
   aux>();
 })cpp";
-  auto Expected = R"cpp(
+  auto *Expected = R"cpp(
 namespace a {
   template  class Bar {
   public:
@@ -350,7 +350,7 @@
 }
 
 TEST_F(DefineInlineTest, TransformMembers) {
-  auto Test = R"cpp(
+  auto *Test = R"cpp(
 class Foo {
   void foo();
 };
@@ -358,7 +358,7 @@
 void Foo::f^oo() {
   return;
 })cpp";
-  auto Expected = R"cpp(
+  auto *Expected = R"cpp(
 class Foo {
   void foo(){
   return;
@@ -395,7 +395,7 @@
 }
 
 TEST_F(DefineInlineTest, TransformDependentTypes) {
-  auto Test = R"cpp(
+  auto *Test = R"cpp(
 namespace a {
   template  class Bar {};
 }
@@ -409,7 +409,7 @@
   Bar B;
   Bar> q;
 })cpp";
-  auto Expected = R"cpp(
+  auto *Expected = R"cpp(
 namespace a {
   template  class Bar {};
 }
@@ -511,7 +511,7 @@
 }
 
 TEST_F(DefineInlineTest, TransformTypeLocs) {
-  auto Test = R"cpp(
+  auto *Test = R"cpp(
 namespace a {
   template  class Bar {
   public:
@@ -528,7 +528,7 @@
   Foo foo;
   a::Bar>::Baz> q;
 })cpp";
-  auto Expected = R"cpp(
+  auto *Expected = R"cpp(
 namespace a {
   template  class Bar {
   public:
@@ -549,7 +549,7 @@
 }
 
 TEST_F(DefineInlineTest, TransformDeclRefs) {
-  auto Test = R"cpp(
+  auto *Test = R"cpp(
 namespace a {
   template  class Bar {
   public:
@@ -575,7 +575,7 @@
   bar();
   a::test();
 })cpp";
-  auto Expected = R"cpp(
+  auto *Expected = R"cpp(
 namespace a {
   template  class Bar {
   public:
@@ -605,12 +605,12 @@
 }
 
 TEST_F(DefineInlineTest, StaticMembers) {
-  auto Test = R"cpp(
+  auto *Test = R"cpp(
 namespace ns { class X { static void foo(); void bar(); }; }
 void ns::X::b^ar() {
   foo();
 })cpp";
-  auto Expected = R"cpp(
+  auto *Expected = R"cpp(
 namespace ns { class X { static void foo(); void bar(){
   foo();
 } }; }
@@ -654,7 +654,7 @@
 }
 
 TEST_F(DefineInlineTest, TransformTemplParamNames) {
-  auto Test = R"cpp(
+  auto *Test = R"cpp(
 struct Foo {
   struct Bar {
 template

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

2021-12-13 Thread Christian Kühnel via Phabricator via cfe-commits 
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
  clang-tools-extra/clangd/CodeComplete.cpp
  clang-tools-extra/clangd/ExpectedTypes.cpp
  clang-tools-extra/clangd/FindSymbols.cpp
  clang-tools-extra/clangd/HeaderSourceSwitch.cpp
  clang-tools-extra/clangd/Hover.cpp
  clang-tools-extra/clangd/TUScheduler.cpp
  clang-tools-extra/clangd/index/IndexAction.cpp
  clang-tools-extra/clangd/index/SymbolCollector.cpp
  clang-tools-extra/clangd/index/dex/Iterator.cpp
  clang-tools-extra/clangd/refactor/tweaks/AnnotateHighlightings.cpp
  clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp
  clang-tools-extra/clangd/refactor/tweaks/DumpAST.cpp
  clang-tools-extra/clangd/refactor/tweaks/ExpandMacro.cpp
  clang-tools-extra/clangd/unittests/ClangdTests.cpp
  clang-tools-extra/clangd/unittests/FileIndexTests.cpp
  clang-tools-extra/clangd/unittests/HoverTests.cpp
  clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
  clang-tools-extra/clangd/unittests/tweaks/DefineInlineTests.cpp

Index: clang-tools-extra/clangd/unittests/tweaks/DefineInlineTests.cpp
===
--- clang-tools-extra/clangd/unittests/tweaks/DefineInlineTests.cpp
+++ clang-tools-extra/clangd/unittests/tweaks/DefineInlineTests.cpp
@@ -192,7 +192,7 @@
 }
 
 TEST_F(DefineInlineTest, TransformNestedNamespaces) {
-  auto Test = R"cpp(
+  const auto *Test = R"cpp(
 namespace a {
   void bar();
   namespace b {
@@ -220,7 +220,7 @@
   b::c::aux();
   a::b::c::aux();
 })cpp";
-  auto Expected = R"cpp(
+  const auto *Expected = R"cpp(
 namespace a {
   void bar();
   namespace b {
@@ -252,7 +252,7 @@
 }
 
 TEST_F(DefineInlineTest, TransformUsings) {
-  auto Test = R"cpp(
+  const auto *Test = R"cpp(
 namespace a { namespace b { namespace c { void aux(); } } }
 
 void foo();
@@ -263,7 +263,7 @@
   using c::aux;
   namespace d = c;
 })cpp";
-  auto Expected = R"cpp(
+  const auto *Expected = R"cpp(
 namespace a { namespace b { namespace c { void aux(); } } }
 
 void foo(){
@@ -278,7 +278,7 @@
 }
 
 TEST_F(DefineInlineTest, TransformDecls) {
-  auto Test = R"cpp(
+  const auto *Test = R"cpp(
 void foo();
 void f^oo() {
   class Foo {
@@ -293,7 +293,7 @@
   enum class EnClass { Zero, One };
   EnClass y = EnClass::Zero;
 })cpp";
-  auto Expected = R"cpp(
+  const auto *Expected = R"cpp(
 void foo(){
   class Foo {
   public:
@@ -312,7 +312,7 @@
 }
 
 TEST_F(DefineInlineTest, TransformTemplDecls) {
-  auto Test = R"cpp(
+  const auto *Test = R"cpp(
 namespace a {
   template  class Bar {
   public:
@@ -329,7 +329,7 @@
   bar>.bar();
   aux>();
 })cpp";
-  auto Expected = R"cpp(
+  const auto *Expected = R"cpp(
 namespace a {
   template  class Bar {
   public:
@@ -350,7 +350,7 @@
 }
 
 TEST_F(DefineInlineTest, TransformMembers) {
-  auto Test = R"cpp(
+  const auto *Test = R"cpp(
 class Foo {
   void foo();
 };
@@ -358,7 +358,7 @@
 void Foo::f^oo() {
   return;
 })cpp";
-  auto Expected = R"cpp(
+  const auto *Expected = R"cpp(
 class Foo {
   void foo(){
   return;
@@ -395,7 +395,7 @@
 }
 
 TEST_F(DefineInlineTest, TransformDependentTypes) {
-  auto Test = R"cpp(
+  const auto *Test = R"cpp(
 namespace a {
   template  class Bar {};
 }
@@ -409,7 +409,7 @@
   Bar B;
   Bar> q;
 })cpp";
-  auto Expected = R"cpp(
+  const auto *Expected = R"cpp(
 namespace a {
   template  class Bar {};
 }
@@ -511,7 +511,7 @@
 }
 
 TEST_F(DefineInlineTest, TransformTypeLocs) {
-  auto Test = R"cpp(
+  const auto *Test = R"cpp(
 namespace a {
   template  class Bar {
   public:
@@ -528,7 +528,7 @@
   Foo foo;
   a::Bar>::Baz> q;
 })cpp";
-  auto Expected = R"cpp(
+  const auto *Expected = R"cpp(
 namespace a {
   template  class Bar {
   public:
@@ -549,7 +549,7 @@
 }
 
 TEST_F(DefineInlineTest, TransformDeclRefs) {
-  auto Test = R"cpp(
+  const auto *Test = R"cpp(
 namespace a {
   template  class Bar {
   public:
@@ -575,7 +575,7 @@
   bar();
   a::test();
 })cpp";
-  auto Expected = R"cpp(
+  const auto *Expected = R"cpp(
 namespace a {
   template  class Bar {
   public:
@@ -605,12 +605,12 @@
 }
 
 TEST_F(DefineInlineTest, StaticMembers) {
-  auto Test = R"cpp(
+  const auto *Test = R"cpp(
 namespace ns { class X { static void foo(); void bar(); }; }
 void ns::X::b^ar() {
   foo();
 })cpp";
-  auto Expected = R"cpp(
+  const auto *Expected = R"cpp(
 namespace ns { class X { static void foo(); void bar(){
   foo();
 } }; }
@@ -654,7 +654,7 @@
 }
 
 TEST_F(DefineInlineTest, TransformTe

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

2021-12-13 Thread Christian Kühnel via Phabricator via cfe-commits 
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:
  clang-tools-extra/clangd/AST.cpp
  clang-tools-extra/clangd/CodeComplete.cpp
  clang-tools-extra/clangd/ExpectedTypes.cpp
  clang-tools-extra/clangd/FindSymbols.cpp
  clang-tools-extra/clangd/HeaderSourceSwitch.cpp
  clang-tools-extra/clangd/Hover.cpp
  clang-tools-extra/clangd/TUScheduler.cpp
  clang-tools-extra/clangd/index/IndexAction.cpp
  clang-tools-extra/clangd/index/SymbolCollector.cpp
  clang-tools-extra/clangd/index/dex/Iterator.cpp
  clang-tools-extra/clangd/refactor/tweaks/AnnotateHighlightings.cpp
  clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp
  clang-tools-extra/clangd/refactor/tweaks/DumpAST.cpp
  clang-tools-extra/clangd/refactor/tweaks/ExpandMacro.cpp
  clang-tools-extra/clangd/unittests/ClangdTests.cpp
  clang-tools-extra/clangd/unittests/FileIndexTests.cpp
  clang-tools-extra/clangd/unittests/HoverTests.cpp
  clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
  clang-tools-extra/clangd/unittests/tweaks/DefineInlineTests.cpp

Index: clang-tools-extra/clangd/unittests/tweaks/DefineInlineTests.cpp
===
--- clang-tools-extra/clangd/unittests/tweaks/DefineInlineTests.cpp
+++ clang-tools-extra/clangd/unittests/tweaks/DefineInlineTests.cpp
@@ -192,7 +192,7 @@
 }
 
 TEST_F(DefineInlineTest, TransformNestedNamespaces) {
-  auto Test = R"cpp(
+  const auto *Test = R"cpp(
 namespace a {
   void bar();
   namespace b {
@@ -220,7 +220,7 @@
   b::c::aux();
   a::b::c::aux();
 })cpp";
-  auto Expected = R"cpp(
+  const auto *Expected = R"cpp(
 namespace a {
   void bar();
   namespace b {
@@ -252,7 +252,7 @@
 }
 
 TEST_F(DefineInlineTest, TransformUsings) {
-  auto Test = R"cpp(
+  const auto *Test = R"cpp(
 namespace a { namespace b { namespace c { void aux(); } } }
 
 void foo();
@@ -263,7 +263,7 @@
   using c::aux;
   namespace d = c;
 })cpp";
-  auto Expected = R"cpp(
+  const auto *Expected = R"cpp(
 namespace a { namespace b { namespace c { void aux(); } } }
 
 void foo(){
@@ -278,7 +278,7 @@
 }
 
 TEST_F(DefineInlineTest, TransformDecls) {
-  auto Test = R"cpp(
+  const auto *Test = R"cpp(
 void foo();
 void f^oo() {
   class Foo {
@@ -293,7 +293,7 @@
   enum class EnClass { Zero, One };
   EnClass y = EnClass::Zero;
 })cpp";
-  auto Expected = R"cpp(
+  const auto *Expected = R"cpp(
 void foo(){
   class Foo {
   public:
@@ -312,7 +312,7 @@
 }
 
 TEST_F(DefineInlineTest, TransformTemplDecls) {
-  auto Test = R"cpp(
+  const auto *Test = R"cpp(
 namespace a {
   template  class Bar {
   public:
@@ -329,7 +329,7 @@
   bar>.bar();
   aux>();
 })cpp";
-  auto Expected = R"cpp(
+  const auto *Expected = R"cpp(
 namespace a {
   template  class Bar {
   public:
@@ -350,7 +350,7 @@
 }
 
 TEST_F(DefineInlineTest, TransformMembers) {
-  auto Test = R"cpp(
+  const auto *Test = R"cpp(
 class Foo {
   void foo();
 };
@@ -358,7 +358,7 @@
 void Foo::f^oo() {
   return;
 })cpp";
-  auto Expected = R"cpp(
+  const auto *Expected = R"cpp(
 class Foo {
   void foo(){
   return;
@@ -395,7 +395,7 @@
 }
 
 TEST_F(DefineInlineTest, TransformDependentTypes) {
-  auto Test = R"cpp(
+  const auto *Test = R"cpp(
 namespace a {
   template  class Bar {};
 }
@@ -409,7 +409,7 @@
   Bar B;
   Bar> q;
 })cpp";
-  auto Expected = R"cpp(
+  const auto *Expected = R"cpp(
 namespace a {
   template  class Bar {};
 }
@@ -511,7 +511,7 @@
 }
 
 TEST_F(DefineInlineTest, TransformTypeLocs) {
-  auto Test = R"cpp(
+  const auto *Test = R"cpp(
 namespace a {
   template  class Bar {
   public:
@@ -528,7 +528,7 @@
   Foo foo;
   a::Bar>::Baz> q;
 })cpp";
-  auto Expected = R"cpp(
+  const auto *Expected = R"cpp(
 namespace a {
   template  class Bar {
   public:
@@ -549,7 +549,7 @@
 }
 
 TEST_F(DefineInlineTest, TransformDeclRefs) {
-  auto Test = R"cpp(
+  const auto *Test = R"cpp(
 namespace a {
   template  class Bar {
   public:
@@ -575,7 +575,7 @@
   bar();
   a::test();
 })cpp";
-  auto Expected = R"cpp(
+  const auto *Expected = R"cpp(
 namespace a {
   template  class Bar {
   public:
@@ -605,12 +605,12 @@
 }
 
 TEST_F(DefineInlineTest, StaticMembers) {
-  auto Test = R"cpp(
+  const auto *Test = R"cpp(
 namespace ns { class X { static void foo(); void bar(); }; }
 void ns::X::b^ar() {
   foo();
 })cpp";
-  auto Expected = R"cpp(
+  const auto *Expected = R"cpp(
 namespace ns { class X { static void foo(); void bar(){
   foo();
 } }; }
@@ -654,7 +654,7 @@
 }
 
 TEST_F(DefineInlineT

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

2021-12-13 Thread Christian Kühnel via Phabricator via cfe-commits 
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:
>> 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.
>
> I don't see how that is a bug: the docs specially cover the `const auto *` 
> conversion in the first example by drawing the line between `observe()` 
> (non-mutating function) and `change()` (possibly mutating function).

Sorry I should have been more precise here: `AddConstToQualified` allows to 
configure if we want to have `const` added or not. The default for that rule is 
`true`. However the last sentence on that page says:

> Note in the LLVM alias, the default value is false.

So my understanding of this is: With the LLVM-rules we're using, `const` should 
not be added.


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

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

2021-12-02 Thread Kirill Bobyrev via Phabricator via cfe-commits 
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
>
> The rule `llvm-qualified-auto` should not add a `const` qualifier.

I don't see how that is a bug: the docs specially cover the `const auto *` 
conversion in the first example by drawing the line between `observe()` 
(non-mutating function) and `change()` (possibly mutating function).


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

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

2021-12-02 Thread Christian Kühnel via Phabricator via cfe-commits 
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:
  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

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

2021-11-18 Thread Arthur O'Dwyer via Phabricator via cfe-commits 
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 folks seem to be using this right now. If we want to 
> be consistent, we would have to remove these `const` as well.

Just in case it's news to anyone: (1) Theoretically, the compiler has enough 
information to tell you when it's deducing a cv-qualified type for a 
non-forwarding-reference `auto`; and personally I'd like such a warning and 
would go fix my code every time it fired. I like that `const auto *p` has the 
same "shape" as `const Widget *p`. That sometimes a non-const `auto` can 
secretly mean //`const`//`Widget` is //weird// and I don't like it.
(2) In template contexts, `const auto *` can be significant, because maybe in 
some instantiations the `const` doesn't matter and in others it does. 
https://quuxplusone.github.io/blog/2020/03/04/field-report-on-lifetime-extension/#true-positive-3-conditionally-redundant-lifetime-extension-in-template-code
 is related.


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

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

2021-11-18 Thread Sam McCall via Phabricator via cfe-commits 
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 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

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

2021-11-18 Thread Christian Kühnel via Phabricator via cfe-commits 
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 these `const` as well.

I'm not sure what the right way forward would be.

@sammccall  WDYT?


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

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

2021-11-17 Thread Sam McCall via Phabricator via cfe-commits 
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 think that would be the right thing to do: it's 
throwing out the baby with the bathwater.

I think we should rather:

- fix the check to exempt strings (I don't think this would be terribly 
controversial, but I might be wrong)
- land a version of this patch that doesn't add `const` in the places we don't 
normally use it


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

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

2021-11-17 Thread Christian Kühnel via Phabricator via cfe-commits 
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
  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

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

2021-11-15 Thread Sam McCall via Phabricator via cfe-commits 
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(D->getReturnType())) {
+} else if (const auto *DT = dyn_cast(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

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

2021-11-15 Thread Christian Kühnel via Phabricator via cfe-commits 
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 project: clang-tools-extra.

This is a cleanup of all llvm-qualified-auto findings.
This patch was created by automatically applying the fixes from
clang-tidy.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D113898

Files:
  clang-tools-extra/clangd/AST.cpp
  clang-tools-extra/clangd/CodeComplete.cpp
  clang-tools-extra/clangd/ExpectedTypes.cpp
  clang-tools-extra/clangd/FindSymbols.cpp
  clang-tools-extra/clangd/HeaderSourceSwitch.cpp
  clang-tools-extra/clangd/Hover.cpp
  clang-tools-extra/clangd/TUScheduler.cpp
  clang-tools-extra/clangd/index/IndexAction.cpp
  clang-tools-extra/clangd/index/SymbolCollector.cpp
  clang-tools-extra/clangd/index/dex/Iterator.cpp
  clang-tools-extra/clangd/refactor/tweaks/AnnotateHighlightings.cpp
  clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp
  clang-tools-extra/clangd/refactor/tweaks/DumpAST.cpp
  clang-tools-extra/clangd/refactor/tweaks/ExpandMacro.cpp
  clang-tools-extra/clangd/unittests/ClangdTests.cpp
  clang-tools-extra/clangd/unittests/FileIndexTests.cpp
  clang-tools-extra/clangd/unittests/HoverTests.cpp
  clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
  clang-tools-extra/clangd/unittests/tweaks/DefineInlineTests.cpp

Index: clang-tools-extra/clangd/unittests/tweaks/DefineInlineTests.cpp
===
--- clang-tools-extra/clangd/unittests/tweaks/DefineInlineTests.cpp
+++ clang-tools-extra/clangd/unittests/tweaks/DefineInlineTests.cpp
@@ -192,7 +192,7 @@
 }
 
 TEST_F(DefineInlineTest, TransformNestedNamespaces) {
-  auto Test = R"cpp(
+  const auto *Test = R"cpp(
 namespace a {
   void bar();
   namespace b {
@@ -220,7 +220,7 @@
   b::c::aux();
   a::b::c::aux();
 })cpp";
-  auto Expected = R"cpp(
+  const auto *Expected = R"cpp(
 namespace a {
   void bar();
   namespace b {
@@ -252,7 +252,7 @@
 }
 
 TEST_F(DefineInlineTest, TransformUsings) {
-  auto Test = R"cpp(
+  const auto *Test = R"cpp(
 namespace a { namespace b { namespace c { void aux(); } } }
 
 void foo();
@@ -263,7 +263,7 @@
   using c::aux;
   namespace d = c;
 })cpp";
-  auto Expected = R"cpp(
+  const auto *Expected = R"cpp(
 namespace a { namespace b { namespace c { void aux(); } } }
 
 void foo(){
@@ -278,7 +278,7 @@
 }
 
 TEST_F(DefineInlineTest, TransformDecls) {
-  auto Test = R"cpp(
+  const auto *Test = R"cpp(
 void foo();
 void f^oo() {
   class Foo {
@@ -293,7 +293,7 @@
   enum class EnClass { Zero, One };
   EnClass y = EnClass::Zero;
 })cpp";
-  auto Expected = R"cpp(
+  const auto *Expected = R"cpp(
 void foo(){
   class Foo {
   public:
@@ -312,7 +312,7 @@
 }
 
 TEST_F(DefineInlineTest, TransformTemplDecls) {
-  auto Test = R"cpp(
+  const auto *Test = R"cpp(
 namespace a {
   template  class Bar {
   public:
@@ -329,7 +329,7 @@
   bar>.bar();
   aux>();
 })cpp";
-  auto Expected = R"cpp(
+  const auto *Expected = R"cpp(
 namespace a {
   template  class Bar {
   public:
@@ -350,7 +350,7 @@
 }
 
 TEST_F(DefineInlineTest, TransformMembers) {
-  auto Test = R"cpp(
+  const auto *Test = R"cpp(
 class Foo {
   void foo();
 };
@@ -358,7 +358,7 @@
 void Foo::f^oo() {
   return;
 })cpp";
-  auto Expected = R"cpp(
+  const auto *Expected = R"cpp(
 class Foo {
   void foo(){
   return;
@@ -395,7 +395,7 @@
 }
 
 TEST_F(DefineInlineTest, TransformDependentTypes) {
-  auto Test = R"cpp(
+  const auto *Test = R"cpp(
 namespace a {
   template  class Bar {};
 }
@@ -409,7 +409,7 @@
   Bar B;
   Bar> q;
 })cpp";
-  auto Expected = R"cpp(
+  const auto *Expected = R"cpp(
 namespace a {
   template  class Bar {};
 }
@@ -511,7 +511,7 @@
 }
 
 TEST_F(DefineInlineTest, TransformTypeLocs) {
-  auto Test = R"cpp(
+  const auto *Test = R"cpp(
 namespace a {
   template  class Bar {
   public:
@@ -528,7 +528,7 @@
   Foo foo;
   a::Bar>::Baz> q;
 })cpp";
-  auto Expected = R"cpp(
+  const auto *Expected = R"cpp(
 namespace a {
   template  class Bar {
   public:
@@ -549,7 +549,7 @@
 }
 
 TEST_F(DefineInlineTest, TransformDeclRefs) {
-  auto Test = R"cpp(
+  const auto *Test = R"cpp(
 namespace a {
   template  class Bar {
   public:
@@ -575,7 +575,7 @@
   bar();
   a::test();
 })cpp";
-  auto Expected = R"cpp(
+  const auto *Expected = R"cpp(
 namespace a {
   template  class Bar {
   public:
@@ -605,12 +605,12 @@
 }
 
 TEST_F(DefineInlineTest, StaticMembers) {
-  auto Test = R"cpp(
+  const auto *Test = R"cpp(
 namespace ns { class X { static void foo