nridge added inline comments.

================
Comment at: clang-tools-extra/clangd/InlayHints.cpp:578
+    constexpr unsigned HintMinLineLimit = 2;
+    constexpr unsigned HintMaxLengthLimit = 50;
+
----------------
daiyousei-qz wrote:
> sammccall wrote:
> > We actually already have a configurable inlay hint length limit.
> > It has the wrong name (`TypeNameLimit`) , but I think we should use it 
> > anyway (and maybe try to fix the name later).
> > 
> > (I'm not sure making this configurable was a great idea, but given that it 
> > exists, people will expect it to effectively limit the length of hints)
> `TypeNameLimit` was introduced to limit the length of "DeducedTypes" only. I 
> don't think using that number for all hints is a good idea because different 
> hints appear in different contexts. For deduced type hint, it is displayed in 
> the middle of the actual code, so it must be concise to not overwhelm the 
> actual code. However, this hint is usually displayed at the end of an almost 
> empty line. A much larger number of characters should be allowed.
> 
> (I'm not arguing here, but IMO such options are definitely helpful. Not 
> everybody would fork llvm-project and build their own version of clangd 
> binary. Without options to configure length of hints, people would disable 
> "DeducedTypes" entirely if they cannot tolerate the default length limit, 
> while this feature is definitely a cool thing to turn on. Personally, I 
> actually think clangd has too little option compared to say rust-analyzer. 
> But it's just my understanding)
> For deduced type hint, it is displayed in the middle of the actual code, so 
> it must be concise to not overwhelm the actual code. However, this hint is 
> usually displayed at the end of an almost empty line. A much larger number of 
> characters should be allowed.

Another consideration besides the location of the hint that is worth keeping in 
mind, is what type of content is being printed.

Type names in C++ are composable in ways that identifiers are not (e.g. 
`vector<basic_string<char, char_traits<char>, allocator<char>, 
allocator<basic_string<...>>` etc.), such that there is a need to limit type 
hints that doesn't really apply to say, parameter hints.

So, a question I have here is: can template argument lists appear in an 
end-definition-comment hint?

For example, for:

```
template <typename T>
struct S {
  void foo();
};

template <typename T>
void S::foo() {
}  // <--- HERE
```

is the hint at the indicated location `S::foo()`, or `S<T>::foo()`? In the 
latter case, we can imagine the hint getting long due to the type parameters, 
and we may want to consider either limiting its length, or tweaking the code to 
not print the type parameters.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150635

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

Reply via email to