[PATCH] D147395: [Clangd] Make the type hint length limit configurable

[Yi Zhang via Phabricator via cfe-commits]

zhangyi1357 added a comment.

I dont have commit access. Could you help committing the change for me? @hokein




Comment at: clang-tools-extra/clangd/Config.h:151
+// Limit the length of type names in inlay hints.
+size_t TypeNameLimit = 32;
   } InlayHints;

hokein wrote:
> zhangyi1357 wrote:
> > hokein wrote:
> > > I would extend it a bit more -- 0 means no limit. 
> > > 
> > > Can you also add a unittest in `TEST(TypeHints, LongTypeName)` in 
> > > `InlayHintTests.cpp`? 
> > > 0 means no limit.
> > This is quite a good idea. I've done it.
> > 
> > For unittest, there is already `TEST(TypeHints, LongTypeName)` in 
> > `InlayHintTests.cpp`. Do you mean add more tests in the same `TEST` or 
> > another `TEST` with TypeNameLimit configured?
> > 
> I mean adding one more test in the same `TEST(TypeHints, LongTypeName)`.
> 
> This test verifies the the long type name is shown when the limit is set to 
> 0, something like
> 
> ```
> TEST(TypeHints, LongTypeName) {
>   assertTypeHints(R"cpp(
> template 
> struct A {};
> struct MultipleWords {};
> A foo();
> // Omit type hint past a certain length (currently 32)
> auto var = foo();
>   )cpp");
> 
> Config cfg; 
>  ... // set the limit to 0
> 
>assertTypeHints(R"cpp(
> template 
> struct A {};
> struct MultipleWords {};
> A foo();
> // Omit type hint past a certain length (currently 32)
> auto var = foo();
>   )cpp", ExpectedHint...);
> }
> ```
Thanks for your example! I've added that.



Comment at: clang-tools-extra/clangd/unittests/InlayHintTests.cpp:80
   C.InlayHints.Designators = false;
+  C.InlayHints.TypeNameLimit = 1;
   return C;

hokein wrote:
> why do we need this change? I think it should be fine without it.
Yes, without this line the tests will not fail. But I am a little confused 
about the code below without 'C.InlayHints.TypeNameLimit = 1;'.
```
Config noHintsConfig() {
  Config C;
  C.InlayHints.Parameters = false;
  C.InlayHints.DeducedTypes = false;
  C.InlayHints.Designators = false;
  // C.InlayHints.TypeNameLimit = 1;
  return C;
}

template 
void assertHints(InlayHintKind Kind, llvm::StringRef AnnotatedSource,
 ExpectedHints... Expected) {
  Annotations Source(AnnotatedSource);
  TestTU TU = TestTU::withCode(Source.code());
  TU.ExtraArgs.push_back("-std=c++20");
  auto AST = TU.build();

  EXPECT_THAT(hintsOfKind(AST, Kind),
  ElementsAre(HintMatcher(Expected, Source)...));
  // Sneak in a cross-cutting check that hints are disabled by config.
  // We'll hit an assertion failure if addInlayHint still gets called.
  WithContextValue WithCfg(Config::Key, noHintsConfig());
  EXPECT_THAT(inlayHints(AST, std::nullopt), IsEmpty());  // Why does this 
succeed with TypeNameLimit = 32 ?
}

TEST(TypeHints, Smoke) {
  assertTypeHints(R"cpp(
auto $waldo[[waldo]] = 42;
  )cpp",
  ExpectedHint{": int", "waldo"});
}
```
The dault TypeNameLimit is 32, why does the second `EXPECT_THAT` succeed with 
TypeNameLimit = 32.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D147395/new/

https://reviews.llvm.org/D147395

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

[PATCH] D147395: [Clangd] Make the type hint length limit configurable

[Yi Zhang via Phabricator via cfe-commits]

zhangyi1357 updated this revision to Diff 511955.
zhangyi1357 marked an inline comment as done.
zhangyi1357 added a comment.

Combine mutiple commits into single one


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D147395/new/

https://reviews.llvm.org/D147395

Files:
  clang-tools-extra/clangd/Config.h
  clang-tools-extra/clangd/ConfigCompile.cpp
  clang-tools-extra/clangd/ConfigFragment.h
  clang-tools-extra/clangd/ConfigYAML.cpp
  clang-tools-extra/clangd/InlayHints.cpp
  clang-tools-extra/clangd/unittests/InlayHintTests.cpp

Index: clang-tools-extra/clangd/unittests/InlayHintTests.cpp
===
--- clang-tools-extra/clangd/unittests/InlayHintTests.cpp
+++ clang-tools-extra/clangd/unittests/InlayHintTests.cpp
@@ -1324,6 +1324,21 @@
 // Omit type hint past a certain length (currently 32)
 auto var = foo();
   )cpp");
+
+  Config Cfg;
+  Cfg.InlayHints.TypeNameLimit = 0;
+  WithContextValue WithCfg(Config::Key, std::move(Cfg));
+
+  assertTypeHints(
+  R"cpp(
+template 
+struct A {};
+struct MultipleWords {};
+A foo();
+// Should have type hint with TypeNameLimit = 0
+auto $var[[var]] = foo();
+  )cpp",
+  ExpectedHint{": A", "var"});
 }
 
 TEST(TypeHints, DefaultTemplateArgs) {
Index: clang-tools-extra/clangd/InlayHints.cpp
===
--- clang-tools-extra/clangd/InlayHints.cpp
+++ clang-tools-extra/clangd/InlayHints.cpp
@@ -688,7 +688,8 @@
   return;
 
 std::string TypeName = T.getAsString(Policy);
-if (TypeName.length() < TypeNameLimit)
+if (Cfg.InlayHints.TypeNameLimit == 0 ||
+TypeName.length() < Cfg.InlayHints.TypeNameLimit)
   addInlayHint(R, HintSide::Right, InlayHintKind::Type, Prefix, TypeName,
/*Suffix=*/"");
   }
@@ -714,8 +715,6 @@
   // the policies are initialized for more details.)
   PrintingPolicy TypeHintPolicy;
   PrintingPolicy StructuredBindingPolicy;
-
-  static const size_t TypeNameLimit = 32;
 };
 
 } // namespace
Index: clang-tools-extra/clangd/ConfigYAML.cpp
===
--- clang-tools-extra/clangd/ConfigYAML.cpp
+++ clang-tools-extra/clangd/ConfigYAML.cpp
@@ -254,6 +254,10 @@
   if (auto Value = boolValue(N, "Designators"))
 F.Designators = *Value;
 });
+Dict.handle("TypeNameLimit", [&](Node ) {
+  if (auto Value = uint32Value(N, "TypeNameLimit"))
+F.TypeNameLimit = *Value;
+});
 Dict.parse(N);
   }
 
@@ -375,6 +379,17 @@
 return std::nullopt;
   }
 
+  std::optional> uint32Value(Node , llvm::StringRef Desc) {
+if (auto Scalar = scalarValue(N, Desc)) {
+  unsigned long long Num;
+  if (!llvm::getAsUnsignedInteger(**Scalar, 0, Num)) {
+return Located(Num, Scalar->Range);
+  }
+}
+warning(Desc + " invalid number", N);
+return std::nullopt;
+  }
+
   // Try to parse a list of single scalar values, or just a single value.
   std::optional>> scalarValues(Node ) {
 std::vector> Result;
Index: clang-tools-extra/clangd/ConfigFragment.h
===
--- clang-tools-extra/clangd/ConfigFragment.h
+++ clang-tools-extra/clangd/ConfigFragment.h
@@ -322,6 +322,8 @@
 std::optional> DeducedTypes;
 /// Show designators in aggregate initialization.
 std::optional> Designators;
+/// Limit the length of type name hints. (0 means no limit)
+std::optional> TypeNameLimit;
   };
   InlayHintsBlock InlayHints;
 };
Index: clang-tools-extra/clangd/ConfigCompile.cpp
===
--- clang-tools-extra/clangd/ConfigCompile.cpp
+++ clang-tools-extra/clangd/ConfigCompile.cpp
@@ -611,6 +611,11 @@
   Out.Apply.push_back([Value(**F.Designators)](const Params &, Config ) {
 C.InlayHints.Designators = Value;
   });
+if (F.TypeNameLimit)
+  Out.Apply.push_back(
+  [Value(**F.TypeNameLimit)](const Params &, Config ) {
+C.InlayHints.TypeNameLimit = Value;
+  });
   }
 
   constexpr static llvm::SourceMgr::DiagKind Error = llvm::SourceMgr::DK_Error;
Index: clang-tools-extra/clangd/Config.h
===
--- clang-tools-extra/clangd/Config.h
+++ clang-tools-extra/clangd/Config.h
@@ -147,6 +147,8 @@
 bool Parameters = true;
 bool DeducedTypes = true;
 bool Designators = true;
+// Limit the length of type names in inlay hints. (0 means no limit)
+uint32_t TypeNameLimit = 32;
   } InlayHints;
 };
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D147395: [Clangd] Make the type hint length limit configurable

[Yi Zhang via Phabricator via cfe-commits]

zhangyi1357 updated this revision to Diff 511950.
zhangyi1357 marked 2 inline comments as done.
zhangyi1357 added a comment.

- [Clangd] Remove unneccessary modification


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D147395/new/

https://reviews.llvm.org/D147395

Files:
  clang-tools-extra/clangd/Config.h
  clang-tools-extra/clangd/ConfigCompile.cpp
  clang-tools-extra/clangd/ConfigFragment.h
  clang-tools-extra/clangd/ConfigYAML.cpp
  clang-tools-extra/clangd/InlayHints.cpp
  clang-tools-extra/clangd/unittests/InlayHintTests.cpp

Index: clang-tools-extra/clangd/unittests/InlayHintTests.cpp
===
--- clang-tools-extra/clangd/unittests/InlayHintTests.cpp
+++ clang-tools-extra/clangd/unittests/InlayHintTests.cpp
@@ -1324,6 +1324,21 @@
 // Omit type hint past a certain length (currently 32)
 auto var = foo();
   )cpp");
+
+  Config Cfg;
+  Cfg.InlayHints.TypeNameLimit = 0;
+  WithContextValue WithCfg(Config::Key, std::move(Cfg));
+
+  assertTypeHints(
+  R"cpp(
+template 
+struct A {};
+struct MultipleWords {};
+A foo();
+// Should have type hint with TypeNameLimit = 0
+auto $var[[var]] = foo();
+  )cpp",
+  ExpectedHint{": A", "var"});
 }
 
 TEST(TypeHints, DefaultTemplateArgs) {
Index: clang-tools-extra/clangd/InlayHints.cpp
===
--- clang-tools-extra/clangd/InlayHints.cpp
+++ clang-tools-extra/clangd/InlayHints.cpp
@@ -688,7 +688,8 @@
   return;
 
 std::string TypeName = T.getAsString(Policy);
-if (TypeName.length() < TypeNameLimit)
+if (Cfg.InlayHints.TypeNameLimit == 0 ||
+TypeName.length() < Cfg.InlayHints.TypeNameLimit)
   addInlayHint(R, HintSide::Right, InlayHintKind::Type, Prefix, TypeName,
/*Suffix=*/"");
   }
@@ -714,8 +715,6 @@
   // the policies are initialized for more details.)
   PrintingPolicy TypeHintPolicy;
   PrintingPolicy StructuredBindingPolicy;
-
-  static const size_t TypeNameLimit = 32;
 };
 
 } // namespace
Index: clang-tools-extra/clangd/ConfigYAML.cpp
===
--- clang-tools-extra/clangd/ConfigYAML.cpp
+++ clang-tools-extra/clangd/ConfigYAML.cpp
@@ -254,6 +254,10 @@
   if (auto Value = boolValue(N, "Designators"))
 F.Designators = *Value;
 });
+Dict.handle("TypeNameLimit", [&](Node ) {
+  if (auto Value = uint32Value(N, "TypeNameLimit"))
+F.TypeNameLimit = *Value;
+});
 Dict.parse(N);
   }
 
@@ -375,6 +379,17 @@
 return std::nullopt;
   }
 
+  std::optional> uint32Value(Node , llvm::StringRef Desc) {
+if (auto Scalar = scalarValue(N, Desc)) {
+  unsigned long long Num;
+  if (!llvm::getAsUnsignedInteger(**Scalar, 0, Num)) {
+return Located(Num, Scalar->Range);
+  }
+}
+warning(Desc + " invalid number", N);
+return std::nullopt;
+  }
+
   // Try to parse a list of single scalar values, or just a single value.
   std::optional>> scalarValues(Node ) {
 std::vector> Result;
Index: clang-tools-extra/clangd/ConfigFragment.h
===
--- clang-tools-extra/clangd/ConfigFragment.h
+++ clang-tools-extra/clangd/ConfigFragment.h
@@ -322,6 +322,8 @@
 std::optional> DeducedTypes;
 /// Show designators in aggregate initialization.
 std::optional> Designators;
+/// Limit the length of type name hints. (0 means no limit)
+std::optional> TypeNameLimit;
   };
   InlayHintsBlock InlayHints;
 };
Index: clang-tools-extra/clangd/ConfigCompile.cpp
===
--- clang-tools-extra/clangd/ConfigCompile.cpp
+++ clang-tools-extra/clangd/ConfigCompile.cpp
@@ -611,6 +611,11 @@
   Out.Apply.push_back([Value(**F.Designators)](const Params &, Config ) {
 C.InlayHints.Designators = Value;
   });
+if (F.TypeNameLimit)
+  Out.Apply.push_back(
+  [Value(**F.TypeNameLimit)](const Params &, Config ) {
+C.InlayHints.TypeNameLimit = Value;
+  });
   }
 
   constexpr static llvm::SourceMgr::DiagKind Error = llvm::SourceMgr::DK_Error;
Index: clang-tools-extra/clangd/Config.h
===
--- clang-tools-extra/clangd/Config.h
+++ clang-tools-extra/clangd/Config.h
@@ -147,6 +147,8 @@
 bool Parameters = true;
 bool DeducedTypes = true;
 bool Designators = true;
+// Limit the length of type names in inlay hints. (0 means no limit)
+uint32_t TypeNameLimit = 32;
   } InlayHints;
 };
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D147395: [Clangd] Make the type hint length limit configurable

[Yi Zhang via Phabricator via cfe-commits]

zhangyi1357 updated this revision to Diff 510795.
zhangyi1357 added a comment.

- [Clangd] Add unittest for TypeNameLimit config


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D147395/new/

https://reviews.llvm.org/D147395

Files:
  clang-tools-extra/clangd/Config.h
  clang-tools-extra/clangd/ConfigCompile.cpp
  clang-tools-extra/clangd/ConfigFragment.h
  clang-tools-extra/clangd/ConfigYAML.cpp
  clang-tools-extra/clangd/InlayHints.cpp
  clang-tools-extra/clangd/unittests/InlayHintTests.cpp

Index: clang-tools-extra/clangd/unittests/InlayHintTests.cpp
===
--- clang-tools-extra/clangd/unittests/InlayHintTests.cpp
+++ clang-tools-extra/clangd/unittests/InlayHintTests.cpp
@@ -77,6 +77,7 @@
   C.InlayHints.Parameters = false;
   C.InlayHints.DeducedTypes = false;
   C.InlayHints.Designators = false;
+  C.InlayHints.TypeNameLimit = 1;
   return C;
 }
 
@@ -1324,6 +1325,21 @@
 // Omit type hint past a certain length (currently 32)
 auto var = foo();
   )cpp");
+
+  Config Cfg;
+  Cfg.InlayHints.TypeNameLimit = 0;
+  WithContextValue WithCfg(Config::Key, std::move(Cfg));
+
+  assertTypeHints(
+  R"cpp(
+template 
+struct A {};
+struct MultipleWords {};
+A foo();
+// Should have type hint with TypeNameLimit = 0
+auto $var[[var]] = foo();
+  )cpp",
+  ExpectedHint{": A", "var"});
 }
 
 TEST(TypeHints, DefaultTemplateArgs) {
Index: clang-tools-extra/clangd/InlayHints.cpp
===
--- clang-tools-extra/clangd/InlayHints.cpp
+++ clang-tools-extra/clangd/InlayHints.cpp
@@ -688,7 +688,8 @@
   return;
 
 std::string TypeName = T.getAsString(Policy);
-if (TypeName.length() < TypeNameLimit)
+if (Cfg.InlayHints.TypeNameLimit == 0 ||
+TypeName.length() < Cfg.InlayHints.TypeNameLimit)
   addInlayHint(R, HintSide::Right, InlayHintKind::Type, Prefix, TypeName,
/*Suffix=*/"");
   }
