lh123 added inline comments.
================
Comment at: clang-tools-extra/clangd/Hover.cpp:1271
OS << " = " << *P.Default;
+ if (P.Type && P.Type->AKA)
+ OS << llvm::formatv(" (aka {0})", *P.Type->AKA);
----------------
lh123 wrote:
> sammccall wrote:
> > kadircet wrote:
> > > sammccall wrote:
> > > > Hmm, this seems to render as:
> > > >
> > > > ```std::string Foo = "" (aka basic_string<char>)```
> > > >
> > > > it's not *completely* clear what the aka refers to.
> > > > I don't have a better solution, though.
> > > > @kadircet any opinions here?
> > > i think it's attached to the type of the parameter, not it's name nor the
> > > initializer-expr. so I think we should move this closer to the type (i.e.
> > > `std::string (aka basic_sting<char>) Foo = ""`, basically
> > > `llvm::toString(P.Type)` ?)
> > > i think it's attached to the type of the parameter
> >
> > This is logically correct but I think it's harder to read. This puts text
> > in the middle of code, in a way that doesn't seem obvious to me: parens
> > mean several things in C++ and it may be hard to recognize this means none
> > of them.
> >
> > Worst case is we have function types: `word(*)() (aka long(*)()) x =
> > nullptr`
> >
> > It also disrupts the reading flow in the case where the aka is *not* needed
> > for understanding.
> > I think overall the previous version was better, though not great.
> >
> > I'm tempted to say we should scope down this patch further until we have a
> > better feel for how it behaves, i.e. exclude param types from aka for now.
> > Param types are less obviously useful to disambiguate than result types.
> > (e.g. because in most cases you can hover over the input expression).
> > I'm tempted to say we should scope down this patch further until we have a
> > better feel for how it behaves, i.e. exclude param types from aka for now.
> > Param types are less obviously useful to disambiguate than result types.
> > (e.g. because in most cases you can hover over the input expression).
>
> I‘d like to keep the `a.k.a` type for parameter. because:
> 1. sometime we pass `literal (or null pointer)` as parameter, but clang
> doesn't support hover on literal. eg:
> ```
> using mint = int;
> void foo(mint *);
> void code() {
> foo(nullptr); // hover over foo, we can get 'mint * (aka int *)'
> }
> ```
> 2. It may be useful when making function calls, although it does not work
> well when the function is overloaded.**(add a.k.a type to signature help?)**
> eg:
> ```
> using mint = int;
> void foo(mint *);
> void code() {
> foo(); // hover on foo, we can get 'mint * (aka int *)'
> }
> ```
>
> > I think overall the previous version was better, though not great.
>
> Agree with that, it seems that putting a.k.a in the middle of the code makes
> the hover look bad based on my recent use
It is also useful when pass function calling as parameters.
```
struct Test {
Test(int);
};
using Alias = Test;
int foo();
void bar(Alias);
void code() {
bar(foo()); // Hover over bar, we can know that 'int' is implicitly
converted to 'Test'
}
```
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D114522/new/
https://reviews.llvm.org/D114522
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
