ilya-biryukov accepted this revision.
ilya-biryukov marked an inline comment as done.
ilya-biryukov added a comment.
This revision is now accepted and ready to land.
LGTM
================
Comment at: clang-tools-extra/clangd/unittests/HoverTests.cpp:136
HI.Kind = index::SymbolKind::Class;
- HI.Definition = "template <typename T> class vector {}";
- HI.TemplateParameters = {
- {std::string("typename"), std::string("T"), llvm::None},
- };
+ HI.Definition = "template <> class vector<int> {}";
}},
----------------
kadircet wrote:
> ilya-biryukov wrote:
> > Is this the intended behavior?
> > I would've interepreted `Definition` as something that was written in the
> > source code, but this one here is definitely synthesized by the compiler.
> >
> > I'm happy to go either way, just wanted to make sure this is our conscious
> > choice and understand why we're doing this.
> I also wasn't sure about this one, but thought this would be more useful to
> users.
>
> Because shows more semantic information about the symbol under the cursor,
> and we want to achieve that with Hover.
I actually think it would be more useful to say `vector<int>` in `Name` (or
`Name` + `TemplateArguments` per Sam's comments) and have definition reflect
what's written in the source code.
But up to you, can also keep as is.
We can always tweak it later if users complain.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D71596/new/
https://reviews.llvm.org/D71596
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