@@ -714,8 +715,6 @@
   // the policies are initialized for more details.)
   PrintingPolicy TypeHintPolicy;
   PrintingPolicy StructuredBindingPolicy;
-
-  static const size_t TypeNameLimit = 32;
 };
 
 } // namespace
Index: clang-tools-extra/clangd/ConfigYAML.cpp
===
--- clang-tools-extra/clangd/ConfigYAML.cpp
+++ clang-tools-extra/clangd/ConfigYAML.cpp
@@ -254,6 +254,10 @@
   if (auto Value = boolValue(N, "Designators"))
 F.Designators = *Value;
 });
+Dict.handle("TypeNameLimit", [&](Node ) {
+  if (auto Value = uint32Value(N, "TypeNameLimit"))
+F.TypeNameLimit = *Value;
+});
 Dict.parse(N);
   }
 
@@ -375,6 +379,17 @@
 return std::nullopt;
   }
 
+  std::optional> uint32Value(Node , llvm::StringRef Desc) {
+if (auto Scalar = scalarValue(N, Desc)) {
+  unsigned long long Num;
+  if (!llvm::getAsUnsignedInteger(**Scalar, 0, Num)) {
+return Located(Num, Scalar->Range);
+  }
+}
+warning(Desc + " invalid number", N);
+return std::nullopt;
+  }
+
   // Try to parse a list of single scalar values, or just a single value.
   std::optional>> scalarValues(Node ) {
 std::vector> Result;
Index: clang-tools-extra/clangd/ConfigFragment.h
===
--- clang-tools-extra/clangd/ConfigFragment.h
+++ clang-tools-extra/clangd/ConfigFragment.h
@@ -322,6 +322,8 @@
 std::optional> DeducedTypes;
 /// Show designators in aggregate initialization.
 std::optional> Designators;
+/// Limit the length of type name hints. (0 means no limit)
+std::optional> TypeNameLimit;
   };
   InlayHintsBlock InlayHints;
 };
Index: clang-tools-extra/clangd/ConfigCompile.cpp
===
--- clang-tools-extra/clangd/ConfigCompile.cpp
+++ clang-tools-extra/clangd/ConfigCompile.cpp
@@ -611,6 +611,11 @@
   Out.Apply.push_back([Value(**F.Designators)](const Params &, Config ) {
 C.InlayHints.Designators = Value;
   });
+if (F.TypeNameLimit)
+  Out.Apply.push_back(
+  [Value(**F.TypeNameLimit)](const Params &, Config ) {
+C.InlayHints.TypeNameLimit = Value;
+  });
   }
 
   constexpr static llvm::SourceMgr::DiagKind Error = llvm::SourceMgr::DK_Error;
Index: clang-tools-extra/clangd/Config.h
===
--- clang-tools-extra/clangd/Config.h
+++ clang-tools-extra/clangd/Config.h
@@ -147,6 +147,8 @@
 bool Parameters = true;
 bool DeducedTypes = true;
 bool Designators = true;
+// Limit the length of type names in inlay hints. (0 means no limit)
+uint32_t TypeNameLimit = 32;
   } InlayHints;
 };
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org

[PATCH] D147395: [Clangd] Make the type hint length limit configurable

[Yi Zhang via Phabricator via cfe-commits]

zhangyi1357 added a comment.

In D147395#4240227 , @hokein wrote:

> Thanks for the contribution!

It's quite interesting for me. Thanks for your time reviewing and your great 
advice!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D147395/new/

https://reviews.llvm.org/D147395

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

[PATCH] D147395: [Clangd] Make the type hint length limit configurable

[Yi Zhang via Phabricator via cfe-commits]

zhangyi1357 updated this revision to Diff 510521.
zhangyi1357 marked an inline comment as not done.
zhangyi1357 added a comment.

Try to fix the build problem.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D147395/new/

https://reviews.llvm.org/D147395

Files:
  clang-tools-extra/clangd/Config.h
  clang-tools-extra/clangd/ConfigCompile.cpp
  clang-tools-extra/clangd/ConfigFragment.h
  clang-tools-extra/clangd/ConfigYAML.cpp
  clang-tools-extra/clangd/InlayHints.cpp


Index: clang-tools-extra/clangd/InlayHints.cpp
===
--- clang-tools-extra/clangd/InlayHints.cpp
+++ clang-tools-extra/clangd/InlayHints.cpp
@@ -688,7 +688,8 @@
   return;
 
 std::string TypeName = T.getAsString(Policy);
-if (TypeName.length() < TypeNameLimit)
+if (Cfg.InlayHints.TypeNameLimit == 0 ||
+TypeName.length() < Cfg.InlayHints.TypeNameLimit)
   addInlayHint(R, HintSide::Right, InlayHintKind::Type, Prefix, TypeName,
/*Suffix=*/"");
   }
@@ -714,8 +715,6 @@
   // the policies are initialized for more details.)
   PrintingPolicy TypeHintPolicy;
   PrintingPolicy StructuredBindingPolicy;
-
-  static const size_t TypeNameLimit = 32;
 };
 
 } // namespace
Index: clang-tools-extra/clangd/ConfigYAML.cpp
===
--- clang-tools-extra/clangd/ConfigYAML.cpp
+++ clang-tools-extra/clangd/ConfigYAML.cpp
@@ -254,6 +254,10 @@
   if (auto Value = boolValue(N, "Designators"))
 F.Designators = *Value;
 });
+Dict.handle("TypeNameLimit", [&](Node ) {
+  if (auto Value = uint32Value(N, "TypeNameLimit"))
+F.TypeNameLimit = *Value;
+});
 Dict.parse(N);
   }
 
@@ -375,6 +379,17 @@
 return std::nullopt;
   }
 
+  std::optional> uint32Value(Node , llvm::StringRef Desc) {
+if (auto Scalar = scalarValue(N, Desc)) {
+  unsigned long long Num;
+  if (!llvm::getAsUnsignedInteger(**Scalar, 0, Num)) {
+return Located(Num, Scalar->Range);
+  }
+}
+warning(Desc + " invalid number", N);
+return std::nullopt;
+  }
+
   // Try to parse a list of single scalar values, or just a single value.
   std::optional>> scalarValues(Node ) {
 std::vector> Result;
Index: clang-tools-extra/clangd/ConfigFragment.h
===
--- clang-tools-extra/clangd/ConfigFragment.h
+++ clang-tools-extra/clangd/ConfigFragment.h
@@ -322,6 +322,8 @@
 std::optional> DeducedTypes;
 /// Show designators in aggregate initialization.
 std::optional> Designators;
+/// Limit the length of type name hints. (0 means no limit)
+std::optional> TypeNameLimit;
   };
   InlayHintsBlock InlayHints;
 };
Index: clang-tools-extra/clangd/ConfigCompile.cpp
===
--- clang-tools-extra/clangd/ConfigCompile.cpp
+++ clang-tools-extra/clangd/ConfigCompile.cpp
@@ -611,6 +611,11 @@
   Out.Apply.push_back([Value(**F.Designators)](const Params &, Config ) {
 C.InlayHints.Designators = Value;
   });
+if (F.TypeNameLimit)
+  Out.Apply.push_back(
+  [Value(**F.TypeNameLimit)](const Params &, Config ) {
+C.InlayHints.TypeNameLimit = Value;
+  });
   }
 
   constexpr static llvm::SourceMgr::DiagKind Error = llvm::SourceMgr::DK_Error;
Index: clang-tools-extra/clangd/Config.h
===
--- clang-tools-extra/clangd/Config.h
+++ clang-tools-extra/clangd/Config.h
@@ -147,6 +147,8 @@
 bool Parameters = true;
 bool DeducedTypes = true;
 bool Designators = true;
+// Limit the length of type names in inlay hints. (0 means no limit)
+uint32_t TypeNameLimit = 32;
   } InlayHints;
 };
 


Index: clang-tools-extra/clangd/InlayHints.cpp
===
--- clang-tools-extra/clangd/InlayHints.cpp
+++ clang-tools-extra/clangd/InlayHints.cpp
@@ -688,7 +688,8 @@
   return;
 
 std::string TypeName = T.getAsString(Policy);
-if (TypeName.length() < TypeNameLimit)
+if (Cfg.InlayHints.TypeNameLimit == 0 ||
+TypeName.length() < Cfg.InlayHints.TypeNameLimit)
   addInlayHint(R, HintSide::Right, InlayHintKind::Type, Prefix, TypeName,
/*Suffix=*/"");
   }
@@ -714,8 +715,6 @@
   // the policies are initialized for more details.)
   PrintingPolicy TypeHintPolicy;
   PrintingPolicy StructuredBindingPolicy;
-
-  static const size_t TypeNameLimit = 32;
 };
 
 } // namespace
Index: clang-tools-extra/clangd/ConfigYAML.cpp
===
--- clang-tools-extra/clangd/ConfigYAML.cpp
+++ clang-tools-extra/clangd/ConfigYAML.cpp
@@ -254,6 +254,10 @@
   if (auto Value = boolValue(N, "Designators"))
 F.Designators = *Value;
 });
+

[PATCH] D147395: [Clangd] Make the type hint length limit configurable

[Yi Zhang via Phabricator via cfe-commits]

zhangyi1357 marked 2 inline comments as done.
zhangyi1357 added a comment.

In D147395#4240227 , @hokein wrote:

> Thanks for the contribution!

It's really interesting for me. Thanks for your time reviewing!




Comment at: clang-tools-extra/clangd/Config.h:151
+// Limit the length of type names in inlay hints.
+size_t TypeNameLimit = 32;
   } InlayHints;

hokein wrote:
> I would extend it a bit more -- 0 means no limit. 
> 
> Can you also add a unittest in `TEST(TypeHints, LongTypeName)` in 
> `InlayHintTests.cpp`? 
> 0 means no limit.
This is quite a good idea. I've done it.

For unittest, there is already `TEST(TypeHints, LongTypeName)` in 
`InlayHintTests.cpp`. Do you mean add more tests in the same `TEST` or another 
`TEST` with TypeNameLimit configured?



Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D147395/new/

https://reviews.llvm.org/D147395

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

[PATCH] D147395: [Clangd] Make the type hint length limit configurable

[Yi Zhang via Phabricator via cfe-commits]

zhangyi1357 updated this revision to Diff 510508.
zhangyi1357 added a comment.

Add uint32Value() for uint32_t type parsing in config file.
For TypeNameLimit config, support no limit with value 0.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D147395/new/

