[PATCH] D116387: [CodeCompletion][clangd] Clean __uglified parameter names in completion & hover

Mon Jan 10 03:10:04 PST 2022

sammccall added a comment.

In D116387#3230893 <https://reviews.llvm.org/D116387#3230893>, @hokein wrote:

> Agree that __ reserved names are ugly,  but I'm not sure this is a great improvement.
>
> I played around the patch locally, and found the new behavior being confused in some cases (mostly inconsistencies between deuglified places vs uglified places), and seems hard for readers to predict it:

Agree, but I don't think it needs to be predictable, it's enough that the output can be understood.
(e.g. I never particularly noticed *which* identifiers in stdlib were ugly, just that the thing overall was hard to read).
i.e. if we remove the underscores half the time, that seems like a win.

> - inconsistency with doc-comment which still uses the __ name (this seems hard to fix)
>
> F21562126: image.png <https://reviews.llvm.org/F21562126>

Yes :-( However I don't think a human reader is likely to be confused by this.

> - the print type in hover still uses __name (maybe this is expected, or we could introduce a `reserved` field in hover, like `template-type-param Tp (reserved)`)
>
> F21562145: image.png <https://reviews.llvm.org/F21562145>

This is an oversight, I'll fix this.

> - the deuglified behavior is triggered on (template/function) parameters, which means we have uglified name for `void foo(_ReservedClass& abc)` vs deuglified name for `template<typename _ReservedClass> void foo(_ReservedClass& abc)` (expanding the scope could fix that, but it doesn't seem something we want to do)

This is deliberate: `_ReservedClass` is part of the API of `foo`, so renaming it is a semantic change.
I don't this change increases the amount of inconsistency:  today we have`vector` vs `push_back` vs `_ReservedClass` vs `_Tp`.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116387