sammccall added a comment.
Layering and such looks good. This should compose well with D58547
<https://reviews.llvm.org/D58547>
================
Comment at: clang-tools-extra/clangd/XRefs.h:52
+struct HoverInfo {
+ LocatedSymbol Sym;
+ /// Name of the context containing the symbol.
----------------
I'm not sure about reuse of LocatedSymbol - do we want to commit to returning
decl/def locations?
Name might be enough here.
================
Comment at: clang-tools-extra/clangd/XRefs.h:54
+ /// Name of the context containing the symbol.
+ std::string ContainerName;
+ index::SymbolInfo SI;
----------------
This comes from LSP, and within the scope of C++ I think we might want stronger
semantics here.
I think it's likely we actually want to show namespace vs class scope
differently, too - maybe these should be separate fields?
================
Comment at: clang-tools-extra/clangd/XRefs.h:55
+ std::string ContainerName;
+ index::SymbolInfo SI;
+ /// Includes all the qualifiers.
----------------
SymbolInfo is a bit of a mess. Maybe we just want SymbolInfo::Kind? (LSP
SymbolKind is tempting but loses a lot of info for C++).
I do think we'll need to extend SymbolInfo::Kind or eventually use our own
enum, e.g. lambdas may need their own kind (they're variables, but don't have a
printable type and need to be displayed more like functions)
================
Comment at: clang-tools-extra/clangd/XRefs.h:57
+ /// Includes all the qualifiers.
+ std::string Type;
+ /// Empty for non-functions. First element is the return type.
----------------
I think we probably want a struct to represent types, so we can annotate it
with links later on if needed. (because type can be e.g. `mytemplate<mytype>`).
This wouldn't matter except we should probably reuse it...
================
Comment at: clang-tools-extra/clangd/XRefs.h:57
+ /// Includes all the qualifiers.
+ std::string Type;
+ /// Empty for non-functions. First element is the return type.
----------------
sammccall wrote:
> I think we probably want a struct to represent types, so we can annotate it
> with links later on if needed. (because type can be e.g.
> `mytemplate<mytype>`). This wouldn't matter except we should probably reuse
> it...
we may want ReturnType too (unless you want to overload Type for that - I'd
suggest against it because of e.g. lambdas)
================
Comment at: clang-tools-extra/clangd/XRefs.h:59
+ /// Empty for non-functions. First element is the return type.
+ std::vector<LocatedSymbol> Signature;
+ std::string Documentation;
----------------
maybe `vector<param>` params, where param is `struct { string name; Type type
}`?
We might render as:
***
**Parameters**:
- **value**: `std::string`
- **size**: `int` (default = `0`)
================
Comment at: clang-tools-extra/clangd/XRefs.h:63
+ std::string Definition;
+ std::string TemplateArgs;
+};
----------------
TemplateArgs might want to follow the same structure as params?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D61497/new/
https://reviews.llvm.org/D61497
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