https://reviews.llvm.org/D147395

Files:
  clang-tools-extra/clangd/Config.h
  clang-tools-extra/clangd/ConfigCompile.cpp
  clang-tools-extra/clangd/ConfigFragment.h
  clang-tools-extra/clangd/ConfigYAML.cpp
  clang-tools-extra/clangd/InlayHints.cpp


Index: clang-tools-extra/clangd/InlayHints.cpp
===
--- clang-tools-extra/clangd/InlayHints.cpp
+++ clang-tools-extra/clangd/InlayHints.cpp
@@ -688,7 +688,8 @@
   return;
 
 std::string TypeName = T.getAsString(Policy);
-if (TypeName.length() < Cfg.InlayHints.TypeNameLimit)
+if (Cfg.InlayHints.TypeNameLimit == 0 ||
+TypeName.length() < Cfg.InlayHints.TypeNameLimit)
   addInlayHint(R, HintSide::Right, InlayHintKind::Type, Prefix, TypeName,
/*Suffix=*/"");
   }
Index: clang-tools-extra/clangd/ConfigYAML.cpp
===
--- clang-tools-extra/clangd/ConfigYAML.cpp
+++ clang-tools-extra/clangd/ConfigYAML.cpp
@@ -255,7 +255,7 @@
 F.Designators = *Value;
 });
 Dict.handle("TypeNameLimit", [&](Node ) {
-  if (auto Value = scalarValue(N, "TypeNameLimit"))
+  if (auto Value = uint32Value(N, "TypeNameLimit"))
 F.TypeNameLimit = *Value;
 });
 Dict.parse(N);
@@ -379,6 +379,17 @@
 return std::nullopt;
   }
 
+  std::optional> uint32Value(Node , llvm::StringRef Desc) {
+if (auto Scalar = scalarValue(N, Desc)) {
+  unsigned long long Num;
+  if (!llvm::getAsUnsignedInteger(**Scalar, 0, Num)) {
+return Located(Num, Scalar->Range);
+  }
+}
+warning(Desc + " invalid number", N);
+return std::nullopt;
+  }
+
   // Try to parse a list of single scalar values, or just a single value.
   std::optional>> scalarValues(Node ) {
 std::vector> Result;
Index: clang-tools-extra/clangd/ConfigFragment.h
===
--- clang-tools-extra/clangd/ConfigFragment.h
+++ clang-tools-extra/clangd/ConfigFragment.h
@@ -322,8 +322,8 @@
 std::optional> DeducedTypes;
 /// Show designators in aggregate initialization.
 std::optional> Designators;
-/// Limit the length of type name hints.
-std::optional> TypeNameLimit;
+/// Limit the length of type name hints. (0 means no limit)
+std::optional> TypeNameLimit;
   };
   InlayHintsBlock InlayHints;
 };
Index: clang-tools-extra/clangd/ConfigCompile.cpp
===
--- clang-tools-extra/clangd/ConfigCompile.cpp
+++ clang-tools-extra/clangd/ConfigCompile.cpp
@@ -614,7 +614,7 @@
 if (F.TypeNameLimit)
   Out.Apply.push_back(
   [Value(**F.TypeNameLimit)](const Params &, Config ) {
-C.InlayHints.TypeNameLimit = stoul(Value);
+C.InlayHints.TypeNameLimit = Value;
   });
   }
 
Index: clang-tools-extra/clangd/Config.h
===
--- clang-tools-extra/clangd/Config.h
+++ clang-tools-extra/clangd/Config.h
@@ -147,8 +147,8 @@
 bool Parameters = true;
 bool DeducedTypes = true;
 bool Designators = true;
-// Limit the length of type names in inlay hints.
-size_t TypeNameLimit = 32;
+// Limit the length of type names in inlay hints. (0 means no limit)
+uint32_t TypeNameLimit = 32;
   } InlayHints;
 };
 


Index: clang-tools-extra/clangd/InlayHints.cpp
===
--- clang-tools-extra/clangd/InlayHints.cpp
+++ clang-tools-extra/clangd/InlayHints.cpp
@@ -688,7 +688,8 @@
   return;
 
 std::string TypeName = T.getAsString(Policy);
-if (TypeName.length() < Cfg.InlayHints.TypeNameLimit)
+if (Cfg.InlayHints.TypeNameLimit == 0 ||
+TypeName.length() < Cfg.InlayHints.TypeNameLimit)
   addInlayHint(R, HintSide::Right, InlayHintKind::Type, Prefix, TypeName,
/*Suffix=*/"");
   }
Index: clang-tools-extra/clangd/ConfigYAML.cpp
===
--- clang-tools-extra/clangd/ConfigYAML.cpp
+++ clang-tools-extra/clangd/ConfigYAML.cpp
@@ -255,7 +255,7 @@
 F.Designators = *Value;
 });
 Dict.handle("TypeNameLimit", [&](Node ) {
-  if (auto Value = scalarValue(N, "TypeNameLimit"))
+  if (auto Value = uint32Value(N, "TypeNameLimit"))
 F.TypeNameLimit = *Value;
 });
 Dict.parse(N);
@@ -379,6 +379,17 @@
 return std::nullopt;
   }
 
+  std::optional> uint32Value(Node , llvm::StringRef Desc) {
+if (auto Scalar = scalarValue(N, Desc)) {
+  unsigned long long Num;
+  if 

[PATCH] D147395: [Clangd] Make the type hint length limit configurable

[Yi Zhang via Phabricator via cfe-commits]

zhangyi1357 updated this revision to Diff 510292.
zhangyi1357 added a comment.

Format code modified using clang format.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D147395/new/

https://reviews.llvm.org/D147395

Files:
  clang-tools-extra/clangd/Config.h
  clang-tools-extra/clangd/ConfigCompile.cpp
  clang-tools-extra/clangd/ConfigFragment.h
  clang-tools-extra/clangd/ConfigYAML.cpp
  clang-tools-extra/clangd/InlayHints.cpp


Index: clang-tools-extra/clangd/InlayHints.cpp
===
--- clang-tools-extra/clangd/InlayHints.cpp
+++ clang-tools-extra/clangd/InlayHints.cpp
@@ -688,7 +688,7 @@
   return;
 
 std::string TypeName = T.getAsString(Policy);
-if (TypeName.length() < TypeNameLimit)
+if (TypeName.length() < Cfg.InlayHints.TypeNameLimit)
   addInlayHint(R, HintSide::Right, InlayHintKind::Type, Prefix, TypeName,
/*Suffix=*/"");
   }
@@ -714,8 +714,6 @@
   // the policies are initialized for more details.)
   PrintingPolicy TypeHintPolicy;
   PrintingPolicy StructuredBindingPolicy;
-
-  static const size_t TypeNameLimit = 32;
 };
 
 } // namespace
Index: clang-tools-extra/clangd/ConfigYAML.cpp
===
--- clang-tools-extra/clangd/ConfigYAML.cpp
+++ clang-tools-extra/clangd/ConfigYAML.cpp
@@ -254,6 +254,10 @@
   if (auto Value = boolValue(N, "Designators"))
 F.Designators = *Value;
 });
+Dict.handle("TypeNameLimit", [&](Node ) {
+  if (auto Value = scalarValue(N, "TypeNameLimit"))
+F.TypeNameLimit = *Value;
+});
 Dict.parse(N);
   }
 
Index: clang-tools-extra/clangd/ConfigFragment.h
===
--- clang-tools-extra/clangd/ConfigFragment.h
+++ clang-tools-extra/clangd/ConfigFragment.h
@@ -322,6 +322,8 @@
 std::optional> DeducedTypes;
 /// Show designators in aggregate initialization.
 std::optional> Designators;
+/// Limit the length of type name hints.
+std::optional> TypeNameLimit;
   };
   InlayHintsBlock InlayHints;
 };
Index: clang-tools-extra/clangd/ConfigCompile.cpp
===
--- clang-tools-extra/clangd/ConfigCompile.cpp
+++ clang-tools-extra/clangd/ConfigCompile.cpp
@@ -611,6 +611,11 @@
   Out.Apply.push_back([Value(**F.Designators)](const Params &, Config ) {
 C.InlayHints.Designators = Value;
   });
+if (F.TypeNameLimit)
+  Out.Apply.push_back(
+  [Value(**F.TypeNameLimit)](const Params &, Config ) {
+C.InlayHints.TypeNameLimit = stoul(Value);
+  });
   }
 
   constexpr static llvm::SourceMgr::DiagKind Error = llvm::SourceMgr::DK_Error;
Index: clang-tools-extra/clangd/Config.h
===
--- clang-tools-extra/clangd/Config.h
+++ clang-tools-extra/clangd/Config.h
@@ -147,6 +147,8 @@
 bool Parameters = true;
 bool DeducedTypes = true;
 bool Designators = true;
+// Limit the length of type names in inlay hints.
+size_t TypeNameLimit = 32;
   } InlayHints;
 };
 


Index: clang-tools-extra/clangd/InlayHints.cpp
===
--- clang-tools-extra/clangd/InlayHints.cpp
+++ clang-tools-extra/clangd/InlayHints.cpp
@@ -688,7 +688,7 @@
   return;
 
 std::string TypeName = T.getAsString(Policy);
-if (TypeName.length() < TypeNameLimit)
+if (TypeName.length() < Cfg.InlayHints.TypeNameLimit)
   addInlayHint(R, HintSide::Right, InlayHintKind::Type, Prefix, TypeName,
/*Suffix=*/"");
   }
@@ -714,8 +714,6 @@
   // the policies are initialized for more details.)
   PrintingPolicy TypeHintPolicy;
   PrintingPolicy StructuredBindingPolicy;
-
-  static const size_t TypeNameLimit = 32;
 };
 
 } // namespace
Index: clang-tools-extra/clangd/ConfigYAML.cpp
===
--- clang-tools-extra/clangd/ConfigYAML.cpp
+++ clang-tools-extra/clangd/ConfigYAML.cpp
@@ -254,6 +254,10 @@
   if (auto Value = boolValue(N, "Designators"))
 F.Designators = *Value;
 });
+Dict.handle("TypeNameLimit", [&](Node ) {
+  if (auto Value = scalarValue(N, "TypeNameLimit"))
+F.TypeNameLimit = *Value;
+});
 Dict.parse(N);
   }
 
Index: clang-tools-extra/clangd/ConfigFragment.h
===
--- clang-tools-extra/clangd/ConfigFragment.h
+++ clang-tools-extra/clangd/ConfigFragment.h
@@ -322,6 +322,8 @@
 std::optional> DeducedTypes;
 /// Show designators in aggregate initialization.
 std::optional> Designators;
+/// Limit the length of type name hints.
+std::optional> TypeNameLimit;
   };
   InlayHintsBlock InlayHints;
 };
Index: clang-tools-extra/clangd/ConfigCompile.cpp

[PATCH] D147395: [Clangd] Make the type hint length limit configurable

[Yi Zhang via Phabricator via cfe-commits]

zhangyi1357 created this revision.
Herald added subscribers: kadircet, arphaman.
Herald added a project: All.
zhangyi1357 requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov.
Herald added a project: clang-tools-extra.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D147395

Files:
  clang-tools-extra/clangd/Config.h
  clang-tools-extra/clangd/ConfigCompile.cpp
  clang-tools-extra/clangd/ConfigFragment.h
  clang-tools-extra/clangd/ConfigYAML.cpp
  clang-tools-extra/clangd/InlayHints.cpp


Index: clang-tools-extra/clangd/InlayHints.cpp
===
--- clang-tools-extra/clangd/InlayHints.cpp
+++ clang-tools-extra/clangd/InlayHints.cpp
@@ -688,7 +688,7 @@
   return;
 
 std::string TypeName = T.getAsString(Policy);
-if (TypeName.length() < TypeNameLimit)
+if (TypeName.length() < Cfg.InlayHints.TypeNameLimit)
   addInlayHint(R, HintSide::Right, InlayHintKind::Type, Prefix, TypeName,
/*Suffix=*/"");
   }
@@ -714,8 +714,6 @@
   // the policies are initialized for more details.)
   PrintingPolicy TypeHintPolicy;
   PrintingPolicy StructuredBindingPolicy;
-
-  static const size_t TypeNameLimit = 32;
 };
 
 } // namespace
Index: clang-tools-extra/clangd/ConfigYAML.cpp
===
--- clang-tools-extra/clangd/ConfigYAML.cpp
+++ clang-tools-extra/clangd/ConfigYAML.cpp
@@ -254,6 +254,10 @@
   if (auto Value = boolValue(N, "Designators"))
 F.Designators = *Value;
 });
+Dict.handle("TypeNameLimit", [&](Node ) {
+  if (auto Value = scalarValue(N, "TypeNameLimit"))
+F.TypeNameLimit = *Value;
+});
 Dict.parse(N);
   }
 
Index: clang-tools-extra/clangd/ConfigFragment.h
===
--- clang-tools-extra/clangd/ConfigFragment.h
+++ clang-tools-extra/clangd/ConfigFragment.h
@@ -322,6 +322,8 @@
 std::optional> DeducedTypes;
 /// Show designators in aggregate initialization.
 std::optional> Designators;
+/// Limit the length of type name hints.
+std::optional> TypeNameLimit;
   };
   InlayHintsBlock InlayHints;
 };
Index: clang-tools-extra/clangd/ConfigCompile.cpp
===
--- clang-tools-extra/clangd/ConfigCompile.cpp
+++ clang-tools-extra/clangd/ConfigCompile.cpp
@@ -611,6 +611,11 @@
   Out.Apply.push_back([Value(**F.Designators)](const Params &, Config ) {
 C.InlayHints.Designators = Value;
   });
+if (F.TypeNameLimit)
+  Out.Apply.push_back(
+  [Value(**F.TypeNameLimit)](const Params &, Config ) {
+C.InlayHints.TypeNameLimit = stoul(Value);
+  });
   }
 
   constexpr static llvm::SourceMgr::DiagKind Error = llvm::SourceMgr::DK_Error;
Index: clang-tools-extra/clangd/Config.h
===
--- clang-tools-extra/clangd/Config.h
+++ clang-tools-extra/clangd/Config.h
@@ -147,6 +147,8 @@
 bool Parameters = true;
 bool DeducedTypes = true;
 bool Designators = true;
+// Limit the length of type names in inlay hints.
+size_t TypeNameLimit = 32;
   } InlayHints;
 };
 


Index: clang-tools-extra/clangd/InlayHints.cpp
===
--- clang-tools-extra/clangd/InlayHints.cpp
+++ clang-tools-extra/clangd/InlayHints.cpp
@@ -688,7 +688,7 @@
   return;
 
 std::string TypeName = T.getAsString(Policy);
-if (TypeName.length() < TypeNameLimit)
+if (TypeName.length() < Cfg.InlayHints.TypeNameLimit)
   addInlayHint(R, HintSide::Right, InlayHintKind::Type, Prefix, TypeName,
/*Suffix=*/"");
   }
@@ -714,8 +714,6 @@
   // the policies are initialized for more details.)
   PrintingPolicy TypeHintPolicy;
   PrintingPolicy StructuredBindingPolicy;
-
-  static const size_t TypeNameLimit = 32;
 };
 
 } // namespace
Index: clang-tools-extra/clangd/ConfigYAML.cpp
===
--- clang-tools-extra/clangd/ConfigYAML.cpp
+++ clang-tools-extra/clangd/ConfigYAML.cpp
@@ -254,6 +254,10 @@
   if (auto Value = boolValue(N, "Designators"))
 F.Designators = *Value;
 });
+Dict.handle("TypeNameLimit", [&](Node ) {
+  if (auto Value = scalarValue(N, "TypeNameLimit"))
+F.TypeNameLimit = *Value;
+});
 Dict.parse(N);
   }
 
Index: clang-tools-extra/clangd/ConfigFragment.h
===
--- clang-tools-extra/clangd/ConfigFragment.h
+++ clang-tools-extra/clangd/ConfigFragment.h
@@ -322,6 +322,8 @@
 std::optional> DeducedTypes;
 /// Show designators in aggregate initialization.
 std::optional> Designators;
+/// Limit the length of type name hints.
+std::optional> TypeNameLimit;
   };
   InlayHintsBlock